From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 5D891A0487
	for <public@inbox.dpdk.org>; Thu,  4 Jul 2019 12:47:52 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 332A01BE09;
	Thu,  4 Jul 2019 12:47:52 +0200 (CEST)
Received: from mail-vk1-f193.google.com (mail-vk1-f193.google.com
 [209.85.221.193]) by dpdk.org (Postfix) with ESMTP id 7E0561BE06
 for <dev@dpdk.org>; Thu,  4 Jul 2019 12:47:50 +0200 (CEST)
Received: by mail-vk1-f193.google.com with SMTP id y130so541301vkc.0
 for <dev@dpdk.org>; Thu, 04 Jul 2019 03:47:50 -0700 (PDT)
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=tpDEDibudS346dSSHzNPjeLa7/QIyyvXZSHtZPpZWLo=;
 b=du17qlUhsZuVzrrw0KbVC0EdGPD5YDgtpG/w/gSxbHDua1I5dDcJB0apjw+2yDl+bM
 ga/HCMOy8tM7pSVQ2rlZpgEXyz2HNx2WrFRYZycyya2WNXJ2+ZAoUOEpG6+WHTpsKp/P
 1wTallFlr1m0V8b0NczVnBYpr3/PdnATm0X1CV8PLvhZxueic7uzGFYZtMU8ob7xUFjq
 P5UPHZHEEoNhZSjfhy+S7YV7thRsFGPdKZkvIfqvZKVnIxItABBA5nM53oepDx5aNFse
 BI27kIYBiBleLEtqmhF7gqh1pJxDDKaQEV7Vz95k5nAoxThp8qZfWmlE5wWr5SatsXbh
 1N6A==
X-Gm-Message-State: APjAAAUVXlc2bvb0E1ADKfecgDogJnap2KvQ3tLsKyxJ/+WI3S0Q3z2J
 iPDxErcyoLR/xs5gSfExhjCNTecuvuHmnTm54BYNqg==
X-Google-Smtp-Source: APXvYqzwFue/c8iklrm9i0ej4OLnAGEwuUtyAj1yjj2QTw9xgsReM17NxkvEVhdVlFf3Km83PBKB/0c+C1rhs/t3bEY=
X-Received: by 2002:a1f:50c1:: with SMTP id e184mr12882124vkb.86.1562237269847; 
 Thu, 04 Jul 2019 03:47:49 -0700 (PDT)
MIME-Version: 1.0
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>
 <CAJFAV8yztKRupkkc8QUT5FstkwNuaRL46Lddgr8TjpKgH1hztA@mail.gmail.com>
 <7bafa053-2da9-e128-44a4-12b4b8cb0b93@intel.com>
In-Reply-To: <7bafa053-2da9-e128-44a4-12b4b8cb0b93@intel.com>
From: David Marchand <david.marchand@redhat.com>
Date: Thu, 4 Jul 2019 12:47:38 +0200
Message-ID: <CAJFAV8xpBUVPX56B5SReG0Kt3QJd_pg0om2=rmdx2Jo3wwMPQA@mail.gmail.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Cc: dev <dev@dpdk.org>, Ben Walker <benjamin.walker@intel.com>, 
 Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
 Maxime Coquelin <maxime.coquelin@redhat.com>, 
 Thomas Monjalon <thomas@monjalon.net>
Content-Type: text/plain; charset="UTF-8"
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Jul 4, 2019 at 12:44 PM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 04-Jul-19 10:18 AM, David Marchand wrote:
> >
> >
> > On Wed, Jul 3, 2019 at 12:45 PM Burakov, Anatoly
> > <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
> >
> >     On 14-Jun-19 10:39 AM, David Marchand wrote:
> >      > From: Ben Walker <benjamin.walker@intel.com
> >     <mailto:benjamin.walker@intel.com>>
> >      >
> >      > 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 <benjamin.walker@intel.com
> >     <mailto:benjamin.walker@intel.com>>
> >      > Signed-off-by: David Marchand <david.marchand@redhat.com
> >     <mailto:david.marchand@redhat.com>>
> >      > ---
> >
> >     <snip>
> >
> >      > +                  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 <anatoly.burakov@intel.com>
>

Cool, thanks Anatoly!


-- 
David Marchand