From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B47A1A0487 for ; Thu, 4 Jul 2019 12:43:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8B8E31BDFA; Thu, 4 Jul 2019 12:43:44 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 4F0E91BCB8 for ; Thu, 4 Jul 2019 12:43:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jul 2019 03:43:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,450,1557212400"; d="scan'208";a="172398186" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.126]) ([10.237.220.126]) by FMSMGA003.fm.intel.com with ESMTP; 04 Jul 2019 03:43:39 -0700 To: David Marchand Cc: dev , Ben Walker , Jerin Jacob Kollanukkaran , Maxime Coquelin , Thomas Monjalon References: <20190530174819.1160221-1-benjamin.walker@intel.com> <1560505157-9769-1-git-send-email-david.marchand@redhat.com> <1560505157-9769-4-git-send-email-david.marchand@redhat.com> <4fabfd5f-2ba9-ff45-59dd-cfd01b8d49d5@intel.com> From: "Burakov, Anatoly" Message-ID: <7bafa053-2da9-e128-44a4-12b4b8cb0b93@intel.com> Date: Thu, 4 Jul 2019 11:43:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 3/3] bus/pci: only consider usable devices to select IOVA mode 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 04-Jul-19 10:18 AM, David Marchand wrote: > > > On Wed, Jul 3, 2019 at 12:45 PM Burakov, Anatoly > > wrote: > > On 14-Jun-19 10:39 AM, David Marchand wrote: > > From: Ben Walker > > > > > When selecting the preferred IOVA mode of the pci bus, the current > > heuristic ("are devices bound?", "are devices bound to UIO?", > "are pmd > > drivers supporting IOVA as VA?" etc..) should honor the device > > white/blacklist so that an unwanted device does not impact the > decision. > > > > There is no reason to consider a device which has no driver > available. > > > > This applies to all OS, so implements this in common code then call a > > OS specific callback. > > > > On Linux side: > > - the VFIO special considerations should be evaluated only if VFIO > >    support is built, > > - there is no strong requirement on using VA rather than PA if a > driver > >    supports VA, so defaulting to DC in such a case. > > > > Signed-off-by: Ben Walker > > > Signed-off-by: David Marchand > > > --- > > > > > +                  const struct rte_pci_device *pdev) > >   { > > -     struct rte_pci_device *dev = NULL; > > -     struct rte_pci_driver *drv = NULL; > > +     enum rte_iova_mode iova_mode = RTE_IOVA_DC; > > +     static int iommu_no_va = -1; > > > > -     FOREACH_DRIVER_ON_PCIBUS(drv) { > > -             FOREACH_DEVICE_ON_PCIBUS(dev) { > > -                     if (!rte_pci_match(drv, dev)) > > -                             continue; > > -                     /* > > -                      * just one PCI device needs to be checked > out because > > -                      * the IOMMU hardware is the same for all > of them. > > -                      */ > > -                     return pci_one_device_iommu_support_va(dev); > > +     switch (pdev->kdrv) { > > +     case RTE_KDRV_VFIO: { > > +#ifdef VFIO_PRESENT > > +             static int is_vfio_noiommu_enabled = -1; > > + > > +             if (is_vfio_noiommu_enabled == -1) { > > +                     if (rte_vfio_noiommu_is_enabled() == 1) > > +                             is_vfio_noiommu_enabled = 1; > > +                     else > > +                             is_vfio_noiommu_enabled = 0; > > +             } > > +             if ((pdrv->drv_flags & RTE_PCI_DRV_IOVA_AS_VA) == 0) { > > +                     iova_mode = RTE_IOVA_PA; > > +             } else if (is_vfio_noiommu_enabled != 0) { > > +                     RTE_LOG(DEBUG, EAL, "Forcing to 'PA', > vfio-noiommu mode configured\n"); > > +                     iova_mode = RTE_IOVA_PA; > >               } > > +#endif > > +             break; > > I'm not too well-versed in bus code, so please excuse my ignorance of > this codebase. > > It seems that we would be ignoring drv_flags in case VFIO wasn't > compiled - if the driver has no RTE_PCI_DRV_IOVA_AS_VA flag, i'm pretty > sure we can set IOVA mode to PA without caring about VFIO at all. I > think it would be better to have something like this: > > if ((pdrv->drv_flags & RTE_PCI_DRV_IOVA_AS_VA) == 0) { >         iova_mode = RTE_IOVA_PA; >         break; // early exit > } > > > If the device is bound to VFIO, but the dpdk binary has no vfio support, > we don't need to consider this device in the decision. > Did I miss something in what you suggest? > Yep, you're correct :) Reviewed-by: Anatoly Burakov > > -- > David Marchand -- Thanks, Anatoly