[dpdk-dev] [PATCH v7 3/9] linuxapp/eal_pci: get iommu class

[Burakov, Anatoly]

> From: santosh [mailto:santosh.shukla at caviumnetworks.com]
> Sent: Tuesday, September 5, 2017 9:48 AM
> To: Burakov, Anatoly <anatoly.burakov at intel.com>; dev at dpdk.org
> Cc: thomas at monjalon.net; jerin.jacob at caviumnetworks.com;
> hemant.agrawal at nxp.com; olivier.matz at 6wind.com;
> maxime.coquelin at redhat.com; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy at intel.com>; Richardson, Bruce
> <bruce.richardson at intel.com>; shreyansh.jain at nxp.com;
> gaetan.rivet at 6wind.com; stephen at networkplumber.org;
> aconole at redhat.com
> Subject: Re: [PATCH v7 3/9] linuxapp/eal_pci: get iommu class
> 
> Hi Anatoly,
> 
> 
> On Monday 04 September 2017 08:38 PM, Burakov, Anatoly wrote:
> >> From: Santosh Shukla [mailto:santosh.shukla at caviumnetworks.com]
> >> Sent: Thursday, August 31, 2017 4:26 AM
> >> To: dev at dpdk.org
> >> Cc: thomas at monjalon.net; jerin.jacob at caviumnetworks.com;
> >> hemant.agrawal at nxp.com; olivier.matz at 6wind.com;
> >> maxime.coquelin at redhat.com; Gonzalez Monroy, Sergio
> >> <sergio.gonzalez.monroy at intel.com>; Richardson, Bruce
> >> <bruce.richardson at intel.com>; shreyansh.jain at nxp.com;
> >> gaetan.rivet at 6wind.com; Burakov, Anatoly
> <anatoly.burakov at intel.com>;
> >> stephen at networkplumber.org; aconole at redhat.com; Santosh Shukla
> >> <santosh.shukla at caviumnetworks.com>
> >> Subject: [PATCH v7 3/9] linuxapp/eal_pci: get iommu class
> >>
> >> Get iommu class of PCI device on the bus and returns preferred iova
> >> mapping mode for that bus.
> >>
> >> Patch also introduces RTE_PCI_DRV_IOVA_AS_VA drv flag.
> >> Flag used when driver needs to operate in iova=va mode.
> >>
> >> Algorithm for iova scheme selection for PCI bus:
> >> 0. If no device bound then return with RTE_IOVA_DC mapping mode, else
> >> goto 1).
> >> 1. Look for device attached to vfio kdrv and has .drv_flag set to
> >> RTE_PCI_DRV_IOVA_AS_VA.
> >> 2. Look for any device attached to UIO class of driver.
> >> 3. Check for vfio-noiommu mode enabled.
> >>
> >> If 2) & 3) is false and 1) is true then select mapping scheme as
> RTE_IOVA_VA.
> >> Otherwise use default mapping scheme (RTE_IOVA_PA).
> >>
> >> Signed-off-by: Santosh Shukla <santosh.shukla at caviumnetworks.com>
> >> Signed-off-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> >> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> >> Acked-by: Hemant Agrawal <hemant.agrawal at nxp.com>
> >> ---
> >> v6 --> v7:
> >> - squashed v6 series patch no [01/12] & [05/12]..
> >>   i.e.. moved RTE_PCI_DRV_IOVA_AS_VA flag into this patch. (Aaron
> >> comment).
> >>
> >>  lib/librte_eal/common/include/rte_pci.h         |  2 +
> >>  lib/librte_eal/linuxapp/eal/eal_pci.c           | 95
> >> +++++++++++++++++++++++++
> >>  lib/librte_eal/linuxapp/eal/eal_vfio.c          | 19 +++++
> >>  lib/librte_eal/linuxapp/eal/eal_vfio.h          |  4 ++
> >>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >>  5 files changed, 121 insertions(+)
> >>
> >> diff --git a/lib/librte_eal/common/include/rte_pci.h
> >> b/lib/librte_eal/common/include/rte_pci.h
> >> index 0e36de093..a67d77f22 100644
> >> --- a/lib/librte_eal/common/include/rte_pci.h
> >> +++ b/lib/librte_eal/common/include/rte_pci.h
> >> @@ -202,6 +202,8 @@ struct rte_pci_bus {  #define
> >> RTE_PCI_DRV_INTR_RMV 0x0010
> >>  /** Device driver needs to keep mapped resources if unsupported dev
> >> detected */  #define RTE_PCI_DRV_KEEP_MAPPED_RES 0x0020
> >> +/** Device driver supports iova as va */ #define
> >> RTE_PCI_DRV_IOVA_AS_VA
> >> +0X0040
> >>
> >>  /**
> >>   * A structure describing a PCI mapping.
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> b/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> index 8951ce742..9725fd493 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> @@ -45,6 +45,7 @@
> >>  #include "eal_filesystem.h"
> >>  #include "eal_private.h"
> >>  #include "eal_pci_init.h"
> >> +#include "eal_vfio.h"
> >>
> >>  /**
> >>   * @file
> >> @@ -487,6 +488,100 @@ rte_pci_scan(void)
> >>  	return -1;
> >>  }
> >>
> >> +/*
> >> + * Is pci device bound to any kdrv
> >> + */
> >> +static inline int
> >> +pci_device_is_bound(void)
> >> +{
> >> +	struct rte_pci_device *dev = NULL;
> >> +	int ret = 0;
> >> +
> >> +	FOREACH_DEVICE_ON_PCIBUS(dev) {
> >> +		if (dev->kdrv == RTE_KDRV_UNKNOWN ||
> >> +		    dev->kdrv == RTE_KDRV_NONE) {
> >> +			continue;
> >> +		} else {
> >> +			ret = 1;
> >> +			break;
> >> +		}
> >> +	}
> >> +	return ret;
> >> +}
> >> +
> >> +/*
> >> + * Any one of the device bound to uio  */ static inline int
> >> +pci_device_bound_uio(void)
> >> +{
> >> +	struct rte_pci_device *dev = NULL;
> >> +
> >> +	FOREACH_DEVICE_ON_PCIBUS(dev) {
> >> +		if (dev->kdrv == RTE_KDRV_IGB_UIO ||
> >> +		   dev->kdrv == RTE_KDRV_UIO_GENERIC) {
> >> +			return 1;
> >> +		}
> >> +	}
> >> +	return 0;
> >> +}
> >> +
> >> +/*
> >> + * Any one of the device has iova as va  */ static inline int
> >> +pci_device_has_iova_va(void)
> >> +{
> >> +	struct rte_pci_device *dev = NULL;
> >> +	struct rte_pci_driver *drv = NULL;
> >> +
> >> +	FOREACH_DRIVER_ON_PCIBUS(drv) {
> >> +		if (drv && drv->drv_flags & RTE_PCI_DRV_IOVA_AS_VA) {
> >> +			FOREACH_DEVICE_ON_PCIBUS(dev) {
> >> +				if (dev->kdrv == RTE_KDRV_VFIO &&
> >> +				    rte_pci_match(drv, dev))
> >> +					return 1;
> >> +			}
> >> +		}
> >> +	}
> >> +	return 0;
> >> +}
> >> +
> >> +/*
> >> + * Get iommu class of PCI devices on the bus.
> >> + */
> >> +enum rte_iova_mode
> >> +rte_pci_get_iommu_class(void)
> >> +{
> >> +	bool is_bound;
> >> +	bool is_vfio_noiommu_enabled = true;
> >> +	bool has_iova_va;
> >> +	bool is_bound_uio;
> >> +
> >> +	is_bound = pci_device_is_bound();
> >> +	if (!is_bound)
> >> +		return RTE_IOVA_DC;
> >> +
> >> +	has_iova_va = pci_device_has_iova_va();
> >> +	is_bound_uio = pci_device_bound_uio(); #ifdef VFIO_PRESENT
> >> +	is_vfio_noiommu_enabled = vfio_noiommu_is_enabled() == 1 ? 1 :
> >> 0;
> > If you specify is_vfio_noiommu_enabled as bool, you should probably treat
> it as such, and assign true/false.
> 
> queued for v8.
> 
> > Other than that, I'm curious why is it always set to "true" by default? If we
> don't have VFIO compiled, it seems like the error message would always
> complain about vfio-noiommu mode being enabled, which is confusing.
> 
> Set to 'true' for case when VFIO_PRESENT unset.. meaning platform doesn't
> support VFIO (linux versioned < 3.6) i.e.. using UIO - In that case, flag makes
> sure _pa policy selected.
> 
> On error message: It won't come in non-vfio case, as 'has_iova_va' will set to
> 0.
> Error message will show for those case where few device out of many bind
> to uio, so message will pop-up and iova policy would be _pa in that case.
> 
> Thanks.

Right. My apologies, I misunderstood the meaning of "has_iova_va" flag.

Thanks,
Anatoly

> 
> > Thanks,
> > Anatoly