| Message ID | 20210222161017.570837-4-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vhost-user: warn when guest RAM is not shared | expand |
On Mon, Feb 22, 2021 at 8:11 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > Memory regions must be mmap(MAP_SHARED) so that modifications made by > the vhost device backend process are visible to QEMU and vice versa. Use > the new memory_region_is_mapped_shared() function to check this and > print a warning if guest RAM is not shared: > > qemu-system-x86_64: -device vhost-user-fs-pci,chardev=char0,tag=myfs: > warning: Found vhost-user memory region without MAP_SHARED (did you forget > -object memory-*,share=on?) > qemu-system-x86_64: Failed to read from slave. > > This warning makes it clear that memory needs to be configured with > share=on. Without this patch the vhost device is initialized and the > device fails with vague error messages caused by inconsistent memory > views. The warning should make it easier to troubleshoot QEMU > command-lines that lack share=on memory. > > I couldn't figure out how to make this a vhost_dev_init() error, because > memory regions aren't necessarily final when it is called. Also, hotplug > can add memory regions without MAP_SHARED in the future, so we still > need to warn about that. > > Reported-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- > hw/virtio/vhost-user.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 2fdd5daf74..17c2fb81be 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -2223,11 +2223,23 @@ vhost_user_crypto_close_session(struct vhost_dev > *dev, uint64_t session_id) > static bool vhost_user_mem_section_filter(struct vhost_dev *dev, > MemoryRegionSection *section) > { > - bool result; > + /* An fd is required so we can pass it to the device backend process > */ > + if (memory_region_get_fd(section->mr) < 0) { > + return false; > + } > > - result = memory_region_get_fd(section->mr) >= 0; > - > - return result; > + /* > + * It must be mmap(MAP_SHARED) so memory stores from the device > backend > + * process are visible to us and vice versa. > + */ > + if (!memory_region_is_mapped_shared(section->mr)) { > + static bool warned; > + warn_report_once_cond(&warned, "Found vhost-user memory region " > + "without MAP_SHARED (did you forget -object " > + "memory-*,share=on?)"); > + return false; > + } > + return true; > } > > static int vhost_user_get_inflight_fd(struct vhost_dev *dev, > -- > 2.29.2 > >
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 2fdd5daf74..17c2fb81be 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -2223,11 +2223,23 @@ vhost_user_crypto_close_session(struct vhost_dev *dev, uint64_t session_id) static bool vhost_user_mem_section_filter(struct vhost_dev *dev, MemoryRegionSection *section) { - bool result; + /* An fd is required so we can pass it to the device backend process */ + if (memory_region_get_fd(section->mr) < 0) { + return false; + } - result = memory_region_get_fd(section->mr) >= 0; - - return result; + /* + * It must be mmap(MAP_SHARED) so memory stores from the device backend + * process are visible to us and vice versa. + */ + if (!memory_region_is_mapped_shared(section->mr)) { + static bool warned; + warn_report_once_cond(&warned, "Found vhost-user memory region " + "without MAP_SHARED (did you forget -object " + "memory-*,share=on?)"); + return false; + } + return true; } static int vhost_user_get_inflight_fd(struct vhost_dev *dev,
Memory regions must be mmap(MAP_SHARED) so that modifications made by the vhost device backend process are visible to QEMU and vice versa. Use the new memory_region_is_mapped_shared() function to check this and print a warning if guest RAM is not shared: qemu-system-x86_64: -device vhost-user-fs-pci,chardev=char0,tag=myfs: warning: Found vhost-user memory region without MAP_SHARED (did you forget -object memory-*,share=on?) qemu-system-x86_64: Failed to read from slave. This warning makes it clear that memory needs to be configured with share=on. Without this patch the vhost device is initialized and the device fails with vague error messages caused by inconsistent memory views. The warning should make it easier to troubleshoot QEMU command-lines that lack share=on memory. I couldn't figure out how to make this a vhost_dev_init() error, because memory regions aren't necessarily final when it is called. Also, hotplug can add memory regions without MAP_SHARED in the future, so we still need to warn about that. Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/virtio/vhost-user.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)