| Message ID | 20240717205511.2541693-1-wei.huang2@amd.com |
|---|---|
| Headers | show |
| Series | PCIe TPH and cache direct injection support | expand |
[cc += Paul Luse, Jing Liu] On Wed, Jul 17, 2024 at 03:55:01PM -0500, Wei Huang wrote: > TPH (TLP Processing Hints) is a PCIe feature that allows endpoint devices to > provide optimization hints for requests that target memory space. These hints, > in a format called steering tag (ST), are provided in the requester's TLP > headers and allow the system hardware, including the Root Complex, to > optimize the utilization of platform resources for the requests. [...] > This series introduces generic TPH support in Linux, allowing STs to be > retrieved from ACPI _DSM (as defined by ACPI) and used by PCIe endpoint > drivers as needed. As a demonstration, it includes an example usage in the > Broadcom BNXT driver. When running on Broadcom NICs with the appropriate > firmware, Cache Injection shows substantial memory bandwidth savings and > better network bandwidth using real-world benchmarks. This solution is > vendor-neutral, as both TPH and ACPI _DSM are industry standards. I think you need to add support for saving and restoring TPH registers, otherwise the changes you make to those registers may not survive reset recovery or system sleep. Granted, system sleep may not be relevant for servers (which I assume you're targeting with your patches), but reset recovery very much is. Paul Luse submitted a patch two years ago to save and restore TPH registers, perhaps you can include it in your patch set? https://lore.kernel.org/all/20220712123641.2319-1-paul.e.luse@intel.com/ Bjorn left some comments on Paul's patch: https://lore.kernel.org/all/20220912214516.GA538566@bhelgaas/ In particular, Bjorn asked for shared infrastructure to access TPH registers (which you're adding in your patch set) and spotted several nits (which should be easy to address). So I think you may be able to integrate Paul's patch into your series without too much effort. However note that when writing to TPH registers through the API you're introducing, you also need to update the saved register state so that those changes aren't lost upon a subsequent reset recovery. Thanks, Lukas
On 2024-07-17 13:55, Wei Huang wrote: > Hi All, > > TPH (TLP Processing Hints) is a PCIe feature that allows endpoint devices to > provide optimization hints for requests that target memory space. These hints, > in a format called steering tag (ST), are provided in the requester's TLP > headers and allow the system hardware, including the Root Complex, to > optimize the utilization of platform resources for the requests. > > Upcoming AMD hardware implement a new Cache Injection feature that leverages > TPH. Cache Injection allows PCIe endpoints to inject I/O Coherent DMA writes > directly into an L2 within the CCX (core complex) closest to the CPU core that > will consume it. This technology is aimed at applications requiring high > performance and low latency, such as networking and storage applications. This sounds very exciting Wei and it's good to see bnxt support. When you say 'upcoming AMD hardware' are you able to share exactly which? I would like to try this out.
On 7/20/24 03:08, Lukas Wunner wrote: > [cc += Paul Luse, Jing Liu] > > On Wed, Jul 17, 2024 at 03:55:01PM -0500, Wei Huang wrote: >> TPH (TLP Processing Hints) is a PCIe feature that allows endpoint devices to >> provide optimization hints for requests that target memory space. These hints, >> in a format called steering tag (ST), are provided in the requester's TLP >> headers and allow the system hardware, including the Root Complex, to >> optimize the utilization of platform resources for the requests. > [...] >> This series introduces generic TPH support in Linux, allowing STs to be >> retrieved from ACPI _DSM (as defined by ACPI) and used by PCIe endpoint >> drivers as needed. As a demonstration, it includes an example usage in the >> Broadcom BNXT driver. When running on Broadcom NICs with the appropriate >> firmware, Cache Injection shows substantial memory bandwidth savings and >> better network bandwidth using real-world benchmarks. This solution is >> vendor-neutral, as both TPH and ACPI _DSM are industry standards. > > I think you need to add support for saving and restoring TPH registers, > otherwise the changes you make to those registers may not survive > reset recovery or system sleep. Granted, system sleep may not be > relevant for servers (which I assume you're targeting with your patches), > but reset recovery very much is. > > Paul Luse submitted a patch two years ago to save and restore > TPH registers, perhaps you can include it in your patch set? Thanks for pointing them out. I skimmed through Paul's patch and it is straightforward to integrate. Depending on Bjorn's preference, I can either integrate it into my patchset with full credits to Paul and Jing, or Paul want to resubmit a new version. > > https://lore.kernel.org/all/20220712123641.2319-1-paul.e.luse@intel.com/ > > Bjorn left some comments on Paul's patch: > > https://lore.kernel.org/all/20220912214516.GA538566@bhelgaas/ > > In particular, Bjorn asked for shared infrastructure to access > TPH registers (which you're adding in your patch set) and spotted > several nits (which should be easy to address). So I think you may > be able to integrate Paul's patch into your series without too much > effort. I read Bjorn's comments, lots of them have been addressed in my patchset (e.g. move under /pci/pcie, support _DSM and dev->tph). So, as I said, integration is doable. > > However note that when writing to TPH registers through the API you're > introducing, you also need to update the saved register state so that > those changes aren't lost upon a subsequent reset recovery. > > Thanks, > > Lukas
On Mon, Jul 22, 2024 at 09:44:32AM -0500, Wei Huang wrote: > On 7/20/24 03:08, Lukas Wunner wrote: > > Paul Luse submitted a patch two years ago to save and restore > > TPH registers, perhaps you can include it in your patch set? > > Thanks for pointing them out. I skimmed through Paul's patch and it is > straightforward to integrate. > > Depending on Bjorn's preference, I can either integrate it into my > patchset with full credits to Paul and Jing, or Paul want to resubmit a > new version. The former would likely be better as I'm not sure Paul has the time to respin the patch. My recollection is that TPH save/restore support was dropped as a requirement for the Intel device this was originally developed for, but it would be a shame to lose the time and effort that already went into it and I think it might be useful for your use case as well to support reset recovery. > I read Bjorn's comments, lots of them have been addressed in my patchset > (e.g. move under /pci/pcie, support _DSM and dev->tph). Indeed, good job! Thanks for taking a look! Lukas
On 7/20/24 14:25, David Wei wrote: > On 2024-07-17 13:55, Wei Huang wrote: >> Hi All, >> >> TPH (TLP Processing Hints) is a PCIe feature that allows endpoint devices to >> provide optimization hints for requests that target memory space. These hints, >> in a format called steering tag (ST), are provided in the requester's TLP >> headers and allow the system hardware, including the Root Complex, to >> optimize the utilization of platform resources for the requests. >> >> Upcoming AMD hardware implement a new Cache Injection feature that leverages >> TPH. Cache Injection allows PCIe endpoints to inject I/O Coherent DMA writes >> directly into an L2 within the CCX (core complex) closest to the CPU core that >> will consume it. This technology is aimed at applications requiring high >> performance and low latency, such as networking and storage applications. > > This sounds very exciting Wei and it's good to see bnxt support. When > you say 'upcoming AMD hardware' are you able to share exactly which? I > would like to try this out. I can't specify which server platforms yet. But you can find this feature in either BIOS options or decode it from ACPI DSDT table (search UUID e5c937d0-3553-4d7a-9117-ea4d19c3434d, Func 0x0F).