Re: [dpdk-dev] [PATCH] bus/pci: always check IOMMU capabilities

[David Marchand <david.marchand@redhat.com>]

On Mon, Aug 5, 2019 at 5:57 AM Takeshi T Yoshimura <TYOS@jp.ibm.com> wrote:
>
> -----David Marchand <david.marchand@redhat.com> wrote: -----
>
> >To: dev@dpdk.org
> >From: David Marchand <david.marchand@redhat.com>
> >Date: 08/02/2019 07:14PM
> >Cc: anatoly.burakov@intel.com, tyos@jp.ibm.com,
> >drc@linux.vnet.ibm.com
> >Subject: [PATCH] bus/pci: always check IOMMU capabilities
> >
> >IOMMU capabilities won't change and must be checked even if no PCI
> >device
> >seem to be supported yet when EAL initialised.
> >
> >This is to accommodate with SPDK that registers its drivers after
> >rte_eal_init(), especially on PPC platform where the IOMMU does not
> >support VA.
> >
> >Fixes: 703458e19c16 ("bus/pci: consider only usable devices for IOVA
> >mode")
> >
> >Signed-off-by: David Marchand <david.marchand@redhat.com>
> >---
> > drivers/bus/pci/bsd/pci.c    |  6 ++++++
> > drivers/bus/pci/linux/pci.c  | 25 ++++++-------------------
> > drivers/bus/pci/pci_common.c | 16 +++++++++++++++-
> > drivers/bus/pci/private.h    |  5 ++++-
> > 4 files changed, 31 insertions(+), 21 deletions(-)
> >
> >diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
> >index a2de709..8f07ed9 100644
> >--- a/drivers/bus/pci/bsd/pci.c
> >+++ b/drivers/bus/pci/bsd/pci.c
> >@@ -376,6 +376,12 @@ error:
> >       return -1;
> > }
> >
> >+bool
> >+pci_device_iommu_support_va(__rte_unused const struct rte_pci_device
> >*dev)
> >+{
> >+      return false;
> >+}
> >+
> > enum rte_iova_mode
> > pci_device_iova_mode(const struct rte_pci_driver *pdrv __rte_unused,
> >                    const struct rte_pci_device *pdev)
> >diff --git a/drivers/bus/pci/linux/pci.c
> >b/drivers/bus/pci/linux/pci.c
> >index f4fb742..43debaa 100644
> >--- a/drivers/bus/pci/linux/pci.c
> >+++ b/drivers/bus/pci/linux/pci.c
> >@@ -498,8 +498,8 @@ error:
> > }
> >
> > #if defined(RTE_ARCH_X86)
> >-static bool
> >-pci_one_device_iommu_support_va(const struct rte_pci_device *dev)
> >+bool
> >+pci_device_iommu_support_va(const struct rte_pci_device *dev)
> > {
> > #define VTD_CAP_MGAW_SHIFT    16
> > #define VTD_CAP_MGAW_MASK     (0x3fULL << VTD_CAP_MGAW_SHIFT)
> >@@ -546,14 +546,14 @@ pci_one_device_iommu_support_va(const struct
> >rte_pci_device *dev)
> >       return true;
> > }
> > #elif defined(RTE_ARCH_PPC_64)
> >-static bool
> >-pci_one_device_iommu_support_va(__rte_unused const struct
> >rte_pci_device *dev)
> >+bool
> >+pci_device_iommu_support_va(__rte_unused const struct rte_pci_device
> >*dev)
> > {
> >       return false;
> > }
> > #else
> >-static bool
> >-pci_one_device_iommu_support_va(__rte_unused const struct
> >rte_pci_device *dev)
> >+bool
> >+pci_device_iommu_support_va(__rte_unused const struct rte_pci_device
> >*dev)
> > {
> >       return true;
> > }
> >@@ -564,7 +564,6 @@ pci_device_iova_mode(const struct rte_pci_driver
> >*pdrv,
> >                    const struct rte_pci_device *pdev)
> > {
> >       enum rte_iova_mode iova_mode = RTE_IOVA_DC;
> >-      static int iommu_no_va = -1;
> >
> >       switch (pdev->kdrv) {
> >       case RTE_KDRV_VFIO: {
> >@@ -595,18 +594,6 @@ pci_device_iova_mode(const struct rte_pci_driver
> >*pdrv,
> >                       iova_mode = RTE_IOVA_VA;
> >               break;
> >       }
> >-
> >-      if (iova_mode != RTE_IOVA_PA) {
> >-              /*
> >-               * We can check this only once, because the IOMMU hardware is
> >-               * the same for all of them.
> >-               */
> >-              if (iommu_no_va == -1)
> >-                      iommu_no_va = pci_one_device_iommu_support_va(pdev)
> >-                                      ? 0 : 1;
> >-              if (iommu_no_va != 0)
> >-                      iova_mode = RTE_IOVA_PA;
> >-      }
> >       return iova_mode;
> > }
> >
> >diff --git a/drivers/bus/pci/pci_common.c
> >b/drivers/bus/pci/pci_common.c
> >index 9794552..8d1d6ab 100644
> >--- a/drivers/bus/pci/pci_common.c
> >+++ b/drivers/bus/pci/pci_common.c
> >@@ -616,8 +616,16 @@ rte_pci_get_iommu_class(void)
> >       const struct rte_pci_driver *drv;
> >       bool devices_want_va = false;
> >       bool devices_want_pa = false;
> >+      static int iommu_no_va = -1;
> >
> >       FOREACH_DEVICE_ON_PCIBUS(dev) {
> >+              /*
> >+               * We can check this only once, because the IOMMU hardware is
> >+               * the same for all of them.
> >+               */
> >+              if (iommu_no_va == -1)
> >+                      iommu_no_va = pci_device_iommu_support_va(dev)
> >+                                      ? 0 : 1;
> >               if (pci_ignore_device(dev))
> >                       continue;
> >               if (dev->kdrv == RTE_KDRV_UNKNOWN ||
> >@@ -643,7 +651,13 @@ rte_pci_get_iommu_class(void)
> >                               devices_want_va = true;
> >               }
> >       }
> >-      if (devices_want_va && !devices_want_pa) {
> >+      if (iommu_no_va == 1) {
> >+              iova_mode = RTE_IOVA_PA;
> >+              if (devices_want_va) {
> >+                      RTE_LOG(WARNING, EAL, "Some devices want 'VA' but because IOMMU
> >does not support 'VA'.\n");
> >+                      RTE_LOG(WARNING, EAL, "The devices that want 'VA' won't
> >initialize.\n");
> >+              }
> >+      } else if (devices_want_va && !devices_want_pa) {
> >               iova_mode = RTE_IOVA_VA;
> >       } else if (devices_want_pa && !devices_want_va) {
> >               iova_mode = RTE_IOVA_PA;
> >diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
> >index 8a55240..a205d4d 100644
> >--- a/drivers/bus/pci/private.h
> >+++ b/drivers/bus/pci/private.h
> >@@ -173,9 +173,12 @@ rte_pci_match(const struct rte_pci_driver
> >*pci_drv,
> >             const struct rte_pci_device *pci_dev);
> >
> > /**
> >- * OS specific callback for rte_pci_get_iommu_class
> >+ * OS specific callbacks for rte_pci_get_iommu_class
> >  *
> >  */
> >+bool
> >+pci_device_iommu_support_va(const struct rte_pci_device *dev);
> >+
> > enum rte_iova_mode
> > pci_device_iova_mode(const struct rte_pci_driver *pci_drv,
> >                    const struct rte_pci_device *pci_dev);
> >--
> >1.8.3.1
> >
> >
>
> Tested-by: Takeshi Yoshimura <tyos@jp.ibm.com>

Thank you all.
I'll send a v2 with the comment from Jerin and the log message fix.


-- 
David Marchand