Message-ID: <a1459350-f458-470b-a288-a92e2085f93a@amd.com>
Date: Wed, 4 Sep 2024 14:48:30 -0500
From: Wei Huang <wei.huang2@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, netdev@...r.kernel.org,
 Jonathan.Cameron@...wei.com, corbet@....net, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 alex.williamson@...hat.com, gospo@...adcom.com, michael.chan@...adcom.com,
 ajit.khaparde@...adcom.com, somnath.kotur@...adcom.com,
 andrew.gospodarek@...adcom.com, manoj.panicker2@....com,
 Eric.VanTassell@....com, vadim.fedorenko@...ux.dev, horms@...nel.org,
 bagasdotme@...il.com, bhelgaas@...gle.com, lukas@...ner.de,
 paul.e.luse@...el.com, jing2.liu@...el.com
Subject: Re: [PATCH V4 00/12] PCIe TPH and cache direct injection support



On 9/4/24 13:49, Bjorn Helgaas wrote:
> On Thu, Aug 22, 2024 at 03:41:08PM -0500, 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.
> 
> Thanks for this example, it's a great intro.  Suggest adding something
> similar to a patch commit log, since the cover letter is harder to
> find after this appears in git.

I'll incorporate some of these descriptions into the TPH patches where
relevant. Additionally, I'll enhance the commit log for bnxt.c (patch
11) with examples of the benefits.

> 
>> This series introduces generic TPH support in Linux, allowing STs to be
>> retrieved 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, it shows
>> substantial memory bandwidth savings and better network bandwidth using
>> real-world benchmarks. This solution is vendor-neutral and implemented
>> based on industry standards (PCIe Spec and PCI FW Spec).
>>
>> V3->V4:
>>  * Rebase on top of the latest pci/next tree (tag: 6.11-rc1)
> 
> No need to rebase to pci/next; pci/main is where it will be applied.
> But it currently applies cleanly to either, so no problem.

Got it, will rebase to pci/main in next spin anyway.

> 
>>  * Add new API functioins to query/enable/disable TPH support
>>  * Make pcie_tph_set_st() completely independent from pcie_tph_get_cpu_st()
>>  * Rewrite bnxt.c based on new APIs
>>  * Remove documentation for now due to constantly changing API
> 
> I'd like to see this documentation included.  And updated if the API
> changes, of course.

Will do.

> 
>>  * Remove pci=notph, but keep pci=nostmode with better flow (Bjorn)
> 
> This seems backward to me.  I think "pci=notph" makes sense as a way
> to completely disable the TPH feature in case a user trips over a
> hardware or driver defect.
> 
> But "pci=nostmode" is advertised as a way to quantify the benefit of
> Steering Tags, and that seems like it's of interest to developers but
> not users.
> 
> So my advice would be to keep "pci=notph" and drop "pci=nostmode".

OK, I will replace the "nostmode" patch with "notph" in V5.

> 
> Bjorn

Powered by blists - more mailing lists