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@xxxxxxxxx/ > > 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