From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14511C433DB for ; Wed, 24 Feb 2021 10:39:58 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 57A6964EC9 for ; Wed, 24 Feb 2021 10:39:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57A6964EC9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:39924 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lEraG-0003o4-4P for qemu-devel@archiver.kernel.org; Wed, 24 Feb 2021 05:39:56 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:39622) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lErZI-0003Ny-UA for qemu-devel@nongnu.org; Wed, 24 Feb 2021 05:38:56 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:43983) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lErZE-0003Yn-Dx for qemu-devel@nongnu.org; Wed, 24 Feb 2021 05:38:56 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1614163131; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=C50vrkTaX6k6gco3GBAYudA3yXty6RcSYIpD3hSWnu8=; b=Rs2aAwb9kYTQGpEhhO3JJQxVUNeS3BJ1WZy4oVhfh/mnSUAO3oVDQqWFvN9SYYiKUhxUt3 EkKCEAu7QgKqvzg3QGwfHGVO/en4COduPZxOxjrVzrXSPRECXyXi4J5CGgqCj2OZLaQmhL X8tXz2EnDAPy4counmltp6m9gpFsjqQ= Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-211-fICGEwksOgSHUcrwQOiD8Q-1; Wed, 24 Feb 2021 05:38:49 -0500 X-MC-Unique: fICGEwksOgSHUcrwQOiD8Q-1 Received: by mail-io1-f72.google.com with SMTP id c4so1097817ioq.15 for ; Wed, 24 Feb 2021 02:38:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=C50vrkTaX6k6gco3GBAYudA3yXty6RcSYIpD3hSWnu8=; b=XDgxfcreCTRHz4RYD5lU/NSfQ5cNieyFi9rovAC832rB08j0+EivOtzo/5mpYJX+EJ Pnvw/9Rb2ZJHzOS+4rS/76/4U+tJY6qQblm1+wle/CqpZeZpgOTmbacTZd6X+Rj5+b1Y VFtA9x1zNMHGaeoSUOFZKiwMKR+tsHVFtdb3wtvSsriLTv3AIELHgsl1Zt/PL5iAnvdy smB9ngSMHvz0rk2WB1W8An9/WKhx5hoXt+LZ9nkGQLMVSNzNnoB05OaQTRhYdb8apBuQ fZSKN9SOeY78OJYOtyTga3X6NnMHtezT0ZIUCWd2gbLDitNOrXgS583oWsdsobZ8MGHB fffg== X-Gm-Message-State: AOAM533A+SdhE8xMO036kemSNKheazgcFB07q6pQFY4TXF76u5OphuFa uP1mgk+bu2t+tPxOZkL7lGwThl4bmsJD6BH3IDMPPVGuMx4WMlRYiqVM9JZbcOW+GLbn9k3Z82b yRf9IHunyxse11xCz//1yL8bgpvqQbhg= X-Received: by 2002:a05:6e02:194a:: with SMTP id x10mr23412401ilu.165.1614163128874; Wed, 24 Feb 2021 02:38:48 -0800 (PST) X-Google-Smtp-Source: ABdhPJx8LjYSqVop7Fy2JlpdCFpK7MIVGFatuJBJgrsYMd7aBI+yTx0jhqleyzMqDQNqH/5PF4YuQLd4tGsOBpe27VQ= X-Received: by 2002:a05:6e02:194a:: with SMTP id x10mr23412392ilu.165.1614163128744; Wed, 24 Feb 2021 02:38:48 -0800 (PST) MIME-Version: 1.0 References: <20210222161017.570837-1-stefanha@redhat.com> <20210222161017.570837-4-stefanha@redhat.com> In-Reply-To: <20210222161017.570837-4-stefanha@redhat.com> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Wed, 24 Feb 2021 14:38:37 +0400 Message-ID: Subject: Re: [PATCH 3/3] vhost-user: warn when guest RAM is not shared To: Stefan Hajnoczi Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=mlureau@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="000000000000e6177405bc12a3cd" Received-SPF: pass client-ip=216.205.24.124; envelope-from=mlureau@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Wolf, Kevin" , Laurent Vivier , Thomas Huth , "Michael S. Tsirkin" , qemu-devel , Paolo Bonzini Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000e6177405bc12a3cd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Feb 22, 2021 at 8:11 PM Stefan Hajnoczi 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=3Dchar0,tag=3Dmyf= s: > warning: Found vhost-user memory region without MAP_SHARED (did you forge= t > -object memory-*,share=3Don?) > qemu-system-x86_64: Failed to read from slave. > > This warning makes it clear that memory needs to be configured with > share=3Don. 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=3Don 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 > Signed-off-by: Stefan Hajnoczi > Reviewed-by: Marc-Andr=C3=A9 Lureau --- > 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 =3D memory_region_get_fd(section->mr) >=3D 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=3Don?)"); > + return false; > + } > + return true; > } > > static int vhost_user_get_inflight_fd(struct vhost_dev *dev, > -- > 2.29.2 > > --000000000000e6177405bc12a3cd Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Feb 22, 2021 at 8:11 PM Stefa= n Hajnoczi <stefanha@redhat.com> wrote:
Me= mory 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:

=C2=A0 qemu-system-x86_64: -device vhost-user-fs-pci,chardev=3Dchar0,tag=3D= myfs: warning: Found vhost-user memory region without MAP_SHARED (did you f= orget -object memory-*,share=3Don?)
=C2=A0 qemu-system-x86_64: Failed to read from slave.

This warning makes it clear that memory needs to be configured with
share=3Don. 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=3Don memory.

I couldn't figure out how to make this a vhost_dev_init() error, becaus= e
memory regions aren't necessarily final when it is called. Also, hotplu= g
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=C3=A9 Lureau <marcandre.lureau@redhat.com>=C2=A0
=
---
=C2=A0hw/virtio/vhost-user.c | 20 ++++++++++++++++----
=C2=A01 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 *d= ev, uint64_t session_id)
=C2=A0static bool vhost_user_mem_section_filter(struct vhost_dev *dev,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0MemoryRegionSection *section)
=C2=A0{
-=C2=A0 =C2=A0 bool result;
+=C2=A0 =C2=A0 /* An fd is required so we can pass it to the device backend= process */
+=C2=A0 =C2=A0 if (memory_region_get_fd(section->mr) < 0) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
+=C2=A0 =C2=A0 }

-=C2=A0 =C2=A0 result =3D memory_region_get_fd(section->mr) >=3D 0; -
-=C2=A0 =C2=A0 return result;
+=C2=A0 =C2=A0 /*
+=C2=A0 =C2=A0 =C2=A0* It must be mmap(MAP_SHARED) so memory stores from th= e device backend
+=C2=A0 =C2=A0 =C2=A0* process are visible to us and vice versa.
+=C2=A0 =C2=A0 =C2=A0*/
+=C2=A0 =C2=A0 if (!memory_region_is_mapped_shared(section->mr)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 static bool warned;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 warn_report_once_cond(&warned, "Found= vhost-user memory region "
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "without MAP_= SHARED (did you forget -object "
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "memory-*,sha= re=3Don?)");
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
+=C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 return true;
=C2=A0}

=C2=A0static int vhost_user_get_inflight_fd(struct vhost_dev *dev,
--
2.29.2

--000000000000e6177405bc12a3cd--