On commit 7b94b53db34f ("PCI/P2PDMA: Add Intel Sky Lake-E Root Ports B, C, D to the whitelist")
Andrew Maier added the Sky Lake-E additional devices
2031, 2032 and 2033 root ports to the already existing 2030 device.
Note that the Intel devices 2030, 2031, 2032 and 2033 are ports A, B, C and D.
Consider on a bus X only port C is connected downstream so in the PCI scan only
device 8086:2032 on 0000:X:02.0 will be found as bridges that have no children are ignored.
As a result the routine pci_host_bridge_dev will return NULL for devices under slot C.
In the proposed patch port field is added to the whitelist which is 0 for 2030, 1 for 2031,
2 for 2032 3 for 2033 and 0 for all other devices.
Signed-off-by: Shlomo Pongratz <[email protected]>
---
drivers/pci/p2pdma.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 1015274bd2fe..86f6594a0b8a 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -305,22 +305,23 @@ static bool cpu_supports_p2pdma(void)
static const struct pci_p2pdma_whitelist_entry {
unsigned short vendor;
unsigned short device;
+ unsigned short port;
enum {
REQ_SAME_HOST_BRIDGE = 1 << 0,
} flags;
} pci_p2pdma_whitelist[] = {
/* Intel Xeon E5/Core i7 */
- {PCI_VENDOR_ID_INTEL, 0x3c00, REQ_SAME_HOST_BRIDGE},
- {PCI_VENDOR_ID_INTEL, 0x3c01, REQ_SAME_HOST_BRIDGE},
+ {PCI_VENDOR_ID_INTEL, 0x3c00, 0, REQ_SAME_HOST_BRIDGE},
+ {PCI_VENDOR_ID_INTEL, 0x3c01, 0, REQ_SAME_HOST_BRIDGE},
/* Intel Xeon E7 v3/Xeon E5 v3/Core i7 */
- {PCI_VENDOR_ID_INTEL, 0x2f00, REQ_SAME_HOST_BRIDGE},
- {PCI_VENDOR_ID_INTEL, 0x2f01, REQ_SAME_HOST_BRIDGE},
+ {PCI_VENDOR_ID_INTEL, 0x2f00, 0, REQ_SAME_HOST_BRIDGE},
+ {PCI_VENDOR_ID_INTEL, 0x2f01, 0, REQ_SAME_HOST_BRIDGE},
/* Intel SkyLake-E */
- {PCI_VENDOR_ID_INTEL, 0x2030, 0},
- {PCI_VENDOR_ID_INTEL, 0x2031, 0},
- {PCI_VENDOR_ID_INTEL, 0x2032, 0},
- {PCI_VENDOR_ID_INTEL, 0x2033, 0},
- {PCI_VENDOR_ID_INTEL, 0x2020, 0},
+ {PCI_VENDOR_ID_INTEL, 0x2030, 0, 0},
+ {PCI_VENDOR_ID_INTEL, 0x2031, 1, 0},
+ {PCI_VENDOR_ID_INTEL, 0x2032, 2, 0},
+ {PCI_VENDOR_ID_INTEL, 0x2033, 3, 0},
+ {PCI_VENDOR_ID_INTEL, 0x2020, 0, 0},
{}
};
@@ -332,6 +333,11 @@ static const struct pci_p2pdma_whitelist_entry {
* bus->devices list and that the devfn is 00.0. These assumptions should hold
* for all the devices in the whitelist above.
*
+ * The method above will work in most cases but not for all.
+ * Note that the Intel devices 2030, 2031, 2032 and 2033 are ports A, B, C and D.
+ * Consider on a bus X only port C is connected downstream so in the PCI scan only
+ * device 8086:2032 on 0000:X:02.0 will be found as birdges with no children are ignored
+ *
* This function is equivalent to pci_get_slot(host->bus, 0), however it does
* not take the pci_bus_sem lock seeing __host_bridge_whitelist() must not
* sleep.
@@ -349,7 +355,10 @@ static struct pci_dev *pci_host_bridge_dev(struct pci_host_bridge *host)
if (!root)
return NULL;
- if (root->devfn != PCI_DEVFN(0, 0))
+ /* Here just check that the function is 0
+ * The slot number will be checked later
+ */
+ if (PCI_FUNC(root->devfn) != 0)
return NULL;
return root;
@@ -371,6 +380,12 @@ static bool __host_bridge_whitelist(struct pci_host_bridge *host,
for (entry = pci_p2pdma_whitelist; entry->vendor; entry++) {
if (vendor != entry->vendor || device != entry->device)
continue;
+ /* For Intel Sky Lake-E host root ports check the port is
+ * Identical to the slot number.
+ * For other devices continue to inssist on slot 0
+ */
+ if (PCI_SLOT(root->devfn) != entry->port)
+ return false;
if (entry->flags & REQ_SAME_HOST_BRIDGE && !same_host_bridge)
return false;
--
2.17.1
On 2022-03-21 10:21, Shlomo Pongratz wrote:
> See inline
>
> Shlomo.
>
>> On 21 Mar 2022, at 17:46, Logan Gunthorpe <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>>
>>
>> On 2022-03-21 08:31, Shlomo Pongratz wrote:
>>> On commit 7b94b53db34f ("PCI/P2PDMA: Add Intel Sky Lake-E Root Ports
>>> B, C, D to the whitelist")
>>> Andrew Maier added the Sky Lake-E additional devices
>>> 2031, 2032 and 2033 root ports to the already existing 2030 device.
>>> Note that the Intel devices 2030, 2031, 2032 and 2033 are ports A, B,
>>> C and D.
>>> Consider on a bus X only port C is connected downstream so in the PCI
>>> scan only
>>> device 8086:2032 on 0000:X:02.0 will be found as bridges that have no
>>> children are ignored.
>>> As a result the routine pci_host_bridge_dev will return NULL for
>>> devices under slot C.
>>> In the proposed patch port field is added to the whitelist which is 0
>>> for 2030, 1 for 2031,
>>> 2 for 2032 3 for 2033 and 0 for all other devices.
>>
>> The patch looks largely ok, but I'm not sure I follow this description.
>>
>> It sounds like in practice the host bridges B, C and D are not addressed
>> at function 0 as was assumed. But what does it mean that only C is
>> connected downstream? How can a bridge not be connected downstream?
>>
> Maybe it is wrong usage of words.
> I mean three are no devices behind port A and B, it is possible if there
> if one has empty PCI slots.
> I my case I had only on NVMe SSD connected to port C.
If a bridge has no devices behind it, it's not ever going to get into
the p2pdma code; so I'm not sure how that's relevant. And just because
one user doesn't have any device behind a bridge, doesn't mean that all
users have no devices under that bridge.
Sounds like the commit description and comments just neeed to change to
note that the function of each of the bridge is not always zero, as
originally assumed.
Logan