Re: [PATCH v2] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events.
On Fri, Sep 16, 2022 at 3:44 AM Venu Busireddy
wrote:
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 41f2a5630173..69194c7ae23c 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -608,7 +608,19 @@ static void virtio_scsi_command_complete(SCSIRequest *r,
> size_t resid)
>
> req->resp.cmd.response = VIRTIO_SCSI_S_OK;
> req->resp.cmd.status = r->status;
> -if (req->resp.cmd.status == GOOD) {
> +if (req->dev->reported_luns_changed &&
> +(req->req.cmd.cdb[0] != INQUIRY) &&
> +(req->req.cmd.cdb[0] != REPORT_LUNS) &&
> +(req->req.cmd.cdb[0] != REQUEST_SENSE)) {
> +req->dev->reported_luns_changed = false;
> +req->resp.cmd.resid = 0;
> +req->resp.cmd.status_qualifier = 0;
> +req->resp.cmd.status = CHECK_CONDITION;
> +sense_len = scsi_build_sense(sense,
> SENSE_CODE(REPORTED_LUNS_CHANGED));
> +qemu_iovec_from_buf(>resp_iov, sizeof(req->resp.cmd),
> +sense, sense_len);
> +req->resp.cmd.sense_len = virtio_tswap32(vdev, sense_len);
> +} else if (req->resp.cmd.status == GOOD) {
> req->resp.cmd.resid = virtio_tswap32(vdev, resid);
> } else {
> req->resp.cmd.resid = 0;
Hi,
a unit attention sense must be sent _instead_ of executing the command.
QEMU already has a function scsi_device_set_ua() that handles
everything; you have to call it, if reported_luns_changed is true,
from virtio_scsi_handle_cmd_req_prepare() before scsi_req_new().
It will also skip GET_CONFIGURATION and GET_EVENT_STATUS_NOTIFICATION
commands which are further special-cased in 4.1.6.1 of the MMC
specification.
Thanks,
Paolo
> @@ -956,6 +968,7 @@ static void virtio_scsi_hotplug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> virtio_scsi_push_event(s, sd,
> VIRTIO_SCSI_T_TRANSPORT_RESET,
> VIRTIO_SCSI_EVT_RESET_RESCAN);
> +s->reported_luns_changed = true;
> virtio_scsi_release(s);
> }
> }
> @@ -973,6 +986,7 @@ static void virtio_scsi_hotunplug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> virtio_scsi_push_event(s, sd,
> VIRTIO_SCSI_T_TRANSPORT_RESET,
> VIRTIO_SCSI_EVT_RESET_REMOVED);
> +s->reported_luns_changed = true;
> virtio_scsi_release(s);
> }
>
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index a36aad9c8695..efbcf9ba069a 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -81,6 +81,7 @@ struct VirtIOSCSI {
> SCSIBus bus;
> int resetting;
> bool events_dropped;
> +bool reported_luns_changed;
>
> /* Fields for dataplane below */
> AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
>
Re: [PATCH] i386: Add new CPU model SapphireRapids
* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Wed, Sep 21, 2022 at 03:51:42PM +0100, Dr. David Alan Gilbert wrote: > > * Wang, Lei (lei4.w...@intel.com) wrote: > > > The new CPU model mostly inherits features from Icelake-Server, while > > > adding new features: > > > - AMX (Advance Matrix eXtensions) > > > - Bus Lock Debug Exception > > > and new instructions: > > > - AVX VNNI (Vector Neural Network Instruction): > > > - VPDPBUS: Multiply and Add Unsigned and Signed Bytes > > > - VPDPBUSDS: Multiply and Add Unsigned and Signed Bytes with > > > Saturation > > > - VPDPWSSD: Multiply and Add Signed Word Integers > > > - VPDPWSSDS: Multiply and Add Signed Integers with Saturation > > > - FP16: Replicates existing AVX512 computational SP (FP32) instructions > > >using FP16 instead of FP32 for ~2X performance gain > > > - SERIALIZE: Provide software with a simple way to force the processor to > > >complete all modifications, faster, allowed in all privilege levels and > > >not causing an unconditional VM exit > > > - TSX Suspend Load Address Tracking: Allows programmers to choose which > > >memory accesses do not need to be tracked in the TSX read set > > > - AVX512_BF16: Vector Neural Network Instructions supporting BFLOAT16 > > >inputs and conversion instructions from IEEE single precision > > > > > > Features may be added in future versions: > > > - CET (virtualization support hasn't been merged) > > > Instructions may be added in future versions: > > > - fast zero-length MOVSB (KVM doesn't support yet) > > > - fast short STOSB (KVM doesn't support yet) > > > - fast short CMPSB, SCASB (KVM doesn't support yet) > > > > > > Signed-off-by: Wang, Lei > > > Reviewed-by: Robert Hoo > > > > Hi, > >What fills in the AMX tile and tmul information leafs > > (0x1D, 0x1E)? > > In particular, how would we make sure when we migrate between two > > generations of AMX/Tile/Tmul capable devices with different > > register/palette/tmul limits that the migration is tied to the CPU type > > correctly? > > Would you expect all devices called a 'SappireRapids' to have the same > > sizes? > > We shouldn't assume this will only be used on 'SappireRapids' host > silicon. Thi named CPU model is likely to be used by a guest running > on any host silicon generations that follow SappireRapids too. Indeed, but I wanted to check the opposite question first; whether all SappireRapids had the same sizes; I think you're asking the opposite question. Dave > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: QEMU's FreeBSD 13 CI job is failing
On Wed, Sep 21, 2022 at 1:13 AM Daniel P. Berrangé wrote: > On Tue, Sep 20, 2022 at 02:21:46PM -0600, Warner Losh wrote: > > On Tue, Sep 20, 2022 at 2:57 AM Daniel P. Berrangé > > wrote: > > > > > On Tue, Sep 20, 2022 at 10:23:56AM +0200, Thomas Huth wrote: > > > > On 20/09/2022 10.21, Daniel P. Berrangé wrote: > > > > > On Tue, Sep 20, 2022 at 08:44:27AM +0200, Thomas Huth wrote: > > > > > > > > > > > > Seen here for example: > > > > > > > > > > > > https://gitlab.com/qemu-project/qemu/-/jobs/3050165356#L2543 > > > > > > > > > > > > ld-elf.so.1: /lib/libc.so.7: version FBSD_1.7 required by > > > > > > /usr/local/lib/libpython3.9.so.1.0 not found > > > > > > ERROR: Cannot use '/usr/local/bin/python3', Python >= 3.6 is > > > required. > > > > > > > > > > > > ... looks like the Python binary is not working anymore? Does > > > anybody know > > > > > > what happened here? > > > > > > > > > > FreeBSD ports is only guaranteed to work with latest minor release > > > > > base image. The python binary recently started relying on symbols > > > > > in the 13.1 base image, and we're using 13.0. > > > > > > > > > > I updated lcitool last week to pick 13.1, so we just need a refresh > > > > > on the QEMU side to pick this up. > > > > > > > > OK ... Alex, IIRC you have a patch queued to update the files that > are > > > > refreshed by lcitool ... does that already contain the update for > > > FreeBSD, > > > > too? > > > > > > Oh actually, I'm forgetting that QEMU doesn't use the 'lcitool > manifest' > > > command for auto-generating the gitlab-ci.yml file. In QEMU's case just > > > manually edit .gitlab-ci.d/cirrus.yml to change > > > > > > CIRRUS_VM_IMAGE_NAME: freebsd-13-0 > > > > > > > FreeBSD's support policy is that we EOL minor dot releases a few months > > after > > the next minor release is final. Part of that process involves moving the > > build > > of packages to that new minor version (which is what's not guaranteed to > > work > > on older versions... only old binaries on new versions is guaranteed)... > > And that's > > the problem that was hit here. > > It would be nice if something in the ports tool / packages was > able to report the incompatibility at time of install, rather > than leaving a later runtime failed. > Indeed. I've suggested it to the authors... There's some technical issues around this and the package format they need to work out. > > I'll try to submit changes after the next minor release in that 'few > month' > > window > > to update this in the future. In general, doing so would be the best fit > > with FreeBSD's > > support model... It's one of those things I didn't think of at the time, > > but is obvious in > > hindsight. > > Note, we're reliant on Cirrus CI actually publishing the new images > for use. I've not previously checked before how quickly they publish > them after FreeBSD does the upstream release, but anyway I go by what > they list here: > > https://cirrus-ci.org/guide/FreeBSD/ Yea. They have been pretty good in the past about getting new images up quickly after the release. Warner
[PULL 1/5] target/m68k: Implement atomic test-and-set
From: Richard Henderson
This is slightly more complicated than cas,
because tas is allowed on data registers.
Signed-off-by: Richard Henderson
Reviewed-by: Laurent Vivier
Message-Id: <20220829051746.227094-1-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier
---
target/m68k/translate.c | 40 ++--
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 5098f7e570e0..ffcc761d6011 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2825,19 +2825,39 @@ DISAS_INSN(illegal)
gen_exception(s, s->base.pc_next, EXCP_ILLEGAL);
}
-/* ??? This should be atomic. */
DISAS_INSN(tas)
{
-TCGv dest;
-TCGv src1;
-TCGv addr;
+int mode = extract32(insn, 3, 3);
+int reg0 = REG(insn, 0);
-dest = tcg_temp_new();
-SRC_EA(env, src1, OS_BYTE, 1, );
-gen_logic_cc(s, src1, OS_BYTE);
-tcg_gen_ori_i32(dest, src1, 0x80);
-DEST_EA(env, insn, OS_BYTE, dest, );
-tcg_temp_free(dest);
+if (mode == 0) {
+/* data register direct */
+TCGv dest = cpu_dregs[reg0];
+gen_logic_cc(s, dest, OS_BYTE);
+tcg_gen_ori_tl(dest, dest, 0x80);
+} else {
+TCGv src1, addr;
+
+addr = gen_lea_mode(env, s, mode, reg0, OS_BYTE);
+if (IS_NULL_QREG(addr)) {
+gen_addr_fault(s);
+return;
+}
+src1 = tcg_temp_new();
+tcg_gen_atomic_fetch_or_tl(src1, addr, tcg_constant_tl(0x80),
+ IS_USER(s), MO_SB);
+gen_logic_cc(s, src1, OS_BYTE);
+tcg_temp_free(src1);
+
+switch (mode) {
+case 3: /* Indirect postincrement. */
+tcg_gen_addi_i32(AREG(insn, 0), addr, 1);
+break;
+case 4: /* Indirect predecrememnt. */
+tcg_gen_mov_i32(AREG(insn, 0), addr);
+break;
+}
+}
}
DISAS_INSN(mull)
--
2.37.3
[PULL 2/5] target/m68k: Fix MACSR to CCR
From: Richard Henderson
First, we were writing to the entire SR register, instead
of only the flags portion. Second, we were not clearing C
as per the documentation (X was cleared via the 0xf mask).
Signed-off-by: Richard Henderson
Reviewed-by: Laurent Vivier
Message-Id: <20220913142818.7802-2-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier
---
target/m68k/translate.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index ffcc761d6011..c9bb05380323 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -5912,8 +5912,10 @@ DISAS_INSN(from_mext)
DISAS_INSN(macsr_to_ccr)
{
TCGv tmp = tcg_temp_new();
-tcg_gen_andi_i32(tmp, QREG_MACSR, 0xf);
-gen_helper_set_sr(cpu_env, tmp);
+
+/* Note that X and C are always cleared. */
+tcg_gen_andi_i32(tmp, QREG_MACSR, CCF_N | CCF_Z | CCF_V);
+gen_helper_set_ccr(cpu_env, tmp);
tcg_temp_free(tmp);
set_cc_op(s, CC_OP_FLAGS);
}
--
2.37.3
[PULL 3/5] target/m68k: Perform writback before modifying SR
From: Richard Henderson
Writes to SR may change security state, which may involve
a swap of %ssp with %usp as reflected in %a7. Finish the
writeback of %sp@+ before swapping stack pointers.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1206
Signed-off-by: Richard Henderson
Reviewed-by: Laurent Vivier
Reviewed-by: Mark Cave-Ayland
Message-Id: <20220913142818.7802-3-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier
---
target/m68k/translate.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index c9bb05380323..4640eadf78e1 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2285,9 +2285,9 @@ static void gen_set_sr_im(DisasContext *s, uint16_t val,
int ccr_only)
tcg_gen_movi_i32(QREG_CC_N, val & CCF_N ? -1 : 0);
tcg_gen_movi_i32(QREG_CC_X, val & CCF_X ? 1 : 0);
} else {
-TCGv sr = tcg_const_i32(val);
-gen_helper_set_sr(cpu_env, sr);
-tcg_temp_free(sr);
+/* Must writeback before changing security state. */
+do_writebacks(s);
+gen_helper_set_sr(cpu_env, tcg_constant_i32(val));
}
set_cc_op(s, CC_OP_FLAGS);
}
@@ -2297,6 +2297,8 @@ static void gen_set_sr(DisasContext *s, TCGv val, int
ccr_only)
if (ccr_only) {
gen_helper_set_ccr(cpu_env, val);
} else {
+/* Must writeback before changing security state. */
+do_writebacks(s);
gen_helper_set_sr(cpu_env, val);
}
set_cc_op(s, CC_OP_FLAGS);
--
2.37.3
[PATCH v1 08/10] docs/devel: move API to end of tcg-plugins.rst
The API documentation is quite dry and doesn't flow nicely with the rest of the document. Move it to its own section at the bottom along with a little leader text to remind people to update it. Signed-off-by: Alex Bennée --- docs/devel/tcg-plugins.rst | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst index a6fdde01f8..8b40b2a606 100644 --- a/docs/devel/tcg-plugins.rst +++ b/docs/devel/tcg-plugins.rst @@ -110,11 +110,6 @@ details are opaque to plugins. The plugin is able to query select details of instructions and system configuration only through the exported *qemu_plugin* functions. -API -~~~ - -.. kernel-doc:: include/qemu/qemu-plugin.h - Internals - @@ -448,3 +443,13 @@ The plugin has a number of arguments, all of them are optional: associativity of the L2 cache, respectively. Setting any of the L2 configuration arguments implies ``l2=on``. (default: N = 2097152 (2MB), B = 64, A = 16) + +API +--- + +The following API is generated from the inline documentation in +``include/qemu/qemu-plugin.h``. Please ensure any updates to the API +include the full kernel-doc annotations. + +.. kernel-doc:: include/qemu/qemu-plugin.h + -- 2.34.1
[PATCH v1 07/10] docs/devel: clean-up qemu invocations in tcg-plugins
We currently have the final binaries in the root of the build dir so the build prefix is superfluous. Additionally add a shell prompt to be more in line with the rest of the code. Signed-off-by: Alex Bennée --- docs/devel/tcg-plugins.rst | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst index a503d44cee..a6fdde01f8 100644 --- a/docs/devel/tcg-plugins.rst +++ b/docs/devel/tcg-plugins.rst @@ -172,7 +172,7 @@ slightly faster (but not thread safe) counters. Example:: - ./aarch64-linux-user/qemu-aarch64 \ + $ qemu-aarch64 \ -plugin contrib/plugins/libhotblocks.so -d plugin \ ./tests/tcg/aarch64-linux-user/sha1 SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6 @@ -186,7 +186,7 @@ Example:: Similar to hotblocks but this time tracks memory accesses:: - ./aarch64-linux-user/qemu-aarch64 \ + $ qemu-aarch64 \ -plugin contrib/plugins/libhotpages.so -d plugin \ ./tests/tcg/aarch64-linux-user/sha1 SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6 @@ -220,7 +220,7 @@ counted. You can give a value to the ``count`` argument for a class of instructions to break it down fully, so for example to see all the system registers accesses:: - ./aarch64-softmmu/qemu-system-aarch64 $(QEMU_ARGS) \ + $ qemu-system-aarch64 $(QEMU_ARGS) \ -append "root=/dev/sda2 systemd.unit=benchmark.service" \ -smp 4 -plugin ./contrib/plugins/libhowvec.so,count=sreg -d plugin @@ -288,10 +288,10 @@ for the plugin is a path for the socket the two instances will communicate over:: - ./sparc-softmmu/qemu-system-sparc -monitor none -parallel none \ + $ qemu-system-sparc -monitor none -parallel none \ -net none -M SS-20 -m 256 -kernel day11/zImage.elf \ -plugin ./contrib/plugins/liblockstep.so,sockpath=lockstep-sparc.sock \ - -d plugin,nochain +-d plugin,nochain which will eventually report:: @@ -348,7 +348,7 @@ Please be aware that this will generate a lot of output. The plugin needs default argument:: - qemu-system-arm $(QEMU_ARGS) \ + $ qemu-system-arm $(QEMU_ARGS) \ -plugin ./contrib/plugins/libexeclog.so -d plugin which will output an execution trace following this structure:: @@ -365,10 +365,10 @@ which will output an execution trace following this structure:: 0, 0x10c8, 0xfff96c43, "ldr r3, [r0, #0x44]", load, 0x20e4, RAM the output can be filtered to only track certain instructions or -addresses using the `ifilter` or `afilter` options. You can stack the +addresses using the ``ifilter`` or ``afilter`` options. You can stack the arguments if required:: - qemu-system-arm $(QEMU_ARGS) \ + $ qemu-system-arm $(QEMU_ARGS) \ -plugin ./contrib/plugins/libexeclog.so,ifilter=st1w,afilter=0x40001808 -d plugin - contrib/plugins/cache.c @@ -377,7 +377,7 @@ Cache modelling plugin that measures the performance of a given L1 cache configuration, and optionally a unified L2 per-core cache when a given working set is run:: -qemu-x86_64 -plugin ./contrib/plugins/libcache.so \ + $ qemu-x86_64 -plugin ./contrib/plugins/libcache.so \ -d plugin -D cache.log ./tests/tcg/x86_64-linux-user/float_convs will report the following:: -- 2.34.1
[PATCH] qcow2: fix memory leak in qcow2_read_extensions
From: lu zhipeng
Free feature_table if it is failed in bdrv_pread.
Signed-off-by: lu zhipeng
---
block/qcow2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index c6c6692fb7..c8fc3a6160 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -275,6 +275,7 @@ static int qcow2_read_extensions(BlockDriverState *bs,
uint64_t start_offset,
if (ret < 0) {
error_setg_errno(errp, -ret, "ERROR: ext_feature_table: "
"Could not read table");
+g_free(feature_table);
return ret;
}
--
2.31.1
Re: [PATCH v3 1/1] monitor/hmp: print trace as option in help for log command
Dongli Zhang writes:
> Hi Markus,
>
> On 9/17/22 2:44 PM, Philippe Mathieu-Daudé via wrote:
>> Hi Markus,
>>
>> On 2/9/22 14:24, Markus Armbruster wrote:
>>> Dongli Zhang writes:
>>>
The below is printed when printing help information in qemu-system-x86_64
command line, and when CONFIG_TRACE_LOG is enabled:
$ qemu-system-x86_64 -d help
... ...
trace:PATTERN enable trace events
Use "-d trace:help" to get a list of trace events.
However, the options of "trace:PATTERN" are only printed by
"qemu-system-x86_64 -d help", but missing in hmp "help log" command.
Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
Cc: Joe Jin
Signed-off-by: Dongli Zhang
---
Changed since v1:
- change format for "none" as well.
Changed since v2:
- use "log trace:help" in help message.
- add more clarification in commit message.
- add 'Fixes' tag.
---
monitor/hmp.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
>>
>>> Not this patch's fault:
>>>
>>> 1. "-d help" terminates with exit status 1, "-d trace:help" with 0. The
>>> former is wrong.
>
> May I assume it is expected to have exit status 1 when "-d help"?
Non-zero exit status means error. Asking for and receiving help is not
an error. Therefore, "-d help" should exit with status 0, just like
"-help", "-device help", "-machine help", ...
> According to the output of "-d", there is even not a "help" option, but only a
> "-d trace:help" option. That is, "-d help" is not officially supported.
It *is* documented:
$ qemu-system-x86_64 -help | grep -- '^-d '
-d item1,...enable logging of specified items (use '-d help' for a list
of log items)
> The below example use "-d hellworld" but not "help".
>
> # qemu-system-x86_64 -d helloworld
> Log items (comma separated):
> out_asm show generated host assembly code for each compiled TB
> in_asm show target assembly code for each compiled TB
> op show micro ops for each compiled TB
> op_opt show micro ops after optimization
> op_ind show micro ops before indirect lowering
> int show interrupts/exceptions in short format
> execshow trace before each executed TB (lots of logs)
> cpu show CPU registers before entering a TB (lots of logs)
> fpu include FPU registers in the 'cpu' logging
> mmu log MMU-related activities
> pcall x86 only: show protected mode far calls/returns/exceptions
> cpu_reset show CPU state before CPU resets
> unimp log unimplemented functionality
> guest_errorslog when the guest OS does something invalid (eg accessing a
> non-existent register)
> pagedump pages at beginning of user mode emulation
> nochain do not chain compiled TBs so that "exec" and "cpu" show
> complete traces
> plugin output from TCG plugins
>
> strace log every user-mode syscall, its input, and its result
> tid open a separate log file per thread; filename must contain
> '%d'
> trace:PATTERN enable trace events
>
> Use "-d trace:help" to get a list of trace events.
>
>
> According to the source code, the qemu_str_to_log_mask() expects either log
> items or "trace". For any other inputs (e.g., "help" or "helloworld"),
> qemu_str_to_log_mask() returns 0 (no bit set in the mask).
You're right.
>That indicates the
> input (e.g., "help") is not an expected input.
No, it indicates laziness :)
> Therefore, can I assume this is not a bug? I do not think something like below
> is very helpful.
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 263f029a8e..54c8e624bf 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2389,6 +2389,8 @@ static void qemu_process_early_options(void)
> mask = qemu_str_to_log_mask(log_mask);
> if (!mask) {
> qemu_print_log_usage(stdout);
> +if (g_str_equal(log_mask, "help"))
> +exit(0)
> exit(1);
> }
> }
Let's make "-d help" print help to stdout and terminate successfully,
and "-d crap" report an error and terminate unsuccessfully. Just like
other options, such as -device and -machine.
> Thank you very much!
You're welcome!
[PATCH v1 02/10] disas: generalise plugin_printf and use for monitor_disas
Rather than assembling our output piecemeal lets use the same approach
as the plugin disas interface to build the disassembly string before
printing it.
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
---
disas.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/disas.c b/disas.c
index e31438f349..f07b6e760b 100644
--- a/disas.c
+++ b/disas.c
@@ -239,7 +239,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong
code,
}
}
-static int plugin_printf(FILE *stream, const char *fmt, ...)
+static int gstring_printf(FILE *stream, const char *fmt, ...)
{
/* We abuse the FILE parameter to pass a GString. */
GString *s = (GString *)stream;
@@ -270,7 +270,7 @@ char *plugin_disas(CPUState *cpu, uint64_t addr, size_t
size)
GString *ds = g_string_new(NULL);
initialize_debug_target(, cpu);
-s.info.fprintf_func = plugin_printf;
+s.info.fprintf_func = gstring_printf;
s.info.stream = (FILE *)ds; /* abuse this slot */
s.info.buffer_vma = addr;
s.info.buffer_length = size;
@@ -358,15 +358,19 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
{
int count, i;
CPUDebug s;
+g_autoptr(GString) ds = g_string_new("");
initialize_debug_target(, cpu);
-s.info.fprintf_func = qemu_fprintf;
+s.info.fprintf_func = gstring_printf;
+s.info.stream = (FILE *)ds; /* abuse this slot */
+
if (is_physical) {
s.info.read_memory_func = physical_read_memory;
}
s.info.buffer_vma = pc;
if (s.info.cap_arch >= 0 && cap_disas_monitor(, pc, nb_insn)) {
+monitor_puts(mon, ds->str);
return;
}
@@ -376,13 +380,16 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
return;
}
-for(i = 0; i < nb_insn; i++) {
- monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
+for (i = 0; i < nb_insn; i++) {
+g_string_append_printf(ds, "0x" TARGET_FMT_lx ": ", pc);
count = s.info.print_insn(pc, );
- monitor_printf(mon, "\n");
- if (count < 0)
- break;
+g_string_append_c(ds, '\n');
+if (count < 0) {
+break;
+}
pc += count;
}
+
+monitor_puts(mon, ds->str);
}
#endif
--
2.34.1
[PATCH] Revert "intel_iommu: Fix irqchip / X2APIC configuration checks"
It's true that when vcpus<=255 we don't require the length of 32bit APIC
IDs. However here since we already have EIM=ON it means the hypervisor
will declare the VM as x2apic supported (e.g. VT-d ECAP register will have
EIM bit 4 set), so the guest should assume the APIC IDs are 32bits width
even if vcpus<=255. In short, commit 77250171bdc breaks any simple cmdline
that wants to boot a VM with >=9 but <=255 vcpus with:
-device intel-iommu,intremap=on
For anyone who does not want to enable x2apic, we can use eim=off in the
intel-iommu parameters to skip enabling KVM x2apic.
This partly reverts commit 77250171bdc02aee106083fd2a068147befa1a38, while
keeping the valid bit on checking split irqchip, but revert the other change.
Cc: David Woodhouse
Cc: Claudio Fontana
Cc: Igor Mammedov
Signed-off-by: Peter Xu
---
hw/i386/intel_iommu.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 05d53a1aa9..6524c2ee32 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3818,6 +3818,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error
**errp)
error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split");
return false;
}
+if (!kvm_enable_x2apic()) {
+error_setg(errp, "eim=on requires support on the KVM side"
+ "(X2APIC_API, first shipped in v4.7)");
+return false;
+}
}
/* Currently only address widths supported are 39 and 48 bits */
--
2.32.0
Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled
* Bin Meng (bmeng...@gmail.com) wrote:
> From: Xuzhou Cheng
>
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
>
> Signed-off-by: Xuzhou Cheng
> Signed-off-by: Bin Meng
> Reviewed-by: Marc-André Lureau
Hmm you might want to put a small usleep in that loop; otherwise
it'll burn CPU.
There is a slim risk with this that another, entirely unrelated, process
will start up with the same PID between the end of migrate_cancel
and then you'll be spinning on it rather than the 'to' qemu.
I wonder if there's a better way to check for it dieing; e.g. an error
on it's qmp interface or something?
Dave
> ---
>
> Changes in v2:
> - Change to a busy wait after migration is canceled
>
> tests/qtest/migration-test.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index c87afad9e8..aedd9ddb72 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2133,6 +2133,10 @@ static void test_multifd_tcp_cancel(void)
> wait_for_migration_pass(from);
>
> migrate_cancel(from);
> +/* Make sure QEMU process "to" exited */
> +while (qtest_probe_child(to)) {
> +;
> +}
>
> args = (MigrateStart){
> .only_target = true,
> --
> 2.34.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PATCH v1 09/10] contrib/plugins: reset skip when matching in execlog
The purpose of the matches was to only track the execution of instructions we care about. Without resetting skip to the value at the start of the block we end up dumping all instructions after the match with the consequent load on the instrumentation. Signed-off-by: Alex Bennée Cc: Alexandre Iooss --- contrib/plugins/execlog.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c index e659ac9cbb..b5360f2c8e 100644 --- a/contrib/plugins/execlog.c +++ b/contrib/plugins/execlog.c @@ -147,6 +147,9 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb) /* Register callback on instruction */ qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, output); + +/* reset skip */ +skip = (imatches || amatches) ? true : false; } } -- 2.34.1
Re: [PATCH v2 35/39] tests/qtest: migration-test: Skip running some TLS cases for win32
* Bin Meng (bmeng...@gmail.com) wrote:
> From: Bin Meng
>
> Some migration test cases use TLS to communicate, but they fail on
> Windows with the following error messages:
>
> qemu-system-x86_64: TLS handshake failed: Insufficient credentials for that
> request.
> qemu-system-x86_64: TLS handshake failed: Error in the pull function.
> query-migrate shows failed migration: TLS handshake failed: Error in the
> pull function.
>
> Disable them temporarily.
>
> Signed-off-by: Bin Meng
> ---
> I am not familar with the gnutls and simply enabling the gnutls debug
> output does not give me an immedidate hint on why it's failing on
> Windows. Disable these cases for now until someone or maintainers
> who may want to test this on Windows.
Copying in Dan Berrange, he's our expert on weird TLS failures.
Dave
>
> (no changes since v1)
>
> tests/qtest/migration-test.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index aedd9ddb72..dbee9b528a 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1403,6 +1403,7 @@ static void test_precopy_unix_dirty_ring(void)
> }
>
> #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
> static void test_precopy_unix_tls_psk(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -1415,6 +1416,7 @@ static void test_precopy_unix_tls_psk(void)
>
> test_precopy_common();
> }
> +#endif /* _WIN32 */
>
> #ifdef CONFIG_TASN1
> static void test_precopy_unix_tls_x509_default_host(void)
> @@ -1523,6 +1525,7 @@ static void test_precopy_tcp_plain(void)
> }
>
> #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
> static void test_precopy_tcp_tls_psk_match(void)
> {
> MigrateCommon args = {
> @@ -1533,6 +1536,7 @@ static void test_precopy_tcp_tls_psk_match(void)
>
> test_precopy_common();
> }
> +#endif /* _WIN32 */
>
> static void test_precopy_tcp_tls_psk_mismatch(void)
> {
> @@ -1930,6 +1934,7 @@ static void test_multifd_tcp_zstd(void)
> #endif
>
> #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
> static void *
> test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> QTestState *to)
> @@ -1937,6 +1942,7 @@ test_migrate_multifd_tcp_tls_psk_start_match(QTestState
> *from,
> test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
> return test_migrate_tls_psk_start_match(from, to);
> }
> +#endif /* _WIN32 */
>
> static void *
> test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
> @@ -1988,6 +1994,7 @@
> test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
> }
> #endif /* CONFIG_TASN1 */
>
> +#ifndef _WIN32
> static void test_multifd_tcp_tls_psk_match(void)
> {
> MigrateCommon args = {
> @@ -1997,6 +2004,7 @@ static void test_multifd_tcp_tls_psk_match(void)
> };
> test_precopy_common();
> }
> +#endif /* _WIN32 */
>
> static void test_multifd_tcp_tls_psk_mismatch(void)
> {
> @@ -2497,8 +2505,10 @@ int main(int argc, char **argv)
> qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
> qtest_add_func("/migration/precopy/unix/xbzrle",
> test_precopy_unix_xbzrle);
> #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
> qtest_add_func("/migration/precopy/unix/tls/psk",
> test_precopy_unix_tls_psk);
> +#endif
>
> if (has_uffd) {
> /*
> @@ -2524,8 +2534,10 @@ int main(int argc, char **argv)
>
> qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
> #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
> qtest_add_func("/migration/precopy/tcp/tls/psk/match",
> test_precopy_tcp_tls_psk_match);
> +#endif
> qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
> test_precopy_tcp_tls_psk_mismatch);
> #ifdef CONFIG_TASN1
> @@ -2569,8 +2581,10 @@ int main(int argc, char **argv)
> test_multifd_tcp_zstd);
> #endif
> #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
> qtest_add_func("/migration/multifd/tcp/tls/psk/match",
> test_multifd_tcp_tls_psk_match);
> +#endif
> qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch",
> test_multifd_tcp_tls_psk_mismatch);
> #ifdef CONFIG_TASN1
> --
> 2.34.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v2 00/23] target/i386: pc-relative translation blocks
Looks good! Just a couple weird parts of the architecture where I need
some more explanation.
Paolo
On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson
wrote:
>
> This is the x86 specific changes required to reduce the
> amount of translation for address space randomization.
> This is a re-base, with no other significant changes over v1.
>
>
> r~
>
>
> Based-on: 20220906091126.298041-1-richard.hender...@linaro.org
> ("[PATCH v4 0/7] tcg: pc-relative translation blocks")
>
> branch: https://gitlab.com/rth7680/qemu/-/tree/tgt-x86-pcrel
>
>
> Richard Henderson (23):
> target/i386: Remove pc_start
> target/i386: Return bool from disas_insn
> target/i386: Remove cur_eip argument to gen_exception
> target/i386: Remove cur_eip, next_eip arguments to gen_interrupt
> target/i386: Create gen_update_eip_cur
> target/i386: Create gen_update_eip_next
> target/i386: Introduce DISAS_EOB*
> target/i386: Use DISAS_EOB* in gen_movl_seg_T0
> target/i386: Use DISAS_EOB_NEXT
> target/i386: USe DISAS_EOB_ONLY
> target/i386: Create cur_insn_len, cur_insn_len_i32
> target/i386: Remove cur_eip, next_eip arguments to gen_repz*
> target/i386: Introduce DISAS_JUMP
> target/i386: Truncate values for lcall_real to i32
> target/i386: Create eip_next_*
> target/i386: Use DISAS_TOO_MANY to exit after gen_io_start
> target/i386: Create gen_jmp_rel
> target/i386: Use gen_jmp_rel for loop and jecxz insns
> target/i386: Use gen_jmp_rel for gen_jcc
> target/i386: Use gen_jmp_rel for gen_repz*
> target/i386: Use gen_jmp_rel for DISAS_TOO_MANY
> target/i386: Create gen_eip_cur
> target/i386: Enable TARGET_TB_PCREL
>
> target/i386/cpu-param.h | 1 +
> target/i386/helper.h | 2 +-
> target/i386/tcg/seg_helper.c | 6 +-
> target/i386/tcg/tcg-cpu.c| 8 +-
> target/i386/tcg/translate.c | 712 ++-
> 5 files changed, 369 insertions(+), 360 deletions(-)
>
> --
> 2.34.1
>
[PATCH] add keepalive for qemu-nbd
From: songlinfeng
we want to export a image with qemu-nbd as server, in client we use libnbd to
connect qemu-nbd,but when client power down,the server is still working.
qemu-nbd will exit when last client exit.so,we still want server exit when
client power down.maybe qmp can handle it,but i don't know how to do .
Signed-off-by: songlinfeng
---
qemu-nbd.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 0cd5aa6..115ef2b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,7 +20,8 @@
#include
#include
#include
-
+#include
+#include
#include "qemu/help-texts.h"
#include "qapi/error.h"
#include "qemu/cutils.h"
@@ -365,6 +366,26 @@ static void nbd_accept(QIONetListener *listener,
QIOChannelSocket *cioc,
nb_fds++;
nbd_update_server_watch();
nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed);
+int tcp_keepalive_intvl = 5;
+int tcp_keepalive_probes = 5;
+int tcp_keepalive_time = 60;
+int tcp_keepalive_on = 1;
+if (setsockopt(cioc->fd, SOL_TCP, TCP_KEEPINTVL,
+ _keepalive_intvl, sizeof(tcp_keepalive_intvl)) < 0) {
+perror("setsockopt failed\n");
+}
+if (setsockopt(cioc->fd, SOL_TCP, TCP_KEEPCNT,
+ _keepalive_probes, sizeof(tcp_keepalive_probes)) < 0) {
+perror("setsockopt failed\n");
+}
+if (setsockopt(cioc->fd, SOL_TCP, TCP_KEEPIDLE,
+ _keepalive_time, sizeof(tcp_keepalive_time)) < 0) {
+perror("setsockopt failed\n");
+}
+if (setsockopt(cios->fd, SOL_SOCKET, SO_KEEPALIVE,
+ _keepalive_on, sizeof(tcp_keepalive_on)) < 0) {
+perror("setsockopt failed\n");
+}
}
static void nbd_update_server_watch(void)
--
1.8.3.1
Re: [PATCH RFC 0/7] hostmem: NUMA-aware memory preallocation using ThreadContext
On 7/21/22 14:07, David Hildenbrand wrote: > Ping? Is there any plan how to move forward? I have libvirt patches ready to consume this and I'd like to prune my old local branches :-) Michal
Re: [PATCH] i386: Add new CPU model SapphireRapids
On Wed, Sep 21, 2022 at 03:51:42PM +0100, Dr. David Alan Gilbert wrote: > * Wang, Lei (lei4.w...@intel.com) wrote: > > The new CPU model mostly inherits features from Icelake-Server, while > > adding new features: > > - AMX (Advance Matrix eXtensions) > > - Bus Lock Debug Exception > > and new instructions: > > - AVX VNNI (Vector Neural Network Instruction): > > - VPDPBUS: Multiply and Add Unsigned and Signed Bytes > > - VPDPBUSDS: Multiply and Add Unsigned and Signed Bytes with Saturation > > - VPDPWSSD: Multiply and Add Signed Word Integers > > - VPDPWSSDS: Multiply and Add Signed Integers with Saturation > > - FP16: Replicates existing AVX512 computational SP (FP32) instructions > >using FP16 instead of FP32 for ~2X performance gain > > - SERIALIZE: Provide software with a simple way to force the processor to > >complete all modifications, faster, allowed in all privilege levels and > >not causing an unconditional VM exit > > - TSX Suspend Load Address Tracking: Allows programmers to choose which > >memory accesses do not need to be tracked in the TSX read set > > - AVX512_BF16: Vector Neural Network Instructions supporting BFLOAT16 > >inputs and conversion instructions from IEEE single precision > > > > Features may be added in future versions: > > - CET (virtualization support hasn't been merged) > > Instructions may be added in future versions: > > - fast zero-length MOVSB (KVM doesn't support yet) > > - fast short STOSB (KVM doesn't support yet) > > - fast short CMPSB, SCASB (KVM doesn't support yet) > > > > Signed-off-by: Wang, Lei > > Reviewed-by: Robert Hoo > > Hi, >What fills in the AMX tile and tmul information leafs > (0x1D, 0x1E)? > In particular, how would we make sure when we migrate between two > generations of AMX/Tile/Tmul capable devices with different > register/palette/tmul limits that the migration is tied to the CPU type > correctly? > Would you expect all devices called a 'SappireRapids' to have the same > sizes? We shouldn't assume this will only be used on 'SappireRapids' host silicon. Thi named CPU model is likely to be used by a guest running on any host silicon generations that follow SappireRapids too. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 3/4] tcg/plugins: Support for inter-plugin interactions
Andrew Fasano writes:
> Expand tcg-plugin system to allow for plugins to export functions
> and callbacks that can be used by other plugins. Exported functions
> can be called at runtime by other loaded plugins. Loaded plugins
> can register functions with exported callbacks and have these
> functions run whenever the callback is triggered.
Could you please split the callback and function handling in future
patches to aid review.
>
> Signed-off-by: Andrew Fasano
> ---
> include/qemu/plugin-qpp.h| 252 +++
> plugins/core.c | 11 ++
> plugins/loader.c | 24
> plugins/plugin.h | 5 +
> plugins/qemu-plugins.symbols | 1 +
> 5 files changed, 293 insertions(+)
> create mode 100644 include/qemu/plugin-qpp.h
>
> diff --git a/include/qemu/plugin-qpp.h b/include/qemu/plugin-qpp.h
> new file mode 100644
> index 00..befa4f9b8b
> --- /dev/null
> +++ b/include/qemu/plugin-qpp.h
> @@ -0,0 +1,252 @@
> +#ifndef PLUGIN_QPP_H
> +#define PLUGIN_QPP_H
> +
> +/*
> + * Facilities for "QEMU plugin to plugin" (QPP) interactions between tcg
> + * plugins. These allow for an inter-plugin callback system as well
> + * as direct function calls between loaded plugins. For more details see
> + * docs/devel/plugin.rst.
> + */
> +
> +#include
> +#include
> +#include
> +extern GModule * qemu_plugin_name_to_handle(const char *);
Plugin API functions should have /** */ kernel-doc annotations for the
benefit of the autogenerated API docs. Moreover handles to plugins are
usually anonymous pointer types to discourage them fishing about in the
contents.
The fact we expose GModule to the plugin to do the linking makes me
think that maybe the linking should be pushed into loader and be done on
behalf of the plugin.
> +
> +#define PLUGIN_CONCAT(x, y) _PLUGIN_CONCAT(x, y)
> +#define _PLUGIN_CONCAT(x, y) x##y
> +#define QPP_NAME(plugin, fn) PLUGIN_CONCAT(plugin, PLUGIN_CONCAT(_, fn))
> +#define QPP_MAX_CB 256
> +#define _QPP_SETUP_NAME(plugin, fn) PLUGIN_CONCAT(_qpp_setup_, \
> +QPP_NAME(plugin, fn))
> +
> +/*
> + **
> + * The QPP_CREATE_CB and QPP_RUN_CB macros are to be used by a plugin that
> + * wishs to create and later trigger QPP-based callback events. These are
> + * events that the plugin can detect (i.e., through analysis of guest state)
> + * that may be of interest to other plugins.
> + **
> + */
> +
> +/*
> + * QPP_CREATE_CB(name) will create a callback by defining necessary internal
> + * functions and variables based off the provided name. It must be run before
> + * triggering the callback event (with QPP_RUN_CB). This macro will create
> the
> + * following variables and functions, based off the provided name:
> + *
> + * 1) qpp_[name]_cb is an array of function pointers storing the
> + *registered callbacks.
> + * 2) qpp_[name]_num_cb stores the number of functions stored with this
> + *callback.
> + * 3) qpp_add_cb_[name] is a function to add a pointer into the qpp_[name]_cb
> + *array and increment qpp_[name]_num_cb.
> + * 4) qpp_remove_cb_[name] finds a registered callback, deletes it,
> decrements
> + *_num_cb and shifts the _cb array appropriately to fill the gap.
> + *
> + * Important notes:
> + *
> + * 1) Multiple callbacks can be registered for the same event, however
> consumers
> + *can not control the order in which they are called and this order may
> + *change in the future.
> + *
> + * 2) If this macro is incorrectly used multiple times in the same plugin
> with
> + *the same callback name set, it will raise a compilation error since
> + *these variables would then be defined multiple times. The same callback
> + *name can, however, be created in distrinct plugins without issue.
> + */
> +#define QPP_CREATE_CB(cb_name) \
> +void qpp_add_cb_##cb_name(cb_name##_t fptr);\
> +bool qpp_remove_cb_##cb_name(cb_name##_t fptr); \
> +cb_name##_t * qpp_##cb_name##_cb[QPP_MAX_CB]; \
> +int qpp_##cb_name##_num_cb; \
> +\
> +void qpp_add_cb_##cb_name(cb_name##_t fptr) \
> +{ \
> + assert(qpp_##cb_name##_num_cb < QPP_MAX_CB); \
> + qpp_##cb_name##_cb[qpp_##cb_name##_num_cb] = fptr;\
> + qpp_##cb_name##_num_cb += 1; \
> +} \
> +\
> +bool qpp_remove_cb_##cb_name(cb_name##_t fptr) \
> +{ \
> + int i = 0;
[PULL 0/5] M68k for 7.2 patches
The following changes since commit 832e9e33bc51f52fc3ea667d48912e95af3e28f3: Merge tag 'pull-loongarch-20220920' of https://gitlab.com/gaosong/qemu into staging (2022-09-20 14:17:03 -0400) are available in the Git repository at: https://github.com/vivier/qemu-m68k.git tags/m68k-for-7.2-pull-request for you to fetch changes up to c7546abfaa1b1c2729eaddd41c6268a73cdae14f: target/m68k: always call gen_exit_tb() after writes to SR (2022-09-21 15:10:57 +0200) m68k pull request 20220921 - several fixes for SR - implement TAS - feature cleanup Mark Cave-Ayland (2): target/m68k: rename M68K_FEATURE_M68000 to M68K_FEATURE_M68K target/m68k: always call gen_exit_tb() after writes to SR Richard Henderson (3): target/m68k: Implement atomic test-and-set target/m68k: Fix MACSR to CCR target/m68k: Perform writback before modifying SR target/m68k/cpu.h | 5 +- target/m68k/cpu.c | 2 +- target/m68k/helper.c| 2 +- target/m68k/op_helper.c | 2 +- target/m68k/translate.c | 196 +++- 5 files changed, 118 insertions(+), 89 deletions(-) -- 2.37.3
Re: [PATCH v1 01/10] monitor: expose monitor_puts to rest of code
On 21/9/22 18:07, Alex Bennée wrote: This helps us construct strings elsewhere before echoing to the monitor. It avoids having to jump through hoops like: monitor_printf(mon, "%s", s->str); It will be useful in following patches but for now convert all existing plain "%s" printfs to use the _puts api. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson --- v2 - s/monitor_printf(mon, "%s"/monitor_puts(mon, / --- docs/devel/writing-monitor-commands.rst | 2 +- include/monitor/monitor.h | 1 + monitor/monitor-internal.h | 1 - block/monitor/block-hmp-cmds.c | 10 +- hw/misc/mos6522.c | 2 +- monitor/hmp-cmds.c | 8 monitor/hmp.c | 2 +- target/i386/helper.c| 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PULL 00/21] Misc patches for 2022-09-19
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v1 06/10] plugins: Assert mmu_idx in range before use in qemu_plugin_get_hwaddr
On 21/9/22 18:07, Alex Bennée wrote: From: Richard Henderson Coverity reports out-of-bound accesses here. This should be a false positive due to how the index is decoded from MemOpIdx. Fixes: Coverity CID 1487201 Signed-off-by: Richard Henderson Reviewed-by: Damien Hedde Message-Id: <20220401190233.329360-1-richard.hender...@linaro.org> Signed-off-by: Alex Bennée --- plugins/api.c | 2 ++ 1 file changed, 2 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 0/1] hw/display: expose linear framebuffer address in Bochs VBE registers
On 9/21/22 09:14, Gerd Hoffmann wrote: Nope. Even if you fix the framebuffer address conflict you still have the io address conflict. Yeah, that is why I explicitly said that this is needed to be fixed as well in later patches. Yep. That's why isa-pc is pretty much unused these days. Well, I can't say I use it frequently, but I do test it with the SerenityOS kernel and it works pretty well. The SerenityOS kernel is able to drive an isa-vga device just fine after my patches were merged yesterday (in the GitHub pull request I provided a link to), so I do see that machine type as a valuable test platform. When you want build a sysbus variant of the bochs-display device and make that discoverable by the guest somehow (dt or acpi) you can expose both io ports and framebuffer address that way. No need to touch the bochs dispi interface for that. This is an interesting idea. A sysbus-bochs-display device might work well as you suggested, instead of using an isa-vga device. This idea is quite neat in my opinion, because it could speed up the boot of such VM while still providing sufficient display capabilities for those we need them. It could help developers to test their OSes on such hardware setups to ensure multi-monitor configuration works reliably when there's no PCI bus at all but many framebuffer devices being used in one VM. Why not just use virtio-gpu? Trying to run this command: qemu-system-x86_64 -M microvm -m 2048 -device virtio-gpu Results in: qemu-system-x86_64: -device virtio-gpu: No 'PCI' bus found for device 'virtio-gpu-pci' The idea was to not use PCI at all but still to have multiple framebuffer devices. So clearly virtio-gpu is not usable in this situation. 2. This is more related to the SerenityOS project - I prefer to not hardcode physical addresses at all wherever I can do so. This makes the code cleaner and more understandable as far as I observe this from the angle of the person which is not me, that tries to make sense from the code flow. Yea, fully agree, but why continue to use non-discoverable isa bus devices then? On the ISA-PC machine, I still want to be able to boot into a graphical environment with the SerenityOS kernel. The only option is to use the isa-vga device, which works OK. On the microvm machine, it is really not that important if I use the isa-vga device or a sysbus-bochs-display device (when I implement that device). I just want to support as many x86 platform configurations as possible - modern non-PCI machines, ISA-PC machines and regular i440fx/Q35 machines. 3. The costs of adding this feature are pretty negligible compared to the possible value of this, especially if we apply the idea of running multiple ISA-VGA devices on one microvm machine. Still, the only major "issue" that one can point to is the fact that I bump up the Bochs VBE version number, which could be questionable with how the feature might be insignificant for many guest OSes out there. Touching isa-vga at this point doesn't make sense at all. We simply can't move around the framebuffer without screwing up users. That's an issue I didn't consider, but this is not a real problem on the microvm machine if you use the device tree approach to expose the resources of the device, which in some sense makes it unnecessary to use the bochs dispi interface to expose the framebuffer physical address. Also I don't see any actual value in this. Even considering the multiple devices case the patch is a partial solution only (handles the framebuffer but not the ioports) which is pointless. Considering the possibility of exposing the framebuffer address within the device tree blob, it is indeed not making more value to go with this approach. I'm still fond of the idea to create a sysbus variant of the bochs-display device, so I might work on that instead :) Best regards, Liav
Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically
chenh writes:
> From: Hao Chen
>
> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> start the virtual machine through libvirt or qemu, but now, the libvirt or
> qemu can call dpdk vdpa vendor driver's ops .get_config through
> vhost_net_get_config
> to get the mac address of the vdpa hardware without manual configuration.
>
> v1->v2:
> Only copy ETH_ALEN data of netcfg for some vdpa device such as
> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> We only need the mac address and don't care about the status field.
>
> Signed-off-by: Hao Chen
> ---
> hw/block/vhost-user-blk.c | 1 -
> hw/net/virtio-net.c | 7 +++
> hw/virtio/vhost-user.c| 19 ---
> 3 files changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9117222456..5dca4eab09 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
> **errp)
>
> vhost_dev_set_config_notifier(>dev, _ops);
>
> -s->vhost_user.supports_config = true;
NACK from me. The supports_config flag is there for a reason.
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index bd24741be8..8b01078249 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev
> *dev, void *opaque,
> }
>
> if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> -bool supports_f_config = vus->supports_config ||
> -(dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> uint64_t protocol_features;
>
> dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev
> *dev, void *opaque,
> */
> protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>
> -if (supports_f_config) {
> -if (!virtio_has_feature(protocol_features,
> -VHOST_USER_PROTOCOL_F_CONFIG)) {
> -error_setg(errp, "vhost-user device expecting "
> - "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user
> backend does "
> - "not support it.");
> -return -EPROTO;
> -}
> -} else {
> -if (virtio_has_feature(protocol_features,
> - VHOST_USER_PROTOCOL_F_CONFIG)) {
> -warn_reportf_err(*errp, "vhost-user backend supports "
> - "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does
> not.");
> -protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> -}
> -}
> -
> /* final set of protocol features */
> dev->protocol_features = protocol_features;
> err = vhost_user_set_protocol_features(dev, dev->protocol_features);
--
Alex Bennée
[PULL v1 1/1] hw/microblaze: pass random seed to fdt
From: "Jason A. Donenfeld"
If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
initialize early. Set this using the usual guest random number
generation function. This FDT node is part of the DT specification.
Reviewed-by: Edgar E. Iglesias
Signed-off-by: Jason A. Donenfeld
Signed-off-by: Edgar E. Iglesias
---
hw/microblaze/boot.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 8b92a9801a..25ad54754e 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -30,6 +30,7 @@
#include "qemu/option.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
+#include "qemu/guest-random.h"
#include "sysemu/device_tree.h"
#include "sysemu/reset.h"
#include "hw/boards.h"
@@ -75,6 +76,7 @@ static int microblaze_load_dtb(hwaddr addr,
int fdt_size;
void *fdt = NULL;
int r;
+uint8_t rng_seed[32];
if (dtb_filename) {
fdt = load_device_tree(dtb_filename, _size);
@@ -83,6 +85,9 @@ static int microblaze_load_dtb(hwaddr addr,
return 0;
}
+qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
+qemu_fdt_setprop(fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
+
if (kernel_cmdline) {
r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
kernel_cmdline);
--
2.25.1
Re: [PATCH v2 35/39] tests/qtest: migration-test: Skip running some TLS cases for win32
On Wed, Sep 21, 2022 at 05:51:33PM +0100, Dr. David Alan Gilbert wrote:
> * Bin Meng (bmeng...@gmail.com) wrote:
> > From: Bin Meng
> >
> > Some migration test cases use TLS to communicate, but they fail on
> > Windows with the following error messages:
> >
> > qemu-system-x86_64: TLS handshake failed: Insufficient credentials for
> > that request.
> > qemu-system-x86_64: TLS handshake failed: Error in the pull function.
> > query-migrate shows failed migration: TLS handshake failed: Error in the
> > pull function.
> >
> > Disable them temporarily.
> >
> > Signed-off-by: Bin Meng
> > ---
> > I am not familar with the gnutls and simply enabling the gnutls debug
> > output does not give me an immedidate hint on why it's failing on
> > Windows. Disable these cases for now until someone or maintainers
> > who may want to test this on Windows.
>
> Copying in Dan Berrange, he's our expert on weird TLS failures.
Seems to match this:
https://gnutls.org/faq.html#key-usage-violation2
which suggests we have a configuration mis-match.
I'm surprised to see you are only needing to disable the TLS PSK tests,
not the TLS x509 tests.
I'd like to know if tests/unit/test-crypto-tlssession passes.
If so, it might suggest we are missing 'priority: NORMAL' property
when configuring TLS creds for the migration test.
> > (no changes since v1)
> >
> > tests/qtest/migration-test.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index aedd9ddb72..dbee9b528a 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -1403,6 +1403,7 @@ static void test_precopy_unix_dirty_ring(void)
> > }
> >
> > #ifdef CONFIG_GNUTLS
> > +#ifndef _WIN32
> > static void test_precopy_unix_tls_psk(void)
> > {
> > g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > @@ -1415,6 +1416,7 @@ static void test_precopy_unix_tls_psk(void)
> >
> > test_precopy_common();
> > }
> > +#endif /* _WIN32 */
> >
> > #ifdef CONFIG_TASN1
> > static void test_precopy_unix_tls_x509_default_host(void)
> > @@ -1523,6 +1525,7 @@ static void test_precopy_tcp_plain(void)
> > }
> >
> > #ifdef CONFIG_GNUTLS
> > +#ifndef _WIN32
> > static void test_precopy_tcp_tls_psk_match(void)
> > {
> > MigrateCommon args = {
> > @@ -1533,6 +1536,7 @@ static void test_precopy_tcp_tls_psk_match(void)
> >
> > test_precopy_common();
> > }
> > +#endif /* _WIN32 */
> >
> > static void test_precopy_tcp_tls_psk_mismatch(void)
> > {
> > @@ -1930,6 +1934,7 @@ static void test_multifd_tcp_zstd(void)
> > #endif
> >
> > #ifdef CONFIG_GNUTLS
> > +#ifndef _WIN32
> > static void *
> > test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > QTestState *to)
> > @@ -1937,6 +1942,7 @@
> > test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
> > return test_migrate_tls_psk_start_match(from, to);
> > }
> > +#endif /* _WIN32 */
> >
> > static void *
> > test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
> > @@ -1988,6 +1994,7 @@
> > test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
> > }
> > #endif /* CONFIG_TASN1 */
> >
> > +#ifndef _WIN32
> > static void test_multifd_tcp_tls_psk_match(void)
> > {
> > MigrateCommon args = {
> > @@ -1997,6 +2004,7 @@ static void test_multifd_tcp_tls_psk_match(void)
> > };
> > test_precopy_common();
> > }
> > +#endif /* _WIN32 */
> >
> > static void test_multifd_tcp_tls_psk_mismatch(void)
> > {
> > @@ -2497,8 +2505,10 @@ int main(int argc, char **argv)
> > qtest_add_func("/migration/precopy/unix/plain",
> > test_precopy_unix_plain);
> > qtest_add_func("/migration/precopy/unix/xbzrle",
> > test_precopy_unix_xbzrle);
> > #ifdef CONFIG_GNUTLS
> > +#ifndef _WIN32
> > qtest_add_func("/migration/precopy/unix/tls/psk",
> > test_precopy_unix_tls_psk);
> > +#endif
> >
> > if (has_uffd) {
> > /*
> > @@ -2524,8 +2534,10 @@ int main(int argc, char **argv)
> >
> > qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
> > #ifdef CONFIG_GNUTLS
> > +#ifndef _WIN32
> > qtest_add_func("/migration/precopy/tcp/tls/psk/match",
> > test_precopy_tcp_tls_psk_match);
> > +#endif
> > qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
> > test_precopy_tcp_tls_psk_mismatch);
> > #ifdef CONFIG_TASN1
> > @@ -2569,8 +2581,10 @@ int main(int argc, char **argv)
> > test_multifd_tcp_zstd);
> > #endif
> > #ifdef CONFIG_GNUTLS
> > +#ifndef _WIN32
> > qtest_add_func("/migration/multifd/tcp/tls/psk/match",
> > test_multifd_tcp_tls_psk_match);
> > +#endif
> >
Re: [PATCH v1 08/10] docs/devel: move API to end of tcg-plugins.rst
On 21/9/22 18:07, Alex Bennée wrote: The API documentation is quite dry and doesn't flow nicely with the rest of the document. Move it to its own section at the bottom along with a little leader text to remind people to update it. Signed-off-by: Alex Bennée --- docs/devel/tcg-plugins.rst | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v1 07/10] docs/devel: clean-up qemu invocations in tcg-plugins
On 21/9/22 18:07, Alex Bennée wrote: We currently have the final binaries in the root of the build dir so the build prefix is superfluous. Additionally add a shell prompt to be more in line with the rest of the code. Signed-off-by: Alex Bennée --- docs/devel/tcg-plugins.rst | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 1/7] accel/tcg: Use bool for page_find_alloc
Richard Henderson writes: > Bool is more appropriate type for the alloc parameter. > > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée
Re: [PATCH v4 2/7] accel/tcg: Use DisasContextBase in plugin_gen_tb_start
Richard Henderson writes: > Use the pc coming from db->pc_first rather than the TB. > > Use the cached host_addr rather than re-computing for the > first page. We still need a separate lookup for the second > page because it won't be computed for DisasContextBase until > the translator actually performs a read from the page. > > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée
Re: [PATCH RESEND] hw/microblaze: pass random seed to fdt
On Wed, Sep 21, 2022 at 12:32:37PM +0200, Jason A. Donenfeld wrote:
> On Thu, Sep 8, 2022 at 11:40 AM Jason A. Donenfeld wrote:
> >
> > If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
> > initialize early. Set this using the usual guest random number
> > generation function. This FDT node is part of the DT specification.
> >
> > Reviewed-by: Edgar E. Iglesias
> > Signed-off-by: Jason A. Donenfeld
> > ---
> > hw/microblaze/boot.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> > index 8b92a9801a..25ad54754e 100644
> > --- a/hw/microblaze/boot.c
> > +++ b/hw/microblaze/boot.c
> > @@ -30,6 +30,7 @@
> > #include "qemu/option.h"
> > #include "qemu/config-file.h"
> > #include "qemu/error-report.h"
> > +#include "qemu/guest-random.h"
> > #include "sysemu/device_tree.h"
> > #include "sysemu/reset.h"
> > #include "hw/boards.h"
> > @@ -75,6 +76,7 @@ static int microblaze_load_dtb(hwaddr addr,
> > int fdt_size;
> > void *fdt = NULL;
> > int r;
> > +uint8_t rng_seed[32];
> >
> > if (dtb_filename) {
> > fdt = load_device_tree(dtb_filename, _size);
> > @@ -83,6 +85,9 @@ static int microblaze_load_dtb(hwaddr addr,
> > return 0;
> > }
> >
> > +qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> > +qemu_fdt_setprop(fdt, "/chosen", "rng-seed", rng_seed,
> > sizeof(rng_seed));
> > +
> > if (kernel_cmdline) {
> > r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
> > kernel_cmdline);
> > --
> > 2.37.3
> >
>
> Bumping this patch. Could somebody queue this up?
Hi Jason,
I've just sent a pull-request with this patch.
Thanks,
Edgar
Re: [PATCH v1 03/10] disas: use result of ->read_memory_func
On 21/9/22 18:07, Alex Bennée wrote: This gets especially confusing if you start plugging in host addresses from a trace and you wonder why the output keeps changing. Report when read_memory_func fails instead of blindly disassembling the buffer contents. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson --- disas.c | 20 ++--- disas/capstone.c | 73 2 files changed, 53 insertions(+), 40 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v1 09/10] contrib/plugins: reset skip when matching in execlog
On 21/9/22 18:08, Alex Bennée wrote: The purpose of the matches was to only track the execution of instructions we care about. Without resetting skip to the value at the start of the block we end up dumping all instructions after the match with the consequent load on the instrumentation. Signed-off-by: Alex Bennée Cc: Alexandre Iooss --- contrib/plugins/execlog.c | 3 +++ 1 file changed, 3 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically
If I read your response on the other thread correctly, this change is intended
to prioritize the MAC address exposed by DPDK over the one provided by the
QEMU command line? Sounds reasonable in principle, but I would get confirmation
from vDPA/vhost-net maintainers.
That said the way you’re hacking the vhost-user code breaks a valuable check for
bad vhost-user-blk backends. I would suggest finding another implementation
which
does not regress functionality for other device types.
>From: Hao Chen
>
>When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>start the virtual machine through libvirt or qemu, but now, the libvirt or
>qemu can call dpdk vdpa vendor driver's ops .get_config through
>vhost_net_get_config
>to get the mac address of the vdpa hardware without manual configuration.
>
>v1->v2:
>Only copy ETH_ALEN data of netcfg for some vdpa device such as
>NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>We only need the mac address and don't care about the status field.
>
>Signed-off-by: Hao Chen
>---
> hw/block/vhost-user-blk.c | 1 -
> hw/net/virtio-net.c | 7 +++
> hw/virtio/vhost-user.c| 19 ---
> 3 files changed, 7 insertions(+), 20 deletions(-)
>
>diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>index 9117222456..5dca4eab09 100644
>--- a/hw/block/vhost-user-blk.c
>+++ b/hw/block/vhost-user-blk.c
>@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
>**errp)
>
> vhost_dev_set_config_notifier(>dev, _ops);
>
>-s->vhost_user.supports_config = true;
> ret = vhost_dev_init(>dev, >vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> errp);
> if (ret < 0) {
>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>index dd0d056fde..90405083b1 100644
>--- a/hw/net/virtio-net.c
>+++ b/hw/net/virtio-net.c
>@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev,
>uint8_t *config)
> }
> memcpy(config, , n->config_size);
> }
>+} else if (nc->peer && nc->peer->info->type ==
>NET_CLIENT_DRIVER_VHOST_USER) {
>+ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t
>*),
>+ n->config_size);
>+if (ret != -1) {
>+ /* Automatically obtain the mac address of the vdpa device
>+* when using the dpdk vdpa */
>+memcpy(config, , ETH_ALEN);
> }
> }
>
>diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>index bd24741be8..8b01078249 100644
>--- a/hw/virtio/vhost-user.c
>+++ b/hw/virtio/vhost-user.c
>@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev
>*dev, void *opaque,
> }
>
> if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>-bool supports_f_config = vus->supports_config ||
>-(dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> uint64_t protocol_features;
>
> dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev
>*dev, void *opaque,
> */
> protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>
>-if (supports_f_config) {
>-if (!virtio_has_feature(protocol_features,
>-VHOST_USER_PROTOCOL_F_CONFIG)) {
>-error_setg(errp, "vhost-user device expecting "
>- "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user
>backend does "
>- "not support it.");
>-return -EPROTO;
>-}
>-} else {
>-if (virtio_has_feature(protocol_features,
>- VHOST_USER_PROTOCOL_F_CONFIG)) {
>-warn_reportf_err(*errp, "vhost-user backend supports "
>- "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does
>not.");
>-protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>-}
>-}
>-
> /* final set of protocol features */
> dev->protocol_features = protocol_features;
> err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>--
>2.27.0
>
[PULL v1 0/1] Xilinx queue
From: "Edgar E. Iglesias" The following changes since commit 2906f933dd1de6d94c54881cc16ea7390a6ba300: Merge tag 'pull-request-2022-09-20' of https://gitlab.com/thuth/qemu into staging (2022-09-20 16:24:07 -0400) are available in the Git repository at: g...@github.com:edgarigl/qemu.git tags/edgar/xilinx-next-2022-09-21.for-upstream for you to fetch changes up to b91b6b5a2cd83a096116929dfc8e016091080adc: hw/microblaze: pass random seed to fdt (2022-09-21 19:59:56 +0200) Xilinx queue Jason A. Donenfeld (1): hw/microblaze: pass random seed to fdt hw/microblaze/boot.c | 5 + 1 file changed, 5 insertions(+) Jason A. Donenfeld (1): hw/microblaze: pass random seed to fdt hw/microblaze/boot.c | 5 + 1 file changed, 5 insertions(+) -- 2.25.1
Re: [PULL 00/15] Testing, qga and misc patches
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v2 02/23] target/i386: Return bool from disas_insn
On 8/9/22 14:14, Richard Henderson wrote:
On 9/6/22 15:42, Philippe Mathieu-Daudé wrote:
On 6/9/22 12:09, Richard Henderson wrote:
Instead of returning the new pc, which is present in
DisasContext, return true if an insn was translated.
This is false when we detect a page crossing and must
undo the insn under translation.
Signed-off-by: Richard Henderson
---
target/i386/tcg/translate.c | 42 +++--
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 1e24bb2985..46300ffd91 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -4665,7 +4665,7 @@ static void gen_sse(CPUX86State *env,
DisasContext *s, int b)
/* convert one instruction. s->base.is_jmp is set if the
translation must
be stopped. Return the next pc value */
-static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
+static bool disas_insn(DisasContext *s, CPUState *cpu)
{
CPUX86State *env = cpu->env_ptr;
int b, prefixes;
@@ -4695,12 +4695,13 @@ static target_ulong disas_insn(DisasContext
*s, CPUState *cpu)
return s->pc;
Shouldn't we return 'true' here?
Whoops, yes.
Returning 'true':
Reviewed-by: Philippe Mathieu-Daudé
Re: [PULL 00/12] Publish1 patches
Hi Helge, On 20/9/22 19:31, Helge Deller wrote: The following changes since commit 621da7789083b80d6f1ff1c0fb499334007b4f51: Update version for v7.1.0 release (2022-08-30 09:40:11 -0700) are available in the Git repository at: https://github.com/hdeller/qemu-hppa.git tags/publish1-pull-request for you to fetch changes up to 7f8674a61a908592bb4e8e698f5bef84d0eeb8cc: linux-user: Add parameters of getrandom() syscall for strace (2022-09-18 21:35:27 +0200) linux-user: Add more syscalls, enhance tracing & logging enhancements Here is a bunch of patches for linux-user. Most of them add missing syscalls and enhance the tracing/logging. Some of the patches are target-hppa specific. I've tested those on productive hppa debian buildd servers (running qemu-user). Thanks! Helge Changes to v2: - Fix build of close_range() and pidfd_*() patches on older Linux distributions (noticed by Stefan Hajnoczi) Changes to v1: - Dropped the faccessat2() syscall patch in favour of Richard's patch - Various changes to the "missing signals in strace output" patch based on Richard's feedback, e.g. static arrays, fixed usage of _NSIG, fix build when TARGET_SIGIOT does not exist - Use FUTEX_CMD_MASK in "Show timespec on strace for futex" patch unconditionally and turn into a switch statement - as suggested by Richard Helge Deller (12): linux-user: Add missing signals in strace output linux-user: Add missing clock_gettime64() syscall strace linux-user: Add pidfd_open(), pidfd_send_signal() and pidfd_getfd() syscalls linux-user: Log failing executable in EXCP_DUMP() linux-user/hppa: Use EXCP_DUMP() to show enhanced debug info linux-user/hppa: Dump IIR on register dump linux-user: Fix strace of chmod() if mode == 0 linux-user/hppa: Set TASK_UNMAPPED_BASE to 0xfa00 for hppa arch linux-user: Add strace for clock_nanosleep() linux-user: Show timespec on strace for futex() linux-user: Add close_range() syscall linux-user: Add parameters of getrandom() syscall for strace It seems you missed my review comments: - https://lore.kernel.org/qemu-devel/569161db-c8cf-9ae5-9ae6-161de7f22...@amsat.org/ - https://lore.kernel.org/qemu-devel/d1668b24-9c04-0e54-2a82-7174f0d46...@amsat.org/ - https://lore.kernel.org/qemu-devel/e8bfd1ba-cec7-7c29-9319-eb013c14a...@amsat.org/#t - https://lore.kernel.org/qemu-devel/02090880-0db6-0a6b-60b0-b3313566b...@amsat.org/
Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically
On Wed, Sep 21, 2022 at 07:23:12PM +0100, Alex Bennée wrote:
>
> chenh writes:
>
> > From: Hao Chen
> >
> > When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> > start the virtual machine through libvirt or qemu, but now, the libvirt or
> > qemu can call dpdk vdpa vendor driver's ops .get_config through
> > vhost_net_get_config
> > to get the mac address of the vdpa hardware without manual configuration.
> >
> > v1->v2:
> > Only copy ETH_ALEN data of netcfg for some vdpa device such as
> > NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> > We only need the mac address and don't care about the status field.
> >
> > Signed-off-by: Hao Chen
> > ---
> > hw/block/vhost-user-blk.c | 1 -
> > hw/net/virtio-net.c | 7 +++
> > hw/virtio/vhost-user.c| 19 ---
> > 3 files changed, 7 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > index 9117222456..5dca4eab09 100644
> > --- a/hw/block/vhost-user-blk.c
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev,
> > Error **errp)
> >
> > vhost_dev_set_config_notifier(>dev, _ops);
> >
> > -s->vhost_user.supports_config = true;
>
>
>
> NACK from me. The supports_config flag is there for a reason.
Alex please, do not send NACKs. If you feel compelled to stress
your point, provide extra justification instead. Thanks!
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index bd24741be8..8b01078249 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev
> > *dev, void *opaque,
> > }
> >
> > if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> > -bool supports_f_config = vus->supports_config ||
> > -(dev->config_ops &&
> > dev->config_ops->vhost_dev_config_notifier);
> > uint64_t protocol_features;
> >
> > dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> > @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev
> > *dev, void *opaque,
> > */
> > protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >
> > -if (supports_f_config) {
> > -if (!virtio_has_feature(protocol_features,
> > -VHOST_USER_PROTOCOL_F_CONFIG)) {
> > -error_setg(errp, "vhost-user device expecting "
> > - "VHOST_USER_PROTOCOL_F_CONFIG but the
> > vhost-user backend does "
> > - "not support it.");
> > -return -EPROTO;
> > -}
> > -} else {
> > -if (virtio_has_feature(protocol_features,
> > - VHOST_USER_PROTOCOL_F_CONFIG)) {
> > -warn_reportf_err(*errp, "vhost-user backend supports "
> > - "VHOST_USER_PROTOCOL_F_CONFIG but QEMU
> > does not.");
> > -protocol_features &= ~(1ULL <<
> > VHOST_USER_PROTOCOL_F_CONFIG);
> > -}
> > -}
> > -
> > /* final set of protocol features */
> > dev->protocol_features = protocol_features;
> > err = vhost_user_set_protocol_features(dev,
> > dev->protocol_features);
>
>
> --
> Alex Bennée
Re: [PULL 00/17] ppc queue
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PULL 0/5] M68k for 7.2 patches
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v8 1/8] mm/memfd: Introduce userspace inaccessible memfd
(please excuse any formatting disasters. my internet went out as I was composing this, and i did my best to rescue it.) On Mon, Sep 19, 2022, at 12:10 PM, Sean Christopherson wrote: > +Will, Marc and Fuad (apologies if I missed other pKVM folks) > > On Mon, Sep 19, 2022, David Hildenbrand wrote: >> On 15.09.22 16:29, Chao Peng wrote: >> > From: "Kirill A. Shutemov" >> > >> > KVM can use memfd-provided memory for guest memory. For normal userspace >> > accessible memory, KVM userspace (e.g. QEMU) mmaps the memfd into its >> > virtual address space and then tells KVM to use the virtual address to >> > setup the mapping in the secondary page table (e.g. EPT). >> > >> > With confidential computing technologies like Intel TDX, the >> > memfd-provided memory may be encrypted with special key for special >> > software domain (e.g. KVM guest) and is not expected to be directly >> > accessed by userspace. Precisely, userspace access to such encrypted >> > memory may lead to host crash so it should be prevented. >> >> Initially my thaught was that this whole inaccessible thing is TDX specific >> and there is no need to force that on other mechanisms. That's why I >> suggested to not expose this to user space but handle the notifier >> requirements internally. >> >> IIUC now, protected KVM has similar demands. Either access (read/write) of >> guest RAM would result in a fault and possibly crash the hypervisor (at >> least not the whole machine IIUC). > > Yep. The missing piece for pKVM is the ability to convert from shared > to private > while preserving the contents, e.g. to hand off a large buffer > (hundreds of MiB) > for processing in the protected VM. Thoughts on this at the bottom. > >> > This patch introduces userspace inaccessible memfd (created with >> > MFD_INACCESSIBLE). Its memory is inaccessible from userspace through >> > ordinary MMU access (e.g. read/write/mmap) but can be accessed via >> > in-kernel interface so KVM can directly interact with core-mm without >> > the need to map the memory into KVM userspace. >> >> With secretmem we decided to not add such "concept switch" flags and instead >> use a dedicated syscall. >> > > I have no personal preference whatsoever between a flag and a dedicated > syscall, > but a dedicated syscall does seem like it would give the kernel a bit more > flexibility. The third option is a device node, e.g. /dev/kvm_secretmem or /dev/kvm_tdxmem or similar. But if we need flags or other details in the future, maybe this isn't ideal. > >> What about memfd_inaccessible()? Especially, sealing and hugetlb are not >> even supported and it might take a while to support either. > > Don't know about sealing, but hugetlb support for "inaccessible" memory > needs to > come sooner than later. "inaccessible" in quotes because we might want > to choose > a less binary name, e.g. "restricted"?. > > Regarding pKVM's use case, with the shim approach I believe this can be done > by > allowing userspace mmap() the "hidden" memfd, but with a ton of restrictions > piled on top. > > My first thought was to make the uAPI a set of KVM ioctls so that KVM > could tightly > tightly control usage without taking on too much complexity in the > kernel, but > working through things, routing the behavior through the shim itself > might not be > all that horrific. > > IIRC, we discarded the idea of allowing userspace to map the "private" > fd because > things got too complex, but with the shim it doesn't seem _that_ bad. What's the exact use case? Is it just to pre-populate the memory? > > E.g. on the memfd side: > > 1. The entire memfd must be mapped, and at most one mapping is allowed, i.e. > mapping is all or nothing. > > 2. Acquiring a reference via get_pfn() is disallowed if there's a mapping > for > the restricted memfd. > > 3. Add notifier hooks to allow downstream users to further restrict things. > > 4. Disallow splitting VMAs, e.g. to force userspace to munmap() everything > in > one shot. > > 5. Require that there are no outstanding references at munmap(). Or if this > can't be guaranteed by userspace, maybe add some way for userspace to > wait > until it's ok to convert to private? E.g. so that get_pfn() doesn't need > to do an expensive check every time. Hmm. I haven't looked at the code to see if this would really work, but I think this could be done more in line with how the rest of the kernel works by using the rmap infrastructure. When the pKVM memfd is in not-yet-private mode, just let it be mmapped as usual (but don't allow any form of GUP or pinning). Then have an ioctl to switch to to shared mode that takes locks or sets flags so that no new faults can be serviced and does unmap_mapping_range. As long as the shim arranges to have its own vm_ops, I don't immediately see any reason this can't work.
[PATCH] hw/net: npcm7xx_emc: set MAC in register space
The MAC address set from Qemu wasn't being saved into the register space.
Reviewed-by: Hao Wu
Signed-off-by: Patrick Venture
---
hw/net/npcm7xx_emc.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c
index 7c86bb52e5..6be1008529 100644
--- a/hw/net/npcm7xx_emc.c
+++ b/hw/net/npcm7xx_emc.c
@@ -96,6 +96,9 @@ static const char *emc_reg_name(int regno)
#undef REG
}
+static void npcm7xx_emc_write(void *opaque, hwaddr offset,
+ uint64_t v, unsigned size);
+
static void emc_reset(NPCM7xxEMCState *emc)
{
trace_npcm7xx_emc_reset(emc->emc_num);
@@ -112,6 +115,18 @@ static void emc_reset(NPCM7xxEMCState *emc)
emc->tx_active = false;
emc->rx_active = false;
+
+/* Set the MAC address in the register space. */
+uint32_t value = (emc->conf.macaddr.a[0] << 24) |
+(emc->conf.macaddr.a[1] << 16) |
+(emc->conf.macaddr.a[2] << 8) |
+emc->conf.macaddr.a[3];
+npcm7xx_emc_write(emc, REG_CAMM_BASE * sizeof(uint32_t), value,
+ sizeof(uint32_t));
+
+value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] << 16);
+npcm7xx_emc_write(emc, REG_CAML_BASE * sizeof(uint32_t), value,
+ sizeof(uint32_t));
}
static void npcm7xx_emc_reset(DeviceState *dev)
--
2.37.3.998.g577e59143f-goog
RE: [PATCH v1 3/3] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs
Hi Markus,
Thank you for the review.
> Vivek Kasireddy writes:
>
> > The new parameter named "connector" can be used to assign physical
> > monitors/connectors to individual GFX VCs such that when the monitor
> > is connected or hotplugged, the associated GTK window would be
> > fullscreened on it. If the monitor is disconnected or unplugged,
> > the associated GTK window would be destroyed and a relevant
> > disconnect event would be sent to the Guest.
> >
> > Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,xres=1920,yres=1080...
> >-display gtk,gl=on,connector.0=eDP-1,connector.1=DP-1.
> >
> > Cc: Dongwon Kim
> > Cc: Gerd Hoffmann
> > Cc: Daniel P. Berrangé
> > Cc: Markus Armbruster
> > Cc: Philippe Mathieu-Daudé
> > Cc: Marc-André Lureau
> > Cc: Thomas Huth
> > Signed-off-by: Vivek Kasireddy
> > ---
> > qapi/ui.json| 9 ++-
> > qemu-options.hx | 1 +
> > ui/gtk.c| 168
> > 3 files changed, 177 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 286c5731d1..86787a4c95 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1199,13 +1199,20 @@
> > # interfaces (e.g. VGA and virtual console character devices)
> > # by default.
> > # Since 7.1
> > +# @connector: List of physical monitor/connector names where the GTK
> > windows
> > +# containing the respective graphics virtual consoles (VCs)
> > are
> > +# to be placed. If a mapping exists for a VC, it will be
> > +# fullscreened on that specific monitor or else it would not
> > be
> > +# displayed anywhere and would appear disconnected to the
> > guest.
>
> Let's see whether I understand this... We have VCs numbered 0, 1, ...
> VC #i is mapped to the i-th element of @connector, counting from zero.
> Correct?
[Kasireddy, Vivek] Yes, correct.
>
> Ignorant question: what's a "physical monitor/connector name"?
[Kasireddy, Vivek] IIUC, while the HDMI/DP protocols refer to a receiver (RX)
as a "sink", the DRM Graphics subsystem in the kernel uses the term "connector"
and the GTK library prefers the term "monitor". All of these terms can be
used interchangeably but I chose the term connector for the new parameter
after debating between connector, monitor, output, etc.
The connector name (e.g. DP-1, HDMI-A-2, etc) uniquely identifies a connector
or a monitor on any given system:
https://elixir.bootlin.com/linux/v6.0-rc6/source/include/drm/drm_connector.h#L1328
If you are on Intel hardware, you can find more info related to connectors by
doing:
cat /sys/kernel/debug/dri/0/i915_display_info
>
> Would you mind breaking the lines a bit earlier, between column 70 and
> 75?
[Kasireddy, Vivek] Np, will do that.
>
> > +# Since 7.2
> > #
> > # Since: 2.12
> > ##
> > { 'struct' : 'DisplayGTK',
> >'data': { '*grab-on-hover' : 'bool',
> > '*zoom-to-fit' : 'bool',
> > -'*show-tabs' : 'bool' } }
> > +'*show-tabs' : 'bool',
> > +'*connector' : ['str'] } }
> >
> > ##
> > # @DisplayEGLHeadless:
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 31c04f7eea..576b65ef9f 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1945,6 +1945,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
> > #if defined(CONFIG_GTK)
> > "-display
> > gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
> > "
> > [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]\n"
> > +"[,connector.=]\n"
>
> Is "" a VC number?
[Kasireddy, Vivek] Yes.
>
> > #endif
> > #if defined(CONFIG_VNC)
> > "-display vnc=[,]\n"
>
> Remainder of my review is on style only. Style suggestions are not
> demands :)
[Kasireddy, Vivek] No problem; most of your suggestions are sensible
and I'll include them all in v2.
>
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 945c550909..651aaaf174 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -37,6 +37,7 @@
> > #include "qapi/qapi-commands-misc.h"
> > #include "qemu/cutils.h"
> > #include "qemu/main-loop.h"
> > +#include "qemu/option.h"
> >
> > #include "ui/console.h"
> > #include "ui/gtk.h"
> > @@ -115,6 +116,7 @@
> > #endif
> >
> > #define HOTKEY_MODIFIERS(GDK_CONTROL_MASK | GDK_MOD1_MASK)
> > +#define MAX_NUM_ATTEMPTS 5
>
> Could use a comment, and maybe a more telling name (this one doesn't
> tell me what is being attempted).
[Kasireddy, Vivek] How about MAX_NUM_RETRIES?
gdk_monitor_get_model() can return NULL but if I wait for sometime
(few milliseconds) and try again it may give a valid value. So, I need a
macro to limit the number of attempts or retries.
>
> >
> > static const guint16 *keycode_map;
> > static size_t keycode_maplen;
> > @@ -126,6 +128,15 @@ struct VCChardev {
> > };
> > typedef struct
[PATCH v3 4/5] hw/arm/virt: Improve high memory region address assignment
There are three high memory regions, which are VIRT_HIGH_REDIST2,
VIRT_HIGH_PCIE_ECAM and VIRT_HIGH_PCIE_MMIO. Their base addresses
are floating on highest RAM address. However, they can be disabled
in several cases.
(1) One specific high memory region is disabled by developer by
toggling vms->highmem_{redists, ecam, mmio}.
(2) VIRT_HIGH_PCIE_ECAM region is disabled on machine, which is
'virt-2.12' or ealier than it.
(3) VIRT_HIGH_PCIE_ECAM region is disabled when firmware is loaded
on 32-bits system.
(4) One specific high memory region is disabled when it breaks the
PA space limit.
The current implementation of virt_set_memmap() isn't comprehensive
because the space for one specific high memory region is always
reserved from the PA space for case (1), (2) and (3). In the code,
'base' and 'vms->highest_gpa' are always increased for those three
cases. It's unnecessary since the assigned space of the disabled
high memory region won't be used afterwards.
This improves the address assignment for those three high memory
region by skipping the address assignment for one specific high
memory region if it has been disabled in case (1), (2) and (3).
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 44 ++--
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b0b679d1f4..b702f8f2b5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1693,15 +1693,31 @@ static void virt_set_high_memmap(VirtMachineState *vms,
hwaddr base, int pa_bits)
{
hwaddr region_base, region_size;
-bool fits;
+bool *region_enabled, fits;
int i;
for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
region_base = ROUND_UP(base, extended_memmap[i].size);
region_size = extended_memmap[i].size;
-vms->memmap[i].base = region_base;
-vms->memmap[i].size = region_size;
+switch (i) {
+case VIRT_HIGH_GIC_REDIST2:
+region_enabled = >highmem_redists;
+break;
+case VIRT_HIGH_PCIE_ECAM:
+region_enabled = >highmem_ecam;
+break;
+case VIRT_HIGH_PCIE_MMIO:
+region_enabled = >highmem_mmio;
+break;
+default:
+region_enabled = NULL;
+}
+
+/* Skip unknown region */
+if (!region_enabled) {
+continue;
+}
/*
* Check each device to see if they fit in the PA space,
@@ -1710,23 +1726,15 @@ static void virt_set_high_memmap(VirtMachineState *vms,
* For each device that doesn't fit, disable it.
*/
fits = (region_base + region_size) <= BIT_ULL(pa_bits);
-if (fits) {
-vms->highest_gpa = region_base + region_size - 1;
-}
+if (*region_enabled && fits) {
+vms->memmap[i].base = region_base;
+vms->memmap[i].size = region_size;
-switch (i) {
-case VIRT_HIGH_GIC_REDIST2:
-vms->highmem_redists &= fits;
-break;
-case VIRT_HIGH_PCIE_ECAM:
-vms->highmem_ecam &= fits;
-break;
-case VIRT_HIGH_PCIE_MMIO:
-vms->highmem_mmio &= fits;
-break;
+vms->highest_gpa = region_base + region_size - 1;
+base = region_base + region_size;
+} else {
+*region_enabled = false;
}
-
-base = region_base + region_size;
}
}
--
2.23.0
[PATCH v3 0/5] hw/arm/virt: Improve address assignment for high memory regions
There are three high memory regions, which are VIRT_HIGH_REDIST2,
VIRT_HIGH_PCIE_ECAM and VIRT_HIGH_PCIE_MMIO. Their base addresses
are floating on highest RAM address. However, they can be disabled
in several cases.
(1) One specific high memory region is disabled by developer by
toggling vms->highmem_{redists, ecam, mmio}.
(2) VIRT_HIGH_PCIE_ECAM region is disabled on machine, which is
'virt-2.12' or ealier than it.
(3) VIRT_HIGH_PCIE_ECAM region is disabled when firmware is loaded
on 32-bits system.
(4) One specific high memory region is disabled when it breaks the
PA space limit.
The current implementation of virt_set_memmap() isn't comprehensive
because the space for one specific high memory region is always
reserved from the PA space for case (1), (2) and (3). In the code,
'base' and 'vms->highest_gpa' are always increased for those three
cases. It's unnecessary since the assigned space of the disabled
high memory region won't be used afterwards.
The series intends to improve the address assignment for these
high memory regions.
PATCH[1-3] preparatory work for the improvment
PATCH[4] improve high memory region address assignment
PATCH[5] adds 'highmem-compact' to enable or disable the optimization
History
===
v2: https://lore.kernel.org/all/20220815062958.100366-1-gs...@redhat.com/T/
v1: https://lists.nongnu.org/archive/html/qemu-arm/2022-08/msg00013.html
Changelog
==
v3:
* Reorder the patches(Gavin)
* Add 'highmem-compact' property for backwards compatibility (Eric)
v2:
* Split the patches for easier review(Gavin)
* Improved changelog (Marc)
* Use 'bool fits' in virt_set_high_memmap() (Eric)
Gavin Shan (5):
hw/arm/virt: Introduce virt_set_high_memmap() helper
hw/arm/virt: Rename variable size to region_size in
virt_set_high_memmap()
hw/arm/virt: Introduce variable region_base in virt_set_high_memmap()
hw/arm/virt: Improve high memory region address assignment
hw/arm/virt: Add 'highmem-compact' property
docs/system/arm/virt.rst | 4 ++
hw/arm/virt.c| 116 ---
include/hw/arm/virt.h| 2 +
3 files changed, 89 insertions(+), 33 deletions(-)
--
2.23.0
[PATCH v3 2/5] hw/arm/virt: Rename variable size to region_size in virt_set_high_memmap()
This renames variable 'size' to 'region_size' in virt_set_high_memmap().
Its counterpart ('region_base') will be introduced in next patch.
No functional change intended.
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4dab528b82..187b3ee0e2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1692,15 +1692,16 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState
*vms, int idx)
static void virt_set_high_memmap(VirtMachineState *vms,
hwaddr base, int pa_bits)
{
+hwaddr region_size;
+bool fits;
int i;
for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
-hwaddr size = extended_memmap[i].size;
-bool fits;
+region_size = extended_memmap[i].size;
-base = ROUND_UP(base, size);
+base = ROUND_UP(base, region_size);
vms->memmap[i].base = base;
-vms->memmap[i].size = size;
+vms->memmap[i].size = region_size;
/*
* Check each device to see if they fit in the PA space,
@@ -1708,9 +1709,9 @@ static void virt_set_high_memmap(VirtMachineState *vms,
*
* For each device that doesn't fit, disable it.
*/
-fits = (base + size) <= BIT_ULL(pa_bits);
+fits = (base + region_size) <= BIT_ULL(pa_bits);
if (fits) {
-vms->highest_gpa = base + size - 1;
+vms->highest_gpa = base + region_size - 1;
}
switch (i) {
@@ -1725,7 +1726,7 @@ static void virt_set_high_memmap(VirtMachineState *vms,
break;
}
-base += size;
+base += region_size;
}
}
--
2.23.0
[PATCH v3 1/5] hw/arm/virt: Introduce virt_set_high_memmap() helper
This introduces virt_set_high_memmap() helper. The logic of high
memory region address assignment is moved to the helper. The intention
is to make the subsequent optimization for high memory region address
assignment easier.
No functional change intended.
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 74 ---
1 file changed, 41 insertions(+), 33 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0961e053e5..4dab528b82 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1689,6 +1689,46 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState
*vms, int idx)
return arm_cpu_mp_affinity(idx, clustersz);
}
+static void virt_set_high_memmap(VirtMachineState *vms,
+ hwaddr base, int pa_bits)
+{
+int i;
+
+for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
+hwaddr size = extended_memmap[i].size;
+bool fits;
+
+base = ROUND_UP(base, size);
+vms->memmap[i].base = base;
+vms->memmap[i].size = size;
+
+/*
+ * Check each device to see if they fit in the PA space,
+ * moving highest_gpa as we go.
+ *
+ * For each device that doesn't fit, disable it.
+ */
+fits = (base + size) <= BIT_ULL(pa_bits);
+if (fits) {
+vms->highest_gpa = base + size - 1;
+}
+
+switch (i) {
+case VIRT_HIGH_GIC_REDIST2:
+vms->highmem_redists &= fits;
+break;
+case VIRT_HIGH_PCIE_ECAM:
+vms->highmem_ecam &= fits;
+break;
+case VIRT_HIGH_PCIE_MMIO:
+vms->highmem_mmio &= fits;
+break;
+}
+
+base += size;
+}
+}
+
static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
{
MachineState *ms = MACHINE(vms);
@@ -1744,39 +1784,7 @@ static void virt_set_memmap(VirtMachineState *vms, int
pa_bits)
/* We know for sure that at least the memory fits in the PA space */
vms->highest_gpa = memtop - 1;
-for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
-hwaddr size = extended_memmap[i].size;
-bool fits;
-
-base = ROUND_UP(base, size);
-vms->memmap[i].base = base;
-vms->memmap[i].size = size;
-
-/*
- * Check each device to see if they fit in the PA space,
- * moving highest_gpa as we go.
- *
- * For each device that doesn't fit, disable it.
- */
-fits = (base + size) <= BIT_ULL(pa_bits);
-if (fits) {
-vms->highest_gpa = base + size - 1;
-}
-
-switch (i) {
-case VIRT_HIGH_GIC_REDIST2:
-vms->highmem_redists &= fits;
-break;
-case VIRT_HIGH_PCIE_ECAM:
-vms->highmem_ecam &= fits;
-break;
-case VIRT_HIGH_PCIE_MMIO:
-vms->highmem_mmio &= fits;
-break;
-}
-
-base += size;
-}
+virt_set_high_memmap(vms, base, pa_bits);
if (device_memory_size > 0) {
ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
--
2.23.0
[PATCH v3 5/5] hw/arm/virt: Add 'highmem-compact' property
After the improvement to high memory region address assignment is
applied, the memory layout is changed. For example, VIRT_HIGH_PCIE_MMIO
memory region is enabled when the improvement is applied, but it's
disabled if the improvement isn't applied.
pa_bits = 40;
vms->highmem_redists = false;
vms->highmem_ecam= false;
vms->highmem_mmio= true;
# qemu-system-aarch64 -accel kvm -cpu host \
-machine virt-7.2 -m 4G,maxmem=511G \
-monitor stdio
In order to keep backwords compatibility, we need to disable the
optimization on machines, which is virt-7.1 or ealier than it. It
means the optimization is enabled by default from virt-7.2. Besides,
'highmem-compact' property is added so that the optimization can be
explicitly enabled or disabled on all machine types by users.
Signed-off-by: Gavin Shan
---
docs/system/arm/virt.rst | 4
hw/arm/virt.c| 33 +
include/hw/arm/virt.h| 2 ++
3 files changed, 39 insertions(+)
diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 20442ea2c1..f05ec2253b 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -94,6 +94,10 @@ highmem
address space above 32 bits. The default is ``on`` for machine types
later than ``virt-2.12``.
+highmem-compact
+ Set ``on``/``off`` to enable/disable compact space for high memory regions.
+ The default is ``on`` for machine types later than ``virt-7.2``
+
gic-version
Specify the version of the Generic Interrupt Controller (GIC) to provide.
Valid values are:
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b702f8f2b5..a4fbdaef91 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1734,6 +1734,13 @@ static void virt_set_high_memmap(VirtMachineState *vms,
base = region_base + region_size;
} else {
*region_enabled = false;
+
+if (!vms->highmem_compact) {
+base = region_base + region_size;
+if (fits) {
+vms->highest_gpa = region_base + region_size - 1;
+}
+}
}
}
}
@@ -2348,6 +2355,20 @@ static void virt_set_highmem(Object *obj, bool value,
Error **errp)
vms->highmem = value;
}
+static bool virt_get_highmem_compact(Object *obj, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(obj);
+
+return vms->highmem_compact;
+}
+
+static void virt_set_highmem_compact(Object *obj, bool value, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(obj);
+
+vms->highmem_compact = value;
+}
+
static bool virt_get_its(Object *obj, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -2966,6 +2987,13 @@ static void virt_machine_class_init(ObjectClass *oc,
void *data)
"Set on/off to enable/disable using "
"physical address space above 32
bits");
+object_class_property_add_bool(oc, "highmem-compact",
+ virt_get_highmem_compact,
+ virt_set_highmem_compact);
+object_class_property_set_description(oc, "highmem-compact",
+ "Set on/off to enable/disable
compact "
+ "space for high memory regions");
+
object_class_property_add_str(oc, "gic-version", virt_get_gic_version,
virt_set_gic_version);
object_class_property_set_description(oc, "gic-version",
@@ -3050,6 +3078,7 @@ static void virt_instance_init(Object *obj)
/* High memory is enabled by default */
vms->highmem = true;
+vms->highmem_compact = !vmc->no_highmem_compact;
vms->gic_version = VIRT_GIC_VERSION_NOSEL;
vms->highmem_ecam = !vmc->no_highmem_ecam;
@@ -3119,8 +3148,12 @@ DEFINE_VIRT_MACHINE_AS_LATEST(7, 2)
static void virt_machine_7_1_options(MachineClass *mc)
{
+VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
virt_machine_7_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_7_1, hw_compat_7_1_len);
+/* Compact space for high memory regions was introduced with 7.2 */
+vmc->no_highmem_compact = true;
}
DEFINE_VIRT_MACHINE(7, 1)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 6ec479ca2b..c7dd59d7f1 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -125,6 +125,7 @@ struct VirtMachineClass {
bool no_pmu;
bool claim_edge_triggered_timers;
bool smbios_old_sys_ver;
+bool no_highmem_compact;
bool no_highmem_ecam;
bool no_ged; /* Machines < 4.2 have no support for ACPI GED device */
bool kvm_no_adjvtime;
@@ -144,6 +145,7 @@ struct VirtMachineState {
PFlashCFI01 *flash[2];
bool secure;
bool highmem;
+bool highmem_compact;
bool highmem_ecam;
bool highmem_mmio;
bool highmem_redists;
--
2.23.0
[PATCH v3 3/5] hw/arm/virt: Introduce variable region_base in virt_set_high_memmap()
This introduces variable 'region_base' for the base address of the
specific high memory region. It's the preparatory work to optimize
high memory region address assignment.
No functional change intended.
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 187b3ee0e2..b0b679d1f4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1692,15 +1692,15 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState
*vms, int idx)
static void virt_set_high_memmap(VirtMachineState *vms,
hwaddr base, int pa_bits)
{
-hwaddr region_size;
+hwaddr region_base, region_size;
bool fits;
int i;
for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
+region_base = ROUND_UP(base, extended_memmap[i].size);
region_size = extended_memmap[i].size;
-base = ROUND_UP(base, region_size);
-vms->memmap[i].base = base;
+vms->memmap[i].base = region_base;
vms->memmap[i].size = region_size;
/*
@@ -1709,9 +1709,9 @@ static void virt_set_high_memmap(VirtMachineState *vms,
*
* For each device that doesn't fit, disable it.
*/
-fits = (base + region_size) <= BIT_ULL(pa_bits);
+fits = (region_base + region_size) <= BIT_ULL(pa_bits);
if (fits) {
-vms->highest_gpa = base + region_size - 1;
+vms->highest_gpa = region_base + region_size - 1;
}
switch (i) {
@@ -1726,7 +1726,7 @@ static void virt_set_high_memmap(VirtMachineState *vms,
break;
}
-base += region_size;
+base = region_base + region_size;
}
}
--
2.23.0
Re: [PATCH v2 13/23] target/i386: Introduce DISAS_JUMP
On 9/21/22 12:28, Paolo Bonzini wrote:
On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson
wrote:
Drop the unused dest argument to gen_jr().
Remove most of the calls to gen_jr, and use DISAS_JUMP.
Remove some unused loads of eip for lcall and ljmp.
The only use outside i386_tr_tb_stop is here:
static void gen_goto_tb(DisasContext *s, int tb_num, target_ulong eip)
{
target_ulong pc = s->cs_base + eip;
if (translator_use_goto_tb(>base, pc)) {
/* jump to same page: we can use a direct jump */
tcg_gen_goto_tb(tb_num);
gen_jmp_im(s, eip);
tcg_gen_exit_tb(s->base.tb, tb_num);
s->base.is_jmp = DISAS_NORETURN;
} else {
/* jump to another page */
gen_jmp_im(s, eip);
gen_jr(s);
}
}
Should it set s->base.is_jmp = DISAS_JUMP instead, so that gen_jr() can be
inlined into i386_tr_tb_stop() and removed completely? If not,
It can't, because of conditional branches which do
brcond something, L1
gen_goto_tb
L1
gen_goto_tb
The first gen_goto_tb can't just fall through, it needs to exit.
r~
Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled
Hi
On Tue, Sep 20, 2022 at 3:18 PM Bin Meng wrote:
> From: Xuzhou Cheng
>
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
>
> Signed-off-by: Xuzhou Cheng
> Signed-off-by: Bin Meng
> Reviewed-by: Marc-André Lureau
>
fwiw, I didn't r-b the version with a busy wait
(
https://patchew.org/QEMU/20220824094029.1634519-1-bmeng...@gmail.com/20220824094029.1634519-42-bmeng...@gmail.com/
)
---
>
> Changes in v2:
> - Change to a busy wait after migration is canceled
>
> tests/qtest/migration-test.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index c87afad9e8..aedd9ddb72 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2133,6 +2133,10 @@ static void test_multifd_tcp_cancel(void)
> wait_for_migration_pass(from);
>
> migrate_cancel(from);
> +/* Make sure QEMU process "to" exited */
> +while (qtest_probe_child(to)) {
> +;
> +}
>
> args = (MigrateStart){
> .only_target = true,
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
Re: [PATCH 2/3] vdpa: load vlan configuration at NIC startup
On 9/16/2022 6:45 AM, Eugenio Perez Martin wrote:
On Wed, Sep 14, 2022 at 5:44 PM Si-Wei Liu wrote:
On 9/14/2022 2:57 PM, Eugenio Perez Martin wrote:
On Wed, Sep 14, 2022 at 1:33 PM Si-Wei Liu wrote:
On 9/14/2022 3:20 AM, Jason Wang wrote:
On Fri, Sep 9, 2022 at 4:02 PM Eugenio Perez Martin wrote:
On Fri, Sep 9, 2022 at 8:40 AM Jason Wang wrote:
On Fri, Sep 9, 2022 at 2:38 PM Jason Wang wrote:
On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez wrote:
To have enabled vlans at device startup may happen in the destination of
a live migration, so this configuration must be restored.
At this moment the code is not accessible, since SVQ refuses to start if
vlan feature is exposed by the device.
Signed-off-by: Eugenio Pérez
---
net/vhost-vdpa.c | 46 --
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 4bc3fd01a8..ecbfd08eb9 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
BIT_ULL(VIRTIO_NET_F_STANDBY);
+#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */
+
VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
{
VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
@@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
return *s->status != VIRTIO_NET_OK;
}
+static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
+ const VirtIONet *n,
+ uint16_t vid)
+{
+ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
+ VIRTIO_NET_CTRL_VLAN_ADD,
+ , sizeof(vid));
+if (unlikely(dev_written < 0)) {
+return dev_written;
+}
+
+if (unlikely(*s->status != VIRTIO_NET_OK)) {
+return -EINVAL;
+}
+
+return 0;
+}
+
+static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
+const VirtIONet *n)
+{
+uint64_t features = n->parent_obj.guest_features;
+
+if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
+return 0;
+}
+
+for (int i = 0; i < MAX_VLAN >> 5; i++) {
+for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
+if (n->vlans[i] & (1U << j)) {
+int r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);
This seems to cause a lot of latency if the driver has a lot of vlans.
I wonder if it's simply to let all vlan traffic go by disabling
CTRL_VLAN feature at vDPA layer.
The guest will not be able to recover that vlan configuration.
Spec said it's best effort and we actually don't do this for
vhost-net/user for years and manage to survive.
I thought there's a border line between best effort and do nothing. ;-)
I also think it is worth at least trying to migrate it for sure. For
MAC there may be too many entries, but VLAN should be totally
manageable (and the information is already there, the bitmap is
actually being migrated).
The vlan bitmap is migrated, though you'd still need to add vlan filter
one by one through ctrl vq commands, correct? AFAIK you can add one
filter for one single vlan ID at a time via VIRTIO_NET_CTRL_VLAN_ADD.
I think that the reason this could survive is really software
implementation specific - say if the backend doesn't start with VLAN
filtering disabled (meaning allow all VLAN traffic to pass) then it
would become a problem. This won't be a problem for almost PF NICs but
may be problematic for vDPA e.g. VF backed VDPAs.
I'd say the driver would expect all vlan filters to be cleared after a
reset, isn't it?
If I understand the intended behavior (from QEMU implementation)
correctly, when VIRTIO_NET_F_CTRL_VLAN is negotiated it would start with
all VLANs filtered (meaning only untagged traffic can be received and
traffic with VLAN tag would get dropped). However, if
VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including untagged
and tagged can be received.
The spec doesn't explicitly say anything about that
as far as I see.
Here the spec is totally ruled by the (software artifact of)
implementation rather than what a real device is expected to work with
VLAN rx filters. Are we sure we'd stick to this flawed device
implementation? The guest driver seems to be agnostic with this broken
spec behavior so far, and I am afraid it's an overkill to add another
feature bit or ctrl command to VLAN filter in clean way.
I agree with all of the above. So, double checking, all vlan should be
allowed by default at device start?
That is true only when VIRTIO_NET_F_CTRL_VLAN is not negotiated. If the
guest already negotiated VIRTIO_NET_F_CTRL_VLAN before being migrated,
device should resume with all VLANs filtered/disallowed.
Maybe the spec needs to be more
Re: [PATCH v2 26/39] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled
On Thu, Sep 22, 2022 at 5:54 AM Marc-André Lureau wrote: > > Hi > > On Tue, Sep 20, 2022 at 3:18 PM Bin Meng wrote: >> >> From: Xuzhou Cheng >> >> Make sure QEMU process "to" exited before launching another target >> for migration in the test_multifd_tcp_cancel case. >> >> Signed-off-by: Xuzhou Cheng >> Signed-off-by: Bin Meng >> Reviewed-by: Marc-André Lureau > > > fwiw, I didn't r-b the version with a busy wait > (https://patchew.org/QEMU/20220824094029.1634519-1-bmeng...@gmail.com/20220824094029.1634519-42-bmeng...@gmail.com/) > My mistake. The R-B tag was added before I changed the implementation and I forgot to remove the tag. Regards, Bin
Re: [PATCH v1 01/10] monitor: expose monitor_puts to rest of code
Alex Bennée writes: > This helps us construct strings elsewhere before echoing to the > monitor. It avoids having to jump through hoops like: > > monitor_printf(mon, "%s", s->str); > > It will be useful in following patches but for now convert all > existing plain "%s" printfs to use the _puts api. > > Signed-off-by: Alex Bennée > Reviewed-by: Richard Henderson Reviewed-by: Markus Armbruster
Re: [PATCH v1 3/3] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs
"Kasireddy, Vivek" writes:
> Hi Markus,
>
> Thank you for the review.
>
>> Vivek Kasireddy writes:
>>
>> > The new parameter named "connector" can be used to assign physical
>> > monitors/connectors to individual GFX VCs such that when the monitor
>> > is connected or hotplugged, the associated GTK window would be
>> > fullscreened on it. If the monitor is disconnected or unplugged,
>> > the associated GTK window would be destroyed and a relevant
>> > disconnect event would be sent to the Guest.
>> >
>> > Usage: -device
>> > virtio-gpu-pci,max_outputs=2,blob=true,xres=1920,yres=1080...
>> >-display gtk,gl=on,connector.0=eDP-1,connector.1=DP-1.
>> >
>> > Cc: Dongwon Kim
>> > Cc: Gerd Hoffmann
>> > Cc: Daniel P. Berrangé
>> > Cc: Markus Armbruster
>> > Cc: Philippe Mathieu-Daudé
>> > Cc: Marc-André Lureau
>> > Cc: Thomas Huth
>> > Signed-off-by: Vivek Kasireddy
>> > ---
>> > qapi/ui.json| 9 ++-
>> > qemu-options.hx | 1 +
>> > ui/gtk.c| 168
>> > 3 files changed, 177 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/qapi/ui.json b/qapi/ui.json
>> > index 286c5731d1..86787a4c95 100644
>> > --- a/qapi/ui.json
>> > +++ b/qapi/ui.json
>> > @@ -1199,13 +1199,20 @@
>> > # interfaces (e.g. VGA and virtual console character
>> > devices)
>> > # by default.
>> > # Since 7.1
>> > +# @connector: List of physical monitor/connector names where the GTK
>> > windows
>> > +# containing the respective graphics virtual consoles (VCs)
>> > are
>> > +# to be placed. If a mapping exists for a VC, it will be
>> > +# fullscreened on that specific monitor or else it would
>> > not be
>> > +# displayed anywhere and would appear disconnected to the
>> > guest.
>>
>> Let's see whether I understand this... We have VCs numbered 0, 1, ...
>> VC #i is mapped to the i-th element of @connector, counting from zero.
>> Correct?
>
> [Kasireddy, Vivek] Yes, correct.
>
>> Ignorant question: what's a "physical monitor/connector name"?
>
> [Kasireddy, Vivek] IIUC, while the HDMI/DP protocols refer to a receiver (RX)
> as a "sink", the DRM Graphics subsystem in the kernel uses the term
> "connector"
> and the GTK library prefers the term "monitor".
Awesome.
> All of these terms can be
> used interchangeably but I chose the term connector for the new parameter
> after debating between connector, monitor, output, etc.
Okay.
> The connector name (e.g. DP-1, HDMI-A-2, etc) uniquely identifies a connector
> or a monitor on any given system:
> https://elixir.bootlin.com/linux/v6.0-rc6/source/include/drm/drm_connector.h#L1328
> If you are on Intel hardware, you can find more info related to connectors by
> doing:
> cat /sys/kernel/debug/dri/0/i915_display_info
Thanks!
>> Would you mind breaking the lines a bit earlier, between column 70 and
>> 75?
>
> [Kasireddy, Vivek] Np, will do that.
>
>>
>> > +# Since 7.2
>> > #
>> > # Since: 2.12
>> > ##
>> > { 'struct' : 'DisplayGTK',
>> >'data': { '*grab-on-hover' : 'bool',
>> > '*zoom-to-fit' : 'bool',
>> > -'*show-tabs' : 'bool' } }
>> > +'*show-tabs' : 'bool',
>> > +'*connector' : ['str'] } }
>> >
>> > ##
>> > # @DisplayEGLHeadless:
Elsewhere in ui.json, names of list-valued members use plural: channels,
clients, keys, addresses. Let's name this one connectors for
consistency.
With that, QAPI schema
Acked-by: Markus Armbruster
>> > diff --git a/qemu-options.hx b/qemu-options.hx
>> > index 31c04f7eea..576b65ef9f 100644
>> > --- a/qemu-options.hx
>> > +++ b/qemu-options.hx
>> > @@ -1945,6 +1945,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>> > #if defined(CONFIG_GTK)
>> > "-display
>> > gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
>> > "
>> > [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]\n"
>> > +"[,connector.=]\n"
>>
>> Is "" a VC number?
>
> [Kasireddy, Vivek] Yes.
An "id" is commonly a name. Suggest connector..
>> > #endif
>> > #if defined(CONFIG_VNC)
>> > "-display vnc=[,]\n"
A bit of documentation is missing:
``show-cursor=on|off`` : Force showing the mouse cursor
``window-close=on|off`` : Allow to quit qemu with window close
button
+``connector.`` :
``curses[,charset=]``
>>
>> Remainder of my review is on style only. Style suggestions are not
>> demands :)
>
> [Kasireddy, Vivek] No problem; most of your suggestions are sensible
> and I'll include them all in v2.
Thanks!
Re: [PATCH 0/1] hw/display: expose linear framebuffer address in Bochs VBE registers
Hi, > > Why not just use virtio-gpu? > > Trying to run this command: > qemu-system-x86_64 -M microvm -m 2048 -device virtio-gpu '-device virtio-gpu-device' Might also need '-global virtio-mmio.force-legacy=false' to switch virtio-mmio into 1.0 mode. take care, Gerd
Re: [PATCH v3 0/5] hw/arm/virt: Improve address assignment for high memory regions
[PATCH v3 0/5] hw/arm/virt: Improve address assignment for high memory regions
Author: Gavin Shan
Date: Thu Sep 22 07:13:45 2022 +0800
PATCH[1-3] preparatory work for the improvment
PATCH[4] improve high memory region address assignment
PATCH[5] adds 'highmem-compact' to enable or disable the optimization
Signed-off-by: Gavin Shan
The patchs works well on "IPA Size Limit: 40" FUJITSU machine.
I added some debug code to show high memory region address.
The test results are as expected.
1. virt-7.2 default
# /home/qemu/build/qemu-system-aarch64 -accel kvm -cpu host -machine
virt-7.2 -m 4G,maxmem=511G -monitor stdio
=> virt_set_high_memmap: pa_bits=40, base=0x80
[HIGH_GIC_REDIST2]: disabled, highest_gpa=0x7f[no] [yes]
[HIGH_PCIE_ECAM]: disabled, highest_gpa=0x7f[no] [yes]
[HIGH_PCIE_MMIO] enabled, highest_gpa=0xff
2. When virt-7.2,highmem-compact=off
# /home/qemu/build/qemu-system-aarch64 -accel kvm -cpu host -machine
virt-7.2,highmem-compact=off -m 4G,maxmem=511G -monitor stdio
=> virt_set_high_memmap: pa_bits=40, base=0x80
[HIGH_GIC_REDIST2]: disabled, highest_gpa=0x8003ff[no] [yes]
[HIGH_PCIE_ECAM]: disabled, highest_gpa=0x801fff[no] [yes]
[HIGH_PCIE_MMIO]: disabled, highest_gpa=0x801fff[no] [no]
3. virt-7.1 default
# /home/qemu/build/qemu-system-aarch64 -accel kvm -cpu host -machine
virt-7.1 -m 4G,maxmem=511G -monitor stdio
=> virt_set_high_memmap: pa_bits=40, base=0x80
[HIGH_GIC_REDIST2]: disabled, highest_gpa=0x8003ff[no] [yes]
[HIGH_PCIE_ECAM]: disabled, highest_gpa=0x801fff[no] [yes]
[HIGH_PCIE_MMIO]: disabled, highest_gpa=0x801fff[no] [no]
2. When virt-7.1,highmem-compact=on
# /home/qemu/build/qemu-system-aarch64 -accel kvm -cpu host -machine
virt-7.1,highmem-compact=on -m 4G,maxmem=511G -monitor stdio
=> virt_set_high_memmap: pa_bits=40, base=0x80
[HIGH_GIC_REDIST2]: disabled, highest_gpa=0x7f[no] [yes]
[HIGH_PCIE_ECAM]: disabled, highest_gpa=0x7f[no] [yes]
[HIGH_PCIE_MMIO] enabled, highest_gpa=0xff
Tested-by: Zhenyu Zhang
On Thu, Sep 22, 2022 at 7:13 AM Gavin Shan wrote:
>
> There are three high memory regions, which are VIRT_HIGH_REDIST2,
> VIRT_HIGH_PCIE_ECAM and VIRT_HIGH_PCIE_MMIO. Their base addresses
> are floating on highest RAM address. However, they can be disabled
> in several cases.
>
> (1) One specific high memory region is disabled by developer by
> toggling vms->highmem_{redists, ecam, mmio}.
>
> (2) VIRT_HIGH_PCIE_ECAM region is disabled on machine, which is
> 'virt-2.12' or ealier than it.
>
> (3) VIRT_HIGH_PCIE_ECAM region is disabled when firmware is loaded
> on 32-bits system.
>
> (4) One specific high memory region is disabled when it breaks the
> PA space limit.
>
> The current implementation of virt_set_memmap() isn't comprehensive
> because the space for one specific high memory region is always
> reserved from the PA space for case (1), (2) and (3). In the code,
> 'base' and 'vms->highest_gpa' are always increased for those three
> cases. It's unnecessary since the assigned space of the disabled
> high memory region won't be used afterwards.
>
> The series intends to improve the address assignment for these
> high memory regions.
>
> PATCH[1-3] preparatory work for the improvment
> PATCH[4] improve high memory region address assignment
> PATCH[5] adds 'highmem-compact' to enable or disable the optimization
>
> History
> ===
> v2: https://lore.kernel.org/all/20220815062958.100366-1-gs...@redhat.com/T/
> v1: https://lists.nongnu.org/archive/html/qemu-arm/2022-08/msg00013.html
>
> Changelog
> ==
> v3:
> * Reorder the patches(Gavin)
> * Add 'highmem-compact' property for backwards compatibility (Eric)
> v2:
> * Split the patches for easier review(Gavin)
> * Improved changelog (Marc)
> * Use 'bool fits' in virt_set_high_memmap() (Eric)
>
> Gavin Shan (5):
> hw/arm/virt: Introduce virt_set_high_memmap() helper
> hw/arm/virt: Rename variable size to region_size in
> virt_set_high_memmap()
> hw/arm/virt: Introduce variable region_base in virt_set_high_memmap()
> hw/arm/virt: Improve high memory region address assignment
> hw/arm/virt: Add 'highmem-compact' property
>
> docs/system/arm/virt.rst | 4 ++
> hw/arm/virt.c| 116 ---
> include/hw/arm/virt.h| 2 +
> 3 files changed, 89 insertions(+), 33 deletions(-)
>
> --
> 2.23.0
>
Re: [PATCH v2 35/39] tests/qtest: migration-test: Skip running some TLS cases for win32
On Thu, Sep 22, 2022 at 1:23 AM Daniel P. Berrangé wrote:
>
> On Wed, Sep 21, 2022 at 05:51:33PM +0100, Dr. David Alan Gilbert wrote:
> > * Bin Meng (bmeng...@gmail.com) wrote:
> > > From: Bin Meng
> > >
> > > Some migration test cases use TLS to communicate, but they fail on
> > > Windows with the following error messages:
> > >
> > > qemu-system-x86_64: TLS handshake failed: Insufficient credentials for
> > > that request.
> > > qemu-system-x86_64: TLS handshake failed: Error in the pull function.
> > > query-migrate shows failed migration: TLS handshake failed: Error in
> > > the pull function.
> > >
> > > Disable them temporarily.
> > >
> > > Signed-off-by: Bin Meng
> > > ---
> > > I am not familar with the gnutls and simply enabling the gnutls debug
> > > output does not give me an immedidate hint on why it's failing on
> > > Windows. Disable these cases for now until someone or maintainers
> > > who may want to test this on Windows.
> >
> > Copying in Dan Berrange, he's our expert on weird TLS failures.
>
> Seems to match this:
>
>https://gnutls.org/faq.html#key-usage-violation2
>
> which suggests we have a configuration mis-match.
>
> I'm surprised to see you are only needing to disable the TLS PSK tests,
> not the TLS x509 tests.
The TLS x509 qtests all passed.
>
> I'd like to know if tests/unit/test-crypto-tlssession passes.
These unit tests currently are not built on Windows as they simply
don't build due to usage of socketpair().
>
> If so, it might suggest we are missing 'priority: NORMAL' property
> when configuring TLS creds for the migration test.
I did the following changes but the error is still the same:
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index dbee9b528a..c1e3f11873 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -788,7 +788,8 @@ test_migrate_tls_psk_start_common(QTestState *from,
" 'id': 'tlscredspsk0',"
" 'endpoint': 'client',"
" 'dir': %s,"
- " 'username': 'qemu'} }",
+ " 'username': 'qemu',"
+ " 'priority': 'NORMAL'} }",
data->workdir);
qobject_unref(rsp);
@@ -797,7 +798,8 @@ test_migrate_tls_psk_start_common(QTestState *from,
" 'arguments': { 'qom-type': 'tls-creds-psk',"
" 'id': 'tlscredspsk0',"
" 'endpoint': 'server',"
- " 'dir': %s } }",
+ " 'dir': %s,"
+ " 'priority': 'NORMAL'} }",
mismatch ? data->workdiralt : data->workdir);
qobject_unref(rsp);
I am not sure whether I did the right changes.
>
> > > (no changes since v1)
> > >
> > > tests/qtest/migration-test.c | 14 ++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > > index aedd9ddb72..dbee9b528a 100644
> > > --- a/tests/qtest/migration-test.c
> > > +++ b/tests/qtest/migration-test.c
> > > @@ -1403,6 +1403,7 @@ static void test_precopy_unix_dirty_ring(void)
> > > }
> > >
> > > #ifdef CONFIG_GNUTLS
> > > +#ifndef _WIN32
> > > static void test_precopy_unix_tls_psk(void)
> > > {
> > > g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > > @@ -1415,6 +1416,7 @@ static void test_precopy_unix_tls_psk(void)
> > >
> > > test_precopy_common();
> > > }
> > > +#endif /* _WIN32 */
> > >
> > > #ifdef CONFIG_TASN1
> > > static void test_precopy_unix_tls_x509_default_host(void)
> > > @@ -1523,6 +1525,7 @@ static void test_precopy_tcp_plain(void)
> > > }
> > >
> > > #ifdef CONFIG_GNUTLS
> > > +#ifndef _WIN32
> > > static void test_precopy_tcp_tls_psk_match(void)
> > > {
> > > MigrateCommon args = {
> > > @@ -1533,6 +1536,7 @@ static void test_precopy_tcp_tls_psk_match(void)
> > >
> > > test_precopy_common();
> > > }
> > > +#endif /* _WIN32 */
> > >
> > > static void test_precopy_tcp_tls_psk_mismatch(void)
> > > {
> > > @@ -1930,6 +1934,7 @@ static void test_multifd_tcp_zstd(void)
> > > #endif
> > >
> > > #ifdef CONFIG_GNUTLS
> > > +#ifndef _WIN32
> > > static void *
> > > test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > > QTestState *to)
> > > @@ -1937,6 +1942,7 @@
> > > test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
> > > test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
> > > return test_migrate_tls_psk_start_match(from, to);
> > > }
> > > +#endif /* _WIN32 */
> > >
> > > static void *
> > > test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
> > > @@ -1988,6 +1994,7 @@
> > > test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
> > > }
> > > #endif /* CONFIG_TASN1 */
> > >
> > > +#ifndef _WIN32
> > > static void test_multifd_tcp_tls_psk_match(void)
> > > {
> > > MigrateCommon args = {
> > > @@ -1997,6 +2004,7 @@ static void test_multifd_tcp_tls_psk_match(void)
> > > };
> > > test_precopy_common();
> > > }
> > > +#endif /* _WIN32 */
> > >
> > > static void test_multifd_tcp_tls_psk_mismatch(void)
> > > {
> > > @@ -2497,8 +2505,10 @@ int
Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods
On Wed, 2022-09-21 at 15:29 +0200, Igor Mammedov wrote:
> On Tue, 20 Sep 2022 20:28:31 +0800
> Robert Hoo wrote:
>
> > On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> > > On Fri, 16 Sep 2022 21:15:35 +0800
> > > Robert Hoo wrote:
> > >
> > > > On Fri, 2022-09-16 at 09:37 +0200, Igor Mammedov wrote:
> > > >
> > > > > > Fine, get your point now.
> > > > > > In ASL it will look like this:
> > > > > > Local1 = Package (0x3) {STTS, SLSA,
> > > > > > MAXT}
> > > > > > Return (Local1)
> > > > >
> > > > >
> > > > > >
> > > > > > But as for
> > > > > > CreateDWordField (Local0, Zero,
> > > > > > STTS) //
> > > > > > Status
> > > > > > CreateDWordField (Local0, 0x04,
> > > > > > SLSA) //
> > > > > > SizeofLSA
> > > > > > CreateDWordField (Local0, 0x08,
> > > > > > MAXT) //
> > > > > > Max
> > > > > > Trans
> > > > > >
> > > > > > I cannot figure out how to substitute with LocalX. Can you
> > > > > > shed
> > > > > > more
> > > > > > light?
> > > > >
> > > > > Leave this as is, there is no way to make it anonymous/local
> > > > > with
> > > > > FooField.
> > > > >
> > > > > (well one might try to use Index and copy field's bytes into
> > > > > a
> > > > > buffer
> > > > > and
> > > > > then explicitly convert to Integer, but that's a rather
> > > > > convoluted
> > > > > way
> > > > > around limitation so I'd not go this route)
> > > > >
> > > >
> > > > OK, pls. take a look, how about this?
> > > >
> > > > Method (_LSI, 0, Serialized) // _LSI: Label Storage
> > > > Information
> > > > {
> > > > Local0 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-
> > > > 0800200c9a66"),
> > > > 0x02, 0x04, Zero, One)// Buffer
> > > > CreateDWordField (Local0, Zero, STTS) // Status
> > > > CreateDWordField (Local0, 0x04, SLSA) // Size of LSA
> > > > CreateDWordField (Local0, 0x08, MAXT) // Max Transfer Size
> > > > Local1 = Package (0x3) {STTS, SLSA, MAXT}
> > > > Return (Local1)
> > > > }
> > > >
> > > > Method (_LSR, 2, Serialized) // _LSR: Label Storage Read
> > > > {
> > > > Name (INPT, Buffer(8) {})
> > > > CreateDWordField (INPT, Zero, OFST);
> > > > CreateDWordField (INPT, 4, LEN);
> > >
> > > why do you have to create and use INPT, wouldn't local be enough
> > > to
> > > keep the buffer?
> >
> > If substitute INPT with LocalX, then later
> > Local0 = Package (0x01) {LocalX} isn't accepted.
> >
> > PackageElement :=
> > DataObject | NameString
>
> ok, then respin series and lets get it some testing.
Sure. I'd also like it to go through your tests, though I had done some
ordinary tests like guest create/delete/init-labels on vNVDIMM.
>
> BTW:
> it looks like Windows Server starting from v2019 has support for
> NVDIMM-P devices which came with 'Optane DC Persistent Memory
> Modules'
> but it fails to recognize NVDIMMs in QEMU (complaining something
> about
> wrong target). Perhaps you can reach someone with Optane/ACPI
> expertise within your org and try to fix QEMU side.
Yes, it's a known gap there. I will try that once I had some time and
resource.
>
> > >
> > > > OFST = Arg0
> > > > LEN = Arg1
> > > > Local0 = Package (0x01) {INPT}
> > > > Local3 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-
> > > > 0800200c9a66"),
> > > > 0x02, 0x05, Local0, One)
> > > > CreateDWordField (Local3, Zero, STTS)
> > > > CreateField (Local3, 32, LEN << 3, LDAT)
> > > > Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > > Return (Local1)
> > > > }
> > > >
> > > > Method (_LSW, 3, Serialized) // _LSW: Label Storage Write
> > > > {
> > > > Local2 = Arg2
> > > > Name (INPT, Buffer(8) {})
> > >
> > > ditto
> > >
> > > > CreateDWordField (INPT, Zero, OFST);
> > > > CreateDWordField (INPT, 4, TLEN);
> > > > OFST = Arg0
> > > > TLEN = Arg1
> > > > Concatenate(INPT, Local2, INPT)
> > > > Local0 = Package (0x01)
> > > > {
> > > > INPT
> > > > }
> > > > Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-
> > > > 0800200c9a66"),
> > > > 0x02, 0x06, Local0, One)
> > > > CreateDWordField (Local3, 0, STTS)
> > > > Return (STTS)
> > > > }
> >
> >
>
>
Re: [PATCH] Revert "intel_iommu: Fix irqchip / X2APIC configuration checks"
On Thu, Sep 22, 2022 at 12:12 AM Peter Xu wrote:
>
> It's true that when vcpus<=255 we don't require the length of 32bit APIC
> IDs. However here since we already have EIM=ON it means the hypervisor
> will declare the VM as x2apic supported (e.g. VT-d ECAP register will have
> EIM bit 4 set), so the guest should assume the APIC IDs are 32bits width
> even if vcpus<=255. In short, commit 77250171bdc breaks any simple cmdline
> that wants to boot a VM with >=9 but <=255 vcpus with:
>
> -device intel-iommu,intremap=on
>
> For anyone who does not want to enable x2apic, we can use eim=off in the
> intel-iommu parameters to skip enabling KVM x2apic.
>
> This partly reverts commit 77250171bdc02aee106083fd2a068147befa1a38, while
> keeping the valid bit on checking split irqchip, but revert the other change.
>
> Cc: David Woodhouse
> Cc: Claudio Fontana
> Cc: Igor Mammedov
> Signed-off-by: Peter Xu
> ---
> hw/i386/intel_iommu.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 05d53a1aa9..6524c2ee32 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3818,6 +3818,11 @@ static bool vtd_decide_config(IntelIOMMUState *s,
> Error **errp)
> error_setg(errp, "eim=on requires
> accel=kvm,kernel-irqchip=split");
> return false;
> }
> +if (!kvm_enable_x2apic()) {
> +error_setg(errp, "eim=on requires support on the KVM side"
> + "(X2APIC_API, first shipped in v4.7)");
> +return false;
> +}
I wonder if we need some work on the migration compatibility here
(though it could be tricky).
Thanks
> }
>
> /* Currently only address widths supported are 39 and 48 bits */
> --
> 2.32.0
>
Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically
On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
wrote:
>
> If I read your response on the other thread correctly, this change is intended
>
> to prioritize the MAC address exposed by DPDK over the one provided by the
>
> QEMU command line? Sounds reasonable in principle, but I would get
> confirmation
>
> from vDPA/vhost-net maintainers.
I think the best way is to (and it seems easier)
1) have the management layer to make sure the mac came from cli
matches the underlayer vDPA
2) having a sanity check and fail the device initialization if they don't match
Thanks
>
>
>
> That said the way you’re hacking the vhost-user code breaks a valuable check
> for
>
> bad vhost-user-blk backends. I would suggest finding another implementation
> which
>
> does not regress functionality for other device types.
>
>
>
>
>
> >From: Hao Chen
>
> >
>
> >When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>
> >start the virtual machine through libvirt or qemu, but now, the libvirt or
>
> >qemu can call dpdk vdpa vendor driver's ops .get_config through
> >vhost_net_get_config
>
> >to get the mac address of the vdpa hardware without manual configuration.
>
> >
>
> >v1->v2:
>
> >Only copy ETH_ALEN data of netcfg for some vdpa device such as
>
> >NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>
> >We only need the mac address and don't care about the status field.
>
> >
>
> >Signed-off-by: Hao Chen
>
> >---
>
> > hw/block/vhost-user-blk.c | 1 -
>
> > hw/net/virtio-net.c | 7 +++
>
> > hw/virtio/vhost-user.c| 19 ---
>
> > 3 files changed, 7 insertions(+), 20 deletions(-)
>
> >
>
> >diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>
> >index 9117222456..5dca4eab09 100644
>
> >--- a/hw/block/vhost-user-blk.c
>
> >+++ b/hw/block/vhost-user-blk.c
>
> >@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev,
> >Error **errp)
>
> >
>
> > vhost_dev_set_config_notifier(>dev, _ops);
>
> >
>
> >-s->vhost_user.supports_config = true;
>
> > ret = vhost_dev_init(>dev, >vhost_user, VHOST_BACKEND_TYPE_USER,
> > 0,
>
> > errp);
>
> > if (ret < 0) {
>
> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>
> >index dd0d056fde..90405083b1 100644
>
> >--- a/hw/net/virtio-net.c
>
> >+++ b/hw/net/virtio-net.c
>
> >@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev,
> >uint8_t *config)
>
> > }
>
> > memcpy(config, , n->config_size);
>
> > }
>
> >+} else if (nc->peer && nc->peer->info->type ==
> >NET_CLIENT_DRIVER_VHOST_USER) {
>
> >+ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t
> >*),
>
> >+ n->config_size);
>
> >+if (ret != -1) {
>
> >+ /* Automatically obtain the mac address of the vdpa device
>
> >+* when using the dpdk vdpa */
>
> >+memcpy(config, , ETH_ALEN);
>
> > }
>
> > }
>
> >
>
> >diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>
> >index bd24741be8..8b01078249 100644
>
> >--- a/hw/virtio/vhost-user.c
>
> >+++ b/hw/virtio/vhost-user.c
>
> >@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev
> >*dev, void *opaque,
>
> > }
>
> >
>
> > if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>
> >-bool supports_f_config = vus->supports_config ||
>
> >-(dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>
> > uint64_t protocol_features;
>
> >
>
> > dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>
> >@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev
> >*dev, void *opaque,
>
> > */
>
> > protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>
> >
>
> >-if (supports_f_config) {
>
> >-if (!virtio_has_feature(protocol_features,
>
> >-VHOST_USER_PROTOCOL_F_CONFIG)) {
>
> >-error_setg(errp, "vhost-user device expecting "
>
> >- "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user
> >backend does "
>
> >- "not support it.");
>
> >-return -EPROTO;
>
> >-}
>
> >-} else {
>
> >-if (virtio_has_feature(protocol_features,
>
> >- VHOST_USER_PROTOCOL_F_CONFIG)) {
>
> >-warn_reportf_err(*errp, "vhost-user backend supports "
>
> >- "VHOST_USER_PROTOCOL_F_CONFIG but QEMU
> >does not.");
>
> >-protocol_features &= ~(1ULL <<
> >VHOST_USER_PROTOCOL_F_CONFIG);
>
> >-}
>
> >-}
>
> >-
>
> > /* final set of protocol features */
>
> > dev->protocol_features = protocol_features;
>
> > err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>
> >--
>
> >2.27.0
>
> >
