On Mon, Feb 22, 2021 at 8:11 PM Stefan Hajnoczi <stefa...@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 <kw...@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@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
>
>
