From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id D8EA2F72 for ; Mon, 21 Dec 2015 12:47:06 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP; 21 Dec 2015 03:46:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,459,1444719600"; d="scan'208";a="845711811" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by orsmga001.jf.intel.com with ESMTP; 21 Dec 2015 03:46:46 -0800 Received: from sivlogin002.ir.intel.com (sivlogin002.ir.intel.com [10.237.217.37]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id tBLBkiw0002459; Mon, 21 Dec 2015 11:46:44 GMT Received: from sivlogin002.ir.intel.com (localhost [127.0.0.1]) by sivlogin002.ir.intel.com with ESMTP id tBLBkiSj006393; Mon, 21 Dec 2015 11:46:44 GMT Received: (from fyigit@localhost) by sivlogin002.ir.intel.com with œ id tBLBkhXS006389; Mon, 21 Dec 2015 11:46:44 GMT X-Authentication-Warning: sivlogin002.ir.intel.com: fyigit set sender to ferruh.yigit@intel.com using -f Date: Mon, 21 Dec 2015 11:46:43 +0000 From: "Yigit, Ferruh" To: Alex Williamson Message-ID: <20151221114643.GA30129@sivlogin002.ir.intel.com> Mail-Followup-To: Alex Williamson , "Burakov, Anatoly" , "dev@dpdk.org" References: <1450198398.6042.32.camel@redhat.com> <20151216040408.GA18363@sivlogin002.ir.intel.com> <1450240711.2674.11.camel@redhat.com> <1450285912.2674.22.camel@redhat.com> <1450370639.2674.93.camel@redhat.com> <20151218104310.GA11371@sivlogin002.ir.intel.com> <1450449520.2674.162.camel@redhat.com> <1450475417.2674.167.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1450475417.2674.167.camel@redhat.com> User-Agent: Mutt/1.5.17 (2007-11-01) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] VFIO no-iommu X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Dec 2015 11:47:07 -0000 On Fri, Dec 18, 2015 at 02:50:17PM -0700, Alex Williamson wrote: > On Fri, 2015-12-18 at 07:38 -0700, Alex Williamson wrote: > > On Fri, 2015-12-18 at 10:43 +0000, Yigit, Ferruh wrote: > > > On Thu, Dec 17, 2015 at 09:43:59AM -0700, Alex Williamson wrote: > > > <...> > > > > > > > > > > > > > > > > > > Also I need to disable VFIO_CHECK_EXTENSION ioctl, > > > > > > > > > because in > > > > > > > > > vfio > > > > > > > > > module, > > > > > > > > > container->noiommu is not set before doing a > > > > > > > > > vfio_group_set_container() > > > > > > > > > and vfio_for_each_iommu_driver selects wrong driver. > > > > > > > > > > > > > > > > Running CHECK_EXTENSION on a container without the group > > > > > > > > attached is > > > > > > > > only going to tell you what extensions vfio is capable > > > > > > > > of, > > > > > > > > not > > > > > > > > necessarily what extensions are available to you with > > > > > > > > that > > > > > > > > group. > > > > > > > > Is this just a general dpdk- vfio ordering bug? > > > > > > > > > > > > > > Yes, that is how VFIO was implemented in DPDK. I was under > > > > > > > the > > > > > > > impression that checking extension before assigning devices > > > > > > > was > > > > > > > the > > > > > > > correct way to do things, so as to not to try anything we > > > > > > > know > > > > > > > would > > > > > > > fail anyway. Does this imply that CHECK_EXTENSION needs to > > > > > > > be > > > > > > > called > > > > > > > on both container and groups (or just on groups)? > > > > > > > > > > > > Hmm, in Documentation/vfio.txt we do give the following > > > > > > algorithm: > > > > > > > > > > > >         if (ioctl(container, VFIO_GET_API_VERSION) != > > > > > > VFIO_API_VERSION) > > > > > >                 /* Unknown API version */ > > > > > > > > > > > >         if (!ioctl(container, VFIO_CHECK_EXTENSION, > > > > > > VFIO_TYPE1_IOMMU)) > > > > > >                 /* Doesn't support the IOMMU driver we want. > > > > > > */ > > > > > >         ... > > > > > > > > > > > > That's just going to query each iommu driver and we can't yet > > > > > > say > > > > > > whether > > > > > > the group the user attaches to the container later will > > > > > > actually > > > > > > support that > > > > > > extension until we try to do it, that would come at > > > > > > VFIO_SET_IOMMU. > > > > > >  So is > > > > > > it perhaps a vfio bug that we're not advertising no-iommu > > > > > > until > > > > > > the > > > > > > group is > > > > > > attached?  After all, we are capable of it with just an empty > > > > > > container, just > > > > > > like we are with type1, but we're going to fail SET_IOMMU for > > > > > > the > > > > > > wrong > > > > > > combination. > > > > > >  This is exactly the sort of thing that makes me glad we > > > > > > reverted > > > > > > it without > > > > > > feedback from a working user driver.  Thanks, > > > > > > > > > > Whether it should be considered a "bug" in VFIO or "by design" > > > > > is > > > > > up > > > > > to you, of course, but at least according to the VFIO > > > > > documentation, > > > > > we are meant to check for type 1 extension and then attach > > > > > devices, > > > > > so it would be expected to get VFIO_NOIOMMU_IOMMU marked as > > > > > supported > > > > > even without any devices attached to the container (just like > > > > > we > > > > > get > > > > > type 1 as supported without any devices attached). Having said > > > > > that, > > > > > if it was meant to attach devices first and then check the > > > > > extensions, then perhaps the documentation should also point > > > > > out > > > > > that > > > > > fact (or perhaps I missed that detail in my readings of the > > > > > docs, > > > > > in > > > > > which case my apologies). > > > > > > > > Hi Anatoly, > > > > > > > > Does the below patch make it behave more like you'd expect.  This > > > > applies to v4.4-rc4, I'd fold this into the base patch if we > > > > reincorporate it to a future kernel.  Thanks, > > > > > > > > Alex > > > > > > > > commit 88d4dcb6b77624965f0b45b5cd305a2b4a105c94 > > > > Author: Alex Williamson > > > > Date:   Wed Dec 16 19:02:01 2015 -0700 > > > > > > > >     vfio: Fix no-iommu CHECK_EXTENSION > > > >      > > > >     Previously the no-iommu iommu driver was only visible when > > > > the > > > >     container had an attached no-iommu group.  This means that > > > >     CHECK_EXTENSION on and empty container couldn't report the > > > > possibility > > > >     of using VFIO_NOIOMMU_IOMMU.  We report TYPE1 whether or not > > > > the user > > > >     can make use of it with the group, so this is > > > > inconsistent.  Add the > > > >     no-iommu iommu to the list of iommu drivers when enabled via > > > > module > > > >     option, but skip all the others if the container is attached > > > > to > > > > a > > > >     no-iommu groups.  Note that tainting is now done with the > > > > "unsafe" > > > >     module callback rather than explictly within vfio. > > > >      > > > >     Also fixes module option and module description name > > > > inconsistency. > > > >      > > > >     Also make vfio_noiommu_ops const. > > > >      > > > >     Signed-off-by: Alex Williamson > > > > > > Hi Alex, > > > > > > I got following crash with this update: > > Let's try this one: > > commit 8ff839c6ffe9f3b50b50f1cc87e7afbf23171f05 > Author: Alex Williamson > Date:   Fri Dec 18 14:45:55 2015 -0700 > >     v2 vfio fix no-iommu CHECK_EXTENSION >      >     Register and unregister the no-iommu iommu backend at module >     initialization and exit, but disable it unless enabled via module >     option.  Rather than modify the iommu driver walk, selectively skip >     combinations that aren't supported.  CHECK_EXTENSION on a container >     without any groups attached exposes all possible extensions.  Once a >     group is attached, the no-iommu backend is skipped for regular groups >     and regular iommu backends are skipped for no-iommu groups. >      >     This would be folded into a single patch to re-propose vfio no-iommu >     mode upstream. >      >     Signed-off-by: Alex Williamson > Hi Alex, Thank you for the update. I have tested this on both no-iommu and iommu environment and worked successfully. I believe this approach is better because it is simpler. >>From DPDK point of view, only update to support vfio no-iommu is: to use new group names and disable DMA mapping. If VFIO module compiled with "CONFIG_VFIO_NOIOMMU=y" by default, that makes things easier for DPDK, in no-iommu environment inserting vfio module with proper parameter makes it available for DPDK. Thanks, ferruh