Re: [PATCH V3 00/10] PCIe TPH and cache direct injection support

[Wei Huang <wei.huang2@amd.com>]

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