Hi Gaëtan,

On 10/19/2017 7:42 PM, Gaëtan Rivet wrote:
Hi Jianfeng,

On Thu, Oct 19, 2017 at 11:18:29AM +0000, Jianfeng Tan wrote:
When checking if any devices bound to uio, we did not exclud
those which are blacklisted (or in the case that a whitelist
is specified).

This patch fixes it by only checking whitelisted devices.

Fixes: 815c7deaed2d ("pci: get IOMMU class on Linux")

Signed-off-by: Jianfeng Tan <jianfeng....@intel.com>
---
  lib/librte_eal/linuxapp/eal/eal_pci.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c 
b/lib/librte_eal/linuxapp/eal/eal_pci.c
index b4dbf95..2b23d67 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -516,8 +516,26 @@ static inline int
  pci_one_device_bound_uio(void)
  {
        struct rte_pci_device *dev = NULL;
+       struct rte_devargs *devargs;
+       int check_all = 1;
+       int need_check;
+
+       if (rte_pci_bus.bus.conf.scan_mode == RTE_BUS_SCAN_WHITELIST)
+               check_all = 0;
FOREACH_DEVICE_ON_PCIBUS(dev) {
+               devargs = dev->device.devargs;
+
+               need_check = 0;
+               if (check_all)
Unless I'm mistaken, you will check blacklisted devices as well here.

Thank you for pointing out this.

I was referring to rte_pci_probe(), which also only check "probe_all" and (devargs && RTE_DEV_WHITELISTED); but turns out it double checks the blacklisted devices in rte_pci_probe_one_driver().

I'll fix it.

The condition should be something like:

if (check_all && devargs == NULL)

Which means that both ifs can be refactored as

if ((check_all ^ (devargs != NULL)) == 0)
         continue;

Removing need_check. But it can be hard to read.

Yes, I prefer to make it easy to understand. Please let me know if you are OK with below code (remove check_all):

        FOREACH_DEVICE_ON_PCIBUS(dev) {
                devargs = dev->device.devargs;

                need_check = 0;
                switch (rte_pci_bus.bus.conf.scan_mode) {
                case RTE_BUS_SCAN_UNDEFINED:
                        need_check = 1;
                        break;
                case RTE_BUS_SCAN_WHITELIST:
if (devargs && devargs->policy == RTE_DEV_WHITELISTED)
                                need_check = 1;
                        break;
                case RTE_BUS_SCAN_BLACKLIST:
if (!devargs || devargs->policy != RTE_DEV_BLACKLISTED)
                                need_check = 1;
                        break;
                }

                if (!need_check)
                        continue;
...

Thanks,
Jianfeng

Reply via email to