On 2021-05-11 10:05 a.m., Don Dutile wrote:

... add a flag (set for p2pdma use)  to the function to print out what the root->devfn is, and what
the device is so the needed quirk &/or modification can added to handle when this assumption fails;
or make it a prdebug that can be flipped on for this failing situation, again, to add needed change to accomodate.

Good idea! Will add.

quoted

    root = NULL;
 out:
    pci_dev_get(root);
    return root;
}
EXPORT_SYMBOL(pci_get_root_slot);

...I think that's a lot clearer to the reader, about what's going on here.

Note that I'm not really sure if it *is* safe, I would need to ask other
PCIe subsystem developers with more experience. But I don't think anyone
is trying to make p2pdma calls so early that PCIe buses are uninitialized.

quoted

+
+    if (!root || root->devfn)
          return false;
        vendor = root->vendor;
      device = root->device;
-    pci_dev_put(root);

and the reason to remove the dev_put is b/c it can sleep as well?
is that ok, given the dev_get that John put into the new pci_get_root_slot()?
... seems like a locking version with no get/put's is needed, or, fix the host-bridge setups so no !NULL self pointers.

The dev_get is redundant here seeing we hold references to child
devices. It was only in the previous code because we were using
pci_get_slot() to get the device which did the get for us.

Logan