On Mon, Aug 5, 2019 at 5:57 AM Takeshi T Yoshimura <t...@jp.ibm.com> wrote:
>
> -----David Marchand <david.march...@redhat.com> wrote: -----
>
> >To: dev@dpdk.org
> >From: David Marchand <david.march...@redhat.com>
> >Date: 08/02/2019 07:14PM
> >Cc: anatoly.bura...@intel.com, t...@jp.ibm.com,
> >d...@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.march...@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 <t...@jp.ibm.com>
Thank you all.
I'll send a v2 with the comment from Jerin and the log message fix.
--
David Marchand
