qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Laurent Vivier" <lvivier@redhat.com>,
	kwolf@redhat.com, "Thomas Huth" <thuth@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: [PATCH v3 3/3] vhost-user: warn when guest RAM is not shared
Date: Wed, 14 Jul 2021 10:29:46 +0100	[thread overview]
Message-ID: <20210714092946.569516-4-stefanha@redhat.com> (raw)
In-Reply-To: <20210714092946.569516-1-stefanha@redhat.com>

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>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@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 29ea2b4fce..3978292514 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2295,11 +2295,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.31.1


  parent reply	other threads:[~2021-07-14  9:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  9:29 [PATCH v3 0/3] vhost-user: warn when guest RAM is not shared Stefan Hajnoczi
2021-07-14  9:29 ` [PATCH v3 1/3] tests/qtest/vhost-user-test: use share=on with memfd Stefan Hajnoczi
2021-07-14 10:14   ` Pankaj Gupta
2021-07-14  9:29 ` [PATCH v3 2/3] memory: add memory_region_is_mapped_shared() Stefan Hajnoczi
2021-07-14 10:12   ` Pankaj Gupta
2021-07-14  9:29 ` Stefan Hajnoczi [this message]
2021-07-20 11:32 ` [PATCH v3 0/3] vhost-user: warn when guest RAM is not shared Kevin Wolf
2021-07-20 13:06   ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210714092946.569516-4-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).