Messages in this thread | | | Date | Mon, 21 Mar 2022 09:46:04 -0600 | From | Logan Gunthorpe <> | Subject | Re: [PATCH v1] Intel Sky Lake-E host root ports check. |
| |
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?
> Signed-off-by: Shlomo Pongratz <shlomop@pliops.com> > --- > 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;
If port is placed after flags then we only need to add the port for the three devices that care about it. Designated initializers will set it to 0 if it is omitted.
> } 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}, > {} > }; > >
Logan
| |