linux-kernel - Re: [v2] PCI: cadence: Add configuration space capability search API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250314130511.hmceagpx5oq5gvrr@thinkpad>
Date: Fri, 14 Mar 2025 18:35:11 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Hans Zhang <18255117159@....com>
Cc: Siddharth Vadapalli <s-vadapalli@...com>, lpieralisi@...nel.org,
	kw@...ux.com, robh@...nel.org, bhelgaas@...gle.com,
	bwawrzyn@...co.com, thomas.richard@...tlin.com,
	wojciech.jasko-EXT@...tinental-corporation.com,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [v2] PCI: cadence: Add configuration space capability search API

On Mon, Mar 10, 2025 at 11:09:54PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/3/9 18:02, Siddharth Vadapalli wrote:
> > > Hi Siddharth,
> > > 
> > > Prior to this patch, I don't think hard-coded is that reasonable. Because
> > > the SOC design of each company does not guarantee that the offset of each
> > > capability is the same. This parameter can be configured when selecting PCIe
> > > configuration options. The current code that just happens to hit the offset
> > > address is the same.
> > 
> > 1. You are modifying the driver for the Cadence PCIe Controller IP and
> > not the one for your SoC (a.k.a the application/glue/wrapper driver).
> > 2. The offsets are tied to the Controller IP and not to your SoC. Any
> > differences that arise due to IP Integration into your SoC should be
> > handled in the Glue driver (the one which you haven't upstreamed yet).
> > 3. If the offsets in the Controller IP itself have changed, this
> > indicates a different IP version which has nothing to do with the SoC
> > that you are using.
> > 
> > Please clarify whether you are facing problems with the offsets due to a
> > difference in the IP Controller Version, or due to the way in which the IP
> > was integrated into your SoC.
> > 
> 
> Hi Siddharth,
> 
> I have consulted several PCIe RTL designers in the past two days. They told
> me that the controller IP of Synopsys or Cadence can be configured with the
> offset address of the capability. I don't think it has anything to do with
> SOC, it's just a feature of PCIe controller IP. In particular, the number of
> extended capability is relatively large. When RTL is generated, one more
> configuration may cause the offset addresses of extended capability to be
> different. Therefore, it is unreasonable to assign all the offset addresses
> in advance.
> 
> Here, I want to make Cadence PCIe common driver more general. When we keep
> developing new SoCs, the capability or extended capability offset address
> may eventually be inconsistent.
> 
> Thank you very much Siddharth for discussing this patch with me. I would
> like to know what other maintainers have to say about this.
> 

Even though this patch is mostly for an out of tree controller driver which is
not going to be upstreamed, the patch itself is serving some purpose. I really
like to avoid the hardcoded offsets wherever possible. So I'm in favor of this
patch.

However, these newly introduced functions are a duplicated version of DWC
functions. So we will end up with duplicated functions in multiple places. I'd
like them to be moved (both this and DWC) to drivers/pci/pci.c if possible. The
generic function *_find_capability() can accept the controller specific readl/
readw APIs and the controller specific private data.

- Mani

-- 
மணிவண்ணன் சதாசிவம்