On 14-Jun-19 10:39 AM, David Marchand wrote:
From: Ben Walker <benjamin.wal...@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.wal...@intel.com>
Signed-off-by: David Marchand <david.march...@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
}
#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 (is_vfio_noiommu_enabled != 0) {
iova_mode = RTE_IOVA_PA;
}
#endif
break;
In fact, could we not check if devices support both flags, and do an
early exit in case they don't?
Something like this, for example:
if ((pdrv->drv_flags & RTE_PCI_DRV_IOVA_AS_VA) == 0) {
return RTE_IOVA_PA; // early exit - device only wants PA
}
// device supports both PA and VA modes, so do more checks
switch (pdev->kdrv) {
...
}
Unless i'm missing something, this would look much simpler and easier to
understand.
--
Thanks,
Anatoly
