[Virtio-fs] vhost_set_vring_kick failed
Sergio Lopez
slp at redhat.com
Wed Jun 1 06:01:19 UTC 2022
On Tue, May 31, 2022 at 10:25:12PM +0000, Pra.. Dew.. wrote:
> Thank you so much Vivek. I enabled the trace in the code itself and
> ran it. The only message I got was InvalidParam message below. I did
> not get any other message. We are going to try changing the
> structure [VhostUserMemRegMsg] padding to see if that fixes the
> problem. Is there anything else we should try?
Applying 3009edff81 to qemu-5.1.0 should do the trick. I'm attaching
the patch to this email, so you can simply apply it with something
like:
patch -p1 < 0001-vhost-user-fix-VHOST_USER_ADD-REM_MEM_REG-truncation.patch
Sergio.
-------------- next part --------------
From 3009edff8192991293fe9e2b50b0d90db83c4a89 Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha at redhat.com>
Date: Mon, 9 Nov 2020 17:43:55 +0000
Subject: [PATCH] vhost-user: fix VHOST_USER_ADD/REM_MEM_REG truncation
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 at nutanix.com>
Cc: Cornelia Huck <cohuck at redhat.com>
Cc: Michael S. Tsirkin <mst at redhat.com>
Cc: Christian Borntraeger <borntraeger at de.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha at redhat.com>
Message-Id: <20201109174355.1069147-1-stefanha at redhat.com>
Reviewed-by: Michael S. Tsirkin <mst at redhat.com>
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
Fixes: f1aeb14b0809 ("Transmit vhost-user memory regions individually")
Reviewed-by: Cornelia Huck <cohuck at redhat.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz at nutanix.com>
---
contrib/libvhost-user/libvhost-user.h | 2 +-
docs/interop/vhost-user.rst | 21 +++++++++++++++++++--
hw/virtio/vhost-user.c | 5 ++---
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/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
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,
--
2.36.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virtio-fs/attachments/20220601/35dea76c/attachment-0001.sig>
More information about the Virtio-fs
mailing list