From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id AF07B1B2C8 for ; Mon, 6 Nov 2017 15:22:56 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2017 06:22:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,352,1505804400"; d="scan'208";a="918093590" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by FMSMGA003.fm.intel.com with ESMTP; 06 Nov 2017 06:22:54 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.67]) by IRSMSX103.ger.corp.intel.com ([169.254.3.49]) with mapi id 14.03.0319.002; Mon, 6 Nov 2017 14:22:53 +0000 From: "Kavanagh, Mark B" To: Maxime Coquelin , "dev@dpdk.org" , "yliu@fridaylinux.org" , "thomas@monjalon.net" , "ktraynor@redhat.com" Thread-Topic: [PATCH] vhost: disable reply-ack protocol feature if iommu feature disabled Thread-Index: AQHTVM1LJDJfrmsbb0yv5Oi09LuuOaMHZ/Lw Date: Mon, 6 Nov 2017 14:22:52 +0000 Message-ID: References: <20171103175735.24603-1-maxime.coquelin@redhat.com> In-Reply-To: <20171103175735.24603-1-maxime.coquelin@redhat.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODY0MjQ5Y2MtNjJlNi00ZmRkLTliYTItYWFlYTJmOTFhZjk0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJsNFY4b1dVNk1WS1B0UEJZaVR4Tzgxa2YzRThIUEFMeUdqbXBNdTlpdGU0dkxCbENySTFyYVNpZjN2THZJK3d0In0= dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] vhost: disable reply-ack protocol feature if iommu feature disabled X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Nov 2017 14:22:57 -0000 >From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >Sent: Friday, November 3, 2017 5:58 PM >To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B >; thomas@monjalon.net; ktraynor@redhat.com >Cc: Maxime Coquelin >Subject: [PATCH] vhost: disable reply-ack protocol feature if iommu featur= e >disabled > >If the application has disabled VIRTIO_F_IOMMU_PLATFORM, disable >VHOST_USER_PROTOCOL_F_REPLY_ACK protocol feature that is only >mandatory with IOMMU for now. > >This is done to provide a way for the application to support >multiqueue with old Qemu versions (v2.7.0 to v2.9.0) that have >reply-ack feature broken. > >Signed-off-by: Maxime Coquelin >--- > >This is an alternative to my proposition of adding a new flag at >vhost register time. Advantage of this solution is that it does >not bring API change. > > lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > >diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >index 1f6cba4b9..e35218688 100644 >--- a/lib/librte_vhost/vhost_user.c >+++ b/lib/librte_vhost/vhost_user.c >@@ -878,6 +878,27 @@ vhost_user_set_vring_enable(struct virtio_net **pdev, > } > > static void >+vhost_user_get_protocol_features(struct virtio_net *dev, >+ struct VhostUserMsg *msg) >+{ >+ uint64_t features, protocol_features =3D VHOST_USER_PROTOCOL_FEATURES; >+ >+ rte_vhost_driver_get_features(dev->ifname, &features); >+ >+ /* >+ * REPLY_ACK protocol feature is only mandatory for now >+ * for IOMMU feature. If IOMMU is explicitly disabled by the >+ * application, disable also REPLY_ACK feature for older buggy >+ * Qemu versions (from v2.7.0 to v2.9.0). >+ */ >+ if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) Hi Maxime, Thanks for this patch - I like this approach, as it maintains API compatibi= lity. Now for the gotchas, unfortunately: - VIRTIO_F_IOMMU_PLATFORM is defined in vhost.h, as opposed to rte_vhost.h,= so it is not exposed to OvS :( - Currently, we use other similar macros in OvS (VIRTIO_NET_F_CSUM, VIRTIO_= NET_F_HOST_TSO[4|6]); however, we obtain the definition of these from a ker= nel header file, as opposed to a DPDK header (linux/virtio_net.h) -- We could adopt the same approach for VIRTIO_F_IOMMU_PLATFORM, and inclu= de linux/virtio_config.h; unfortunately, the VIRTIO_F_IOMMU_PLATFORM macro = is only defined from kernel 4.8, which creates another problem entirely. One potential solution is to move the VIRTIO_* definitions to rte_vhost.h, = but at this stage in the DPDK release cycle, that's probably a tall ask. Any thoughts? Thanks again, Mark >+ protocol_features &=3D ~(1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK); >+ >+ msg->payload.u64 =3D protocol_features; >+ msg->size =3D sizeof(msg->payload.u64); >+} >+ >+static void > vhost_user_set_protocol_features(struct virtio_net *dev, > uint64_t protocol_features) > { >@@ -1248,8 +1269,7 @@ vhost_user_msg_handler(int vid, int fd) > break; > > case VHOST_USER_GET_PROTOCOL_FEATURES: >- msg.payload.u64 =3D VHOST_USER_PROTOCOL_FEATURES; >- msg.size =3D sizeof(msg.payload.u64); >+ vhost_user_get_protocol_features(dev, &msg); > send_vhost_reply(fd, &msg); > break; > case VHOST_USER_SET_PROTOCOL_FEATURES: >-- >2.13.6