On 2021-07-16 5:12 a.m., Dongdong Liu wrote: > Hi Bjorn > > Many thanks for your review. > > On 2021/7/16 1:23, Bjorn Helgaas wrote: >> [+cc Logan] >> >> On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote: >>> 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag >>> field size from 8 bits to 10 bits. >>> >>> For platforms where the RC supports 10-Bit Tag Completer capability, >>> it is highly recommended for platform firmware or operating software >> >> Recommended by whom? If the spec recommends it, we should provide the >> citation. > PCIe spec 5.0 r1.0 section 2.2.6.2 IMPLEMENTATION NOTE says that. > Will fix. >> >>> that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable >>> bit automatically in Endpoints with 10-Bit Tag Requester capability. This >>> enables the important class of 10-Bit Tag capable adapters that send >>> Memory Read Requests only to host memory. >> >> What is the implication for P2PDMA? What happens if we enable 10-bit >> tags for device A, and A generates Mem Read Requests to device B, >> which does not support 10-bit tags? > PCIe spec 5.0 r1.0 section 2.2.6.2 says > If an Endpoint supports sending Requests to other Endpoints (as opposed > to host memory), the Endpoint must not send 10-Bit Tag Requests to > another given Endpoint unless an implementation-specific mechanism > determines that the Endpoint supports 10-Bit Tag Completer capability. > Not sending 10-Bit Tag Requests to other Endpoints at all > may be acceptable for some implementations. More sophisticated > mechanisms are outside the scope of this specification. > > Not sending 10-Bit Tag Requests to other Endpoints at all seems simple. > Add kernel parameter pci=pcie_bus_peer2peer when boot kernel with > P2PDMA, then do not config 10-BIT Tag. > > if (pcie_bus_config != PCIE_BUS_PEER2PEER) > pci_configure_10bit_tags(dev); > > Bjorn and Logan, any suggestion? I think we need a check in the P2PDMA code to ensure that a device with 10bit tags doesn't interact with a device that has no 10bit tags. Before that happens, the kernel should emit a warning saying to enable a specific kernel parameter. Though a parameter with a bit more granularity might be appropriate. See what was done for disable_acs_redir where it affects only the devices specified in the list. Thanks, Logan
