* [PULL v2 1/7] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 2/7] meson: move vhost_user_blk_server to meson.build Michael S. Tsirkin
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Cornelia Huck, Christian Borntraeger,
Stefan Hajnoczi, Raphael Norwitz
From: Stefan Hajnoczi <stefanha@redhat.com>
QEMU currently truncates the mmap_offset field when sending
VHOST_USER_ADD_MEM_REG and VHOST_USER_REM_MEM_REG messages. The struct
layout looks like this:
typedef struct VhostUserMemoryRegion {
uint64_t guest_phys_addr;
uint64_t memory_size;
uint64_t userspace_addr;
uint64_t mmap_offset;
} VhostUserMemoryRegion;
typedef struct VhostUserMemRegMsg {
uint32_t padding;
/* WARNING: there is a 32-bit hole here! */
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
The payload size is calculated as follows when sending the message in
hw/virtio/vhost-user.c:
msg->hdr.size = sizeof(msg->payload.mem_reg.padding) +
sizeof(VhostUserMemoryRegion);
This calculation produces an incorrect result of only 36 bytes.
sizeof(VhostUserMemRegMsg) is actually 40 bytes.
The consequence of this is that the final field, mmap_offset, is
truncated. This breaks x86_64 TCG guests on s390 hosts. Other guest/host
combinations may get lucky if either of the following holds:
1. The guest memory layout does not need mmap_offset != 0.
2. The host is little-endian and mmap_offset <= 0xffffffff so the
truncation has no effect.
Fix this by extending the existing 32-bit padding field to 64-bit. Now
the padding reflects the actual compiler padding. This can be verified
using pahole(1).
Also document the layout properly in the vhost-user specification. The
vhost-user spec did not document the exact layout. It would be
impossible to implement the spec without looking at the QEMU source
code.
Existing vhost-user frontends and device backends continue to work after
this fix has been applied. The only change in the wire protocol is that
QEMU now sets hdr.size to 40 instead of 36. If a vhost-user
implementation has a hardcoded size check for 36 bytes, then it will
fail with new QEMUs. Both QEMU and DPDK/SPDK don't check the exact
payload size, so they continue to work.
Fixes: f1aeb14b0809e313c74244d838645ed25e85ea63 ("Transmit vhost-user memory regions individually")
Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201109174355.1069147-1-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fixes: f1aeb14b0809 ("Transmit vhost-user memory regions individually")
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
contrib/libvhost-user/libvhost-user.h | 2 +-
hw/virtio/vhost-user.c | 5 ++---
docs/interop/vhost-user.rst | 21 +++++++++++++++++++--
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h
index a1539dbb69..7d47f1364a 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -136,7 +136,7 @@ typedef struct VhostUserMemory {
} VhostUserMemory;
typedef struct VhostUserMemRegMsg {
- uint32_t padding;
+ uint64_t padding;
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9c5b4f7fbc..2fdd5daf74 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -149,7 +149,7 @@ typedef struct VhostUserMemory {
} VhostUserMemory;
typedef struct VhostUserMemRegMsg {
- uint32_t padding;
+ uint64_t padding;
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
@@ -800,8 +800,7 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev,
uint64_t shadow_pcb[VHOST_USER_MAX_RAM_SLOTS] = {};
int nr_add_reg, nr_rem_reg;
- msg->hdr.size = sizeof(msg->payload.mem_reg.padding) +
- sizeof(VhostUserMemoryRegion);
+ msg->hdr.size = sizeof(msg->payload.mem_reg);
/* Find the regions which need to be removed or added. */
scrub_shadow_regions(dev, add_reg, &nr_add_reg, rem_reg, &nr_rem_reg,
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 988f154144..6d4025ba6a 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -131,6 +131,23 @@ A region is:
:mmap offset: 64-bit offset where region starts in the mapped memory
+Single memory region description
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
++---------+---------------+------+--------------+-------------+
+| padding | guest address | size | user address | mmap offset |
++---------+---------------+------+--------------+-------------+
+
+:padding: 64-bit
+
+:guest address: a 64-bit guest address of the region
+
+:size: a 64-bit size
+
+:user address: a 64-bit user address
+
+:mmap offset: 64-bit offset where region starts in the mapped memory
+
Log description
^^^^^^^^^^^^^^^
@@ -1281,7 +1298,7 @@ Master message types
``VHOST_USER_ADD_MEM_REG``
:id: 37
:equivalent ioctl: N/A
- :slave payload: memory region
+ :slave payload: single memory region description
When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
feature has been successfully negotiated, this message is submitted
@@ -1296,7 +1313,7 @@ Master message types
``VHOST_USER_REM_MEM_REG``
:id: 38
:equivalent ioctl: N/A
- :slave payload: memory region
+ :slave payload: single memory region description
When the ``VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS`` protocol
feature has been successfully negotiated, this message is submitted
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL v2 2/7] meson: move vhost_user_blk_server to meson.build
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 1/7] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 3/7] vhost-user-blk-server: depend on CONFIG_VHOST_USER Michael S. Tsirkin
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Max Reitz, Stefan Hajnoczi,
Philippe Mathieu-Daudé
From: Stefan Hajnoczi <stefanha@redhat.com>
The --enable/disable-vhost-user-blk-server options were implemented in
./configure. There has been confusion about them and part of the problem
is that the shell syntax used for setting the default value is not easy
to read. Move the option over to meson where the conditions are easier
to understand:
have_vhost_user_blk_server = (targetos == 'linux')
if get_option('vhost_user_blk_server').enabled()
if targetos != 'linux'
error('vhost_user_blk_server requires linux')
endif
elif get_option('vhost_user_blk_server').disabled() or not have_system
have_vhost_user_blk_server = false
endif
This patch does not change behavior.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201110171121.1265142-2-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
meson_options.txt | 2 ++
configure | 16 ++++------------
block/export/meson.build | 5 ++++-
meson.build | 12 ++++++++++++
4 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/meson_options.txt b/meson_options.txt
index b4f1801875..f6f64785fe 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -64,6 +64,8 @@ option('xkbcommon', type : 'feature', value : 'auto',
description: 'xkbcommon support')
option('virtiofsd', type: 'feature', value: 'auto',
description: 'build virtiofs daemon (virtiofsd)')
+option('vhost_user_blk_server', type: 'feature', value: 'auto',
+ description: 'build vhost-user-blk server')
option('capstone', type: 'combo', value: 'auto',
choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
diff --git a/configure b/configure
index 4cef321d9d..516f28a088 100755
--- a/configure
+++ b/configure
@@ -329,7 +329,7 @@ vhost_crypto=""
vhost_scsi=""
vhost_vsock=""
vhost_user=""
-vhost_user_blk_server=""
+vhost_user_blk_server="auto"
vhost_user_fs=""
kvm="auto"
hax="auto"
@@ -1247,9 +1247,9 @@ for opt do
;;
--enable-vhost-vsock) vhost_vsock="yes"
;;
- --disable-vhost-user-blk-server) vhost_user_blk_server="no"
+ --disable-vhost-user-blk-server) vhost_user_blk_server="disabled"
;;
- --enable-vhost-user-blk-server) vhost_user_blk_server="yes"
+ --enable-vhost-user-blk-server) vhost_user_blk_server="enabled"
;;
--disable-vhost-user-fs) vhost_user_fs="no"
;;
@@ -2390,12 +2390,6 @@ if test "$vhost_net" = ""; then
test "$vhost_kernel" = "yes" && vhost_net=yes
fi
-# libvhost-user is Linux-only
-test "$vhost_user_blk_server" = "" && vhost_user_blk_server=$linux
-if test "$vhost_user_blk_server" = "yes" && test "$linux" = "no"; then
- error_exit "--enable-vhost-user-blk-server is only available on Linux"
-fi
-
##########################################
# pkg-config probe
@@ -6289,9 +6283,6 @@ fi
if test "$vhost_vdpa" = "yes" ; then
echo "CONFIG_VHOST_VDPA=y" >> $config_host_mak
fi
-if test "$vhost_user_blk_server" = "yes" ; then
- echo "CONFIG_VHOST_USER_BLK_SERVER=y" >> $config_host_mak
-fi
if test "$vhost_user_fs" = "yes" ; then
echo "CONFIG_VHOST_USER_FS=y" >> $config_host_mak
fi
@@ -7012,6 +7003,7 @@ NINJA=$ninja $meson setup \
-Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt \
-Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
-Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
+ -Dvhost_user_blk_server=$vhost_user_blk_server \
$cross_arg \
"$PWD" "$source_path"
diff --git a/block/export/meson.build b/block/export/meson.build
index 19526435d8..135b356775 100644
--- a/block/export/meson.build
+++ b/block/export/meson.build
@@ -1,2 +1,5 @@
blockdev_ss.add(files('export.c'))
-blockdev_ss.add(when: 'CONFIG_VHOST_USER_BLK_SERVER', if_true: files('vhost-user-blk-server.c'))
+
+if have_vhost_user_blk_server
+ blockdev_ss.add(files('vhost-user-blk-server.c'))
+endif
diff --git a/meson.build b/meson.build
index b473620321..4b789f18c1 100644
--- a/meson.build
+++ b/meson.build
@@ -751,6 +751,16 @@ statx_test = '''
has_statx = cc.links(statx_test)
+have_vhost_user_blk_server = (targetos == 'linux')
+
+if get_option('vhost_user_blk_server').enabled()
+ if targetos != 'linux'
+ error('vhost_user_blk_server requires linux')
+ endif
+elif get_option('vhost_user_blk_server').disabled() or not have_system
+ have_vhost_user_blk_server = false
+endif
+
#################
# config-host.h #
#################
@@ -775,6 +785,7 @@ config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
config_host_data.set('CONFIG_CURSES', curses.found())
config_host_data.set('CONFIG_SDL', sdl.found())
config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
+config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_PNG', png.found())
@@ -2103,6 +2114,7 @@ summary_info += {'vhost-crypto support': config_host.has_key('CONFIG_VHOST_CRYPT
summary_info += {'vhost-scsi support': config_host.has_key('CONFIG_VHOST_SCSI')}
summary_info += {'vhost-vsock support': config_host.has_key('CONFIG_VHOST_VSOCK')}
summary_info += {'vhost-user support': config_host.has_key('CONFIG_VHOST_KERNEL')}
+summary_info += {'vhost-user-blk server support': have_vhost_user_blk_server}
summary_info += {'vhost-user-fs support': config_host.has_key('CONFIG_VHOST_USER_FS')}
summary_info += {'vhost-vdpa support': config_host.has_key('CONFIG_VHOST_VDPA')}
summary_info += {'Trace backends': config_host['TRACE_BACKENDS']}
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL v2 3/7] vhost-user-blk-server: depend on CONFIG_VHOST_USER
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 1/7] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 2/7] meson: move vhost_user_blk_server to meson.build Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 4/7] configure: mark vhost-user Linux-only Michael S. Tsirkin
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Richard Henderson,
Stefan Hajnoczi, Paolo Bonzini, Marc-André Lureau,
Alex Bennée
From: Stefan Hajnoczi <stefanha@redhat.com>
I interpreted CONFIG_VHOST_USER as controlling only QEMU's vhost-user
device frontends. However, virtiofsd and contrib/ vhost-user device
backends are also controlled by CONFIG_VHOST_USER. Make the
vhost-user-blk server depend on CONFIG_VHOST_USER for consistency.
Now the following error is printed when the vhost-user-blk server is
enabled without CONFIG_VHOST_USER:
$ ./configure --disable-vhost-user --enable-vhost-user-blk ...
../meson.build:761:8: ERROR: Problem encountered: vhost_user_blk_server requires vhost-user support
Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201110171121.1265142-3-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
meson.build | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 4b789f18c1..7fd874eec7 100644
--- a/meson.build
+++ b/meson.build
@@ -751,11 +751,14 @@ statx_test = '''
has_statx = cc.links(statx_test)
-have_vhost_user_blk_server = (targetos == 'linux')
+have_vhost_user_blk_server = (targetos == 'linux' and
+ 'CONFIG_VHOST_USER' in config_host)
if get_option('vhost_user_blk_server').enabled()
if targetos != 'linux'
error('vhost_user_blk_server requires linux')
+ elif 'CONFIG_VHOST_USER' not in config_host
+ error('vhost_user_blk_server requires vhost-user support')
endif
elif get_option('vhost_user_blk_server').disabled() or not have_system
have_vhost_user_blk_server = false
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL v2 4/7] configure: mark vhost-user Linux-only
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
` (2 preceding siblings ...)
2020-11-17 9:19 ` [PULL v2 3/7] vhost-user-blk-server: depend on CONFIG_VHOST_USER Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 5/7] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off Michael S. Tsirkin
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Richard Henderson,
Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé
From: Stefan Hajnoczi <stefanha@redhat.com>
The vhost-user protocol uses the Linux eventfd feature and is typically
connected to Linux kvm.ko ioeventfd and irqfd file descriptors. The
protocol specification in docs/interop/vhost-user.rst does not describe
how platforms without eventfd support work.
The QEMU vhost-user devices compile on other POSIX host operating
systems because eventfd usage is abstracted in QEMU. The libvhost-user
programs in contrib/ do not compile but we failed to notice since they
are not built by default.
Make it clear that vhost-user is only supported on Linux for the time
being. If someone wishes to support it on other platforms then the
details can be added to vhost-user.rst and CI jobs can test the feature
to prevent bitrot.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20201110171121.1265142-4-stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
configure | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index 516f28a088..3fbc2a0c68 100755
--- a/configure
+++ b/configure
@@ -328,7 +328,7 @@ vhost_net=""
vhost_crypto=""
vhost_scsi=""
vhost_vsock=""
-vhost_user=""
+vhost_user="no"
vhost_user_blk_server="auto"
vhost_user_fs=""
kvm="auto"
@@ -718,7 +718,6 @@ fi
case $targetos in
MINGW32*)
mingw32="yes"
- vhost_user="no"
audio_possible_drivers="dsound sdl"
if check_include dsound.h; then
audio_drv_list="dsound"
@@ -797,6 +796,7 @@ Linux)
audio_possible_drivers="oss alsa sdl pa"
linux="yes"
linux_user="yes"
+ vhost_user="yes"
;;
esac
@@ -2341,9 +2341,8 @@ fi
# vhost interdependencies and host support
# vhost backends
-test "$vhost_user" = "" && vhost_user=yes
-if test "$vhost_user" = "yes" && test "$mingw32" = "yes"; then
- error_exit "vhost-user isn't available on win32"
+if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
+ error_exit "vhost-user is only available on Linux"
fi
test "$vhost_vdpa" = "" && vhost_vdpa=$linux
if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL v2 5/7] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
` (3 preceding siblings ...)
2020-11-17 9:19 ` [PULL v2 4/7] configure: mark vhost-user Linux-only Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 6/7] contrib/libvhost-user: Fix bad printf format specifiers Michael S. Tsirkin
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Eduardo Habkost, Igor Mammedov, Ani Sinha,
Paolo Bonzini, Philippe Mathieu-Daudé, Richard Henderson
From: Philippe Mathieu-Daudé <philmd@redhat.com>
GCC 9.3.0 thinks that 'method' can be left uninitialized. This code
is already in the "if (bsel || pcihp_bridge_en)" block statement,
but it isn't smart enough to figure it out.
Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)"
block statement to fix (on Ubuntu):
../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
496 | aml_append(parent_scope, method);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201107194045.438027-1-philmd@redhat.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/acpi-build.c | 45 +++++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4f66642d88..1f5c211245 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -465,34 +465,31 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
*/
if (bsel || pcihp_bridge_en) {
method = aml_method("PCNT", 0, AML_NOTSERIALIZED);
- }
- /* If bus supports hotplug select it and notify about local events */
- if (bsel) {
- uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
- aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
- aml_append(method,
- aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check */)
- );
- aml_append(method,
- aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject Request */)
- );
- }
+ /* If bus supports hotplug select it and notify about local events */
+ if (bsel) {
+ uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
- /* Notify about child bus events in any case */
- if (pcihp_bridge_en) {
- QLIST_FOREACH(sec, &bus->child, sibling) {
- int32_t devfn = sec->parent_dev->devfn;
-
- if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
- continue;
- }
-
- aml_append(method, aml_name("^S%.02X.PCNT", devfn));
+ aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
+ aml_append(method, aml_call2("DVNT", aml_name("PCIU"),
+ aml_int(1))); /* Device Check */
+ aml_append(method, aml_call2("DVNT", aml_name("PCID"),
+ aml_int(3))); /* Eject Request */
+ }
+
+ /* Notify about child bus events in any case */
+ if (pcihp_bridge_en) {
+ QLIST_FOREACH(sec, &bus->child, sibling) {
+ int32_t devfn = sec->parent_dev->devfn;
+
+ if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
+ continue;
+ }
+
+ aml_append(method, aml_name("^S%.02X.PCNT", devfn));
+ }
}
- }
- if (bsel || pcihp_bridge_en) {
aml_append(parent_scope, method);
}
qobject_unref(bsel);
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL v2 6/7] contrib/libvhost-user: Fix bad printf format specifiers
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
` (4 preceding siblings ...)
2020-11-17 9:19 ` [PULL v2 5/7] hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 9:19 ` [PULL v2 7/7] vhost-user-blk/scsi: Fix broken error handling for socket call Michael S. Tsirkin
2020-11-17 14:12 ` [PULL v2 0/7] pc,vhost: fixes Peter Maydell
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Raphael Norwitz, AlexChen, Stefan Hajnoczi,
Euler Robot, Marc-André Lureau, Philippe Mathieu-Daudé
From: AlexChen <alex.chen@huawei.com>
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-Id: <5FA28106.6000901@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
contrib/libvhost-user/libvhost-user.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index bfec8a881a..5c73ffdd6b 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -701,7 +701,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
return false;
}
- DPRINT("Adding region: %d\n", dev->nregions);
+ DPRINT("Adding region: %u\n", dev->nregions);
DPRINT(" guest_phys_addr: 0x%016"PRIx64"\n",
msg_region->guest_phys_addr);
DPRINT(" memory_size: 0x%016"PRIx64"\n",
@@ -848,7 +848,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
VhostUserMemory m = vmsg->payload.memory, *memory = &m;
dev->nregions = memory->nregions;
- DPRINT("Nregions: %d\n", memory->nregions);
+ DPRINT("Nregions: %u\n", memory->nregions);
for (i = 0; i < dev->nregions; i++) {
void *mmap_addr;
VhostUserMemoryRegion *msg_region = &memory->regions[i];
@@ -938,7 +938,7 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
return vu_set_mem_table_exec_postcopy(dev, vmsg);
}
- DPRINT("Nregions: %d\n", memory->nregions);
+ DPRINT("Nregions: %u\n", memory->nregions);
for (i = 0; i < dev->nregions; i++) {
void *mmap_addr;
VhostUserMemoryRegion *msg_region = &memory->regions[i];
@@ -1049,8 +1049,8 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int num = vmsg->payload.state.num;
- DPRINT("State.index: %d\n", index);
- DPRINT("State.num: %d\n", num);
+ DPRINT("State.index: %u\n", index);
+ DPRINT("State.num: %u\n", num);
dev->vq[index].vring.num = num;
return false;
@@ -1105,8 +1105,8 @@ vu_set_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int num = vmsg->payload.state.num;
- DPRINT("State.index: %d\n", index);
- DPRINT("State.num: %d\n", num);
+ DPRINT("State.index: %u\n", index);
+ DPRINT("State.num: %u\n", num);
dev->vq[index].shadow_avail_idx = dev->vq[index].last_avail_idx = num;
return false;
@@ -1117,7 +1117,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
{
unsigned int index = vmsg->payload.state.index;
- DPRINT("State.index: %d\n", index);
+ DPRINT("State.index: %u\n", index);
vmsg->payload.state.num = dev->vq[index].last_avail_idx;
vmsg->size = sizeof(vmsg->payload.state);
@@ -1478,8 +1478,8 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int enable = vmsg->payload.state.num;
- DPRINT("State.index: %d\n", index);
- DPRINT("State.enable: %d\n", enable);
+ DPRINT("State.index: %u\n", index);
+ DPRINT("State.enable: %u\n", enable);
if (index >= dev->max_queues) {
vu_panic(dev, "Invalid vring_enable index: %u", index);
@@ -1728,7 +1728,7 @@ vu_handle_vring_kick(VuDev *dev, VhostUserMsg *vmsg)
return false;
}
- DPRINT("Got kick message: handler:%p idx:%d\n",
+ DPRINT("Got kick message: handler:%p idx:%u\n",
dev->vq[index].handler, index);
if (!dev->vq[index].started) {
@@ -1772,7 +1772,7 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("Request: %s (%d)\n", vu_request_to_string(vmsg->request),
vmsg->request);
DPRINT("Flags: 0x%x\n", vmsg->flags);
- DPRINT("Size: %d\n", vmsg->size);
+ DPRINT("Size: %u\n", vmsg->size);
if (vmsg->fd_num) {
int i;
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL v2 7/7] vhost-user-blk/scsi: Fix broken error handling for socket call
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
` (5 preceding siblings ...)
2020-11-17 9:19 ` [PULL v2 6/7] contrib/libvhost-user: Fix bad printf format specifiers Michael S. Tsirkin
@ 2020-11-17 9:19 ` Michael S. Tsirkin
2020-11-17 14:12 ` [PULL v2 0/7] pc,vhost: fixes Peter Maydell
7 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-11-17 9:19 UTC (permalink / raw)
To: qemu-devel; +Cc: AlexChen, Peter Maydell, Raphael Norwitz, Euler Robot
From: AlexChen <alex.chen@huawei.com>
When socket() fails, it returns -1, 0 is the normal return value and should not return error.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
Message-Id: <5F9A5B48.9030509@huawei.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
contrib/vhost-user-blk/vhost-user-blk.c | 2 +-
contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index caad88637e..dc981bf945 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -476,7 +476,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
- if (sock <= 0) {
+ if (sock < 0) {
perror("socket");
return -1;
}
diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 3c912384e9..0f9ba4b2a2 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
- if (sock <= 0) {
+ if (sock < 0) {
perror("socket");
return -1;
}
--
MST
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PULL v2 0/7] pc,vhost: fixes
2020-11-17 9:18 [PULL v2 0/7] pc,vhost: fixes Michael S. Tsirkin
` (6 preceding siblings ...)
2020-11-17 9:19 ` [PULL v2 7/7] vhost-user-blk/scsi: Fix broken error handling for socket call Michael S. Tsirkin
@ 2020-11-17 14:12 ` Peter Maydell
7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2020-11-17 14:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: QEMU Developers
On Tue, 17 Nov 2020 at 09:19, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit c6f28ed5075df79fef39c500362a3f4089256c9c:
>
> Update version for v5.2.0-rc1 release (2020-11-10 22:29:57 +0000)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 91010f0407a07caeacb11037bb5b493bab7ce203:
>
> vhost-user-blk/scsi: Fix broken error handling for socket call (2020-11-17 04:16:55 -0500)
>
> ----------------------------------------------------------------
> pc,vhost: fixes
>
> Fixes all over the place.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread