From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id E98D658F6 for ; Wed, 26 Apr 2017 10:27:27 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga105.fm.intel.com with ESMTP; 26 Apr 2017 01:27:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,253,1488873600"; d="scan'208";a="79027913" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by orsmga002.jf.intel.com with ESMTP; 26 Apr 2017 01:27:25 -0700 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.12]) by IRSMSX106.ger.corp.intel.com ([169.254.8.202]) with mapi id 14.03.0319.002; Wed, 26 Apr 2017 09:27:24 +0100 From: "Burakov, Anatoly" To: Alexey Kardashevskiy , Andrew Rybchenko , "dev@dpdk.org" CC: "JPF@zurich.ibm.com" , Gowrishankar Muthukrishnan , Alejandro Lucero Thread-Topic: [dpdk-dev] [PATCH dpdk 4/5] vfio: Do try setting IOMMU type if already set Thread-Index: AQHSunzza/DkJu1gcUqgNgvg2IZcvqHXPeIAgAAZ0cA= Date: Wed, 26 Apr 2017 08:27:23 +0000 Message-ID: References: <20170420072402.38106-1-aik@ozlabs.ru> <20170420072402.38106-5-aik@ozlabs.ru> <5b5fe4e4-af39-4c27-fb3c-aff0c601b938@solarflare.com> <9808fd9e-17e5-b873-592a-2309579097dc@ozlabs.ru> In-Reply-To: <9808fd9e-17e5-b873-592a-2309579097dc@ozlabs.ru> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_PUBLIC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmZkOTRmMGEtYTI3Yy00NWY0LTkyOTYtYzk1ODI4MTdkZmY4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX1BVQkxJQyJ9XX1dfSwiU3ViamVjdExhYmVscyI6W10sIlRNQ1ZlcnNpb24iOiIxNi41LjkuMyIsIlRydXN0ZWRMYWJlbEhhc2giOiJLMkVGbytUZitCcDlOcUxkc0l3T0ZSRzBzdkFtbVkrMithRUNPc2dmSkJjPSJ9 dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH dpdk 4/5] vfio: Do try setting IOMMU type if already set 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: Wed, 26 Apr 2017 08:27:28 -0000 > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alexey > Kardashevskiy > Sent: Wednesday, April 26, 2017 8:51 AM > To: Andrew Rybchenko ; dev@dpdk.org > Cc: JPF@zurich.ibm.com; Gowrishankar Muthukrishnan > > Subject: Re: [dpdk-dev] [PATCH dpdk 4/5] vfio: Do try setting IOMMU type = if > already set >=20 > On 21/04/17 18:54, Andrew Rybchenko wrote: > > On 04/20/2017 10:24 AM, Alexey Kardashevskiy wrote: > >> The existing code correctly checks if a container is set to a group > >> and does not try attaching a group to a container for a second/third/.= .. > >> device from the same IOMMU group. > >> > >> However it still tries to set IOMMU type to a container for every > >> device in a group which produces failure and closes container and grou= p > fds. > >> > >> This moves vfio_set_iommu_type() and DMA mapping code under: > >> if (!(group_status.flags & VFIO_GROUP_FLAGS_CONTAINER_SET)) > >> > >> Signed-off-by: Alexey Kardashevskiy > >> --- > >> lib/librte_eal/linuxapp/eal/eal_vfio.c | 50 > >> +++++++++++++++++----------------- > >> 1 file changed, 25 insertions(+), 25 deletions(-) > >> > >> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c > >> b/lib/librte_eal/linuxapp/eal/eal_vfio.c > >> index 6e2e84ca7..46f951f4d 100644 > >> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c > >> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c > >> @@ -298,33 +298,33 @@ vfio_setup_device(const char *sysfs_base, > const > >> char *dev_addr, > >> clear_group(vfio_group_fd); > >> return -1; > >> } > >> - } > >> - /* > >> - * pick an IOMMU type and set up DMA mappings for container > >> - * > >> - * needs to be done only once, only when first group is assigned = to > >> - * a container and only in primary process. Note this can happen > >> several > >> - * times with the hotplug functionality. > >> - */ > >> - if (internal_config.process_type =3D=3D RTE_PROC_PRIMARY && > >> - vfio_cfg.vfio_active_groups =3D=3D 1) { > >> - /* select an IOMMU type which we will be using */ > >> - const struct vfio_iommu_type *t =3D > >> + /* > >> + * pick an IOMMU type and set up DMA mappings for container > >> + * > >> + * needs to be done only once, only when first group is assig= ned to > >> + * a container and only in primary process. Note this can > >> + happen > >> several > >> + * times with the hotplug functionality. > >> + */ > >> + if (internal_config.process_type =3D=3D RTE_PROC_PRIMARY && > >> + vfio_cfg.vfio_active_groups =3D=3D 1) { > >> + /* select an IOMMU type which we will be using */ > >> + const struct vfio_iommu_type *t =3D > >> vfio_set_iommu_type(vfio_cfg.vfio_container_fd); > >> - if (!t) { > >> - RTE_LOG(ERR, EAL, " %s failed to select IOMMU type\n", > >> dev_addr); > >> - close(vfio_group_fd); > >> - clear_group(vfio_group_fd); > >> - return -1; > >> - } > >> - ret =3D t->dma_map_func(vfio_cfg.vfio_container_fd); > >> - if (ret) { > >> - RTE_LOG(ERR, EAL, " %s DMA remapping failed, " > >> - "error %i (%s)\n", dev_addr, errno, strerror(errn= o)); > >> - close(vfio_group_fd); > >> - clear_group(vfio_group_fd); > >> - return -1; > >> + if (!t) { > >> + RTE_LOG(ERR, EAL, " %s failed to select IOMMU > >> + type\n", > >> dev_addr); > >> + close(vfio_group_fd); > >> + clear_group(vfio_group_fd); > >> + return -1; > >> + } > >> + ret =3D t->dma_map_func(vfio_cfg.vfio_container_fd); > >> + if (ret) { > >> + RTE_LOG(ERR, EAL, " %s DMA remapping failed, " > >> + "error %i (%s)\n", dev_addr, errno, > >> strerror(errno)); > >> + close(vfio_group_fd); > >> + clear_group(vfio_group_fd); > >> + return -1; > >> + } > >> } > >> } > > > > It looks like a duplicate of the earlier submitted patch > > http://dpdk.org/ml/archives/dev/2017-April/063077.html >=20 >=20 > It is a duplicate indeed, what needs to be done to have it accepted in th= e > mainline tree? I think Alejandro Lucero (CC'd) mentioned that he has another patch for sim= ilar issue that he's working on (for ports being assigned to the same IOMMU group). That patch would presumably fix this iss= ue as well, so I'm inclined to wait for that one. If that's not happening, then we can accept either this = one or the earlier ones, and fix the remaining issues (I have a setup where I can attempt to reproduce the issue= , so that should be quick enough). Thanks, Anatoly=20