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: <20250320212853.GA1100504@bhelgaas>
Date: Thu, 20 Mar 2025 16:28:53 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Hans Zhang <18255117159@....com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	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 Sat, Mar 15, 2025 at 08:06:52AM +0800, Hans Zhang wrote:
> On 2025/3/15 04:31, Bjorn Helgaas wrote:
> > On Fri, Mar 14, 2025 at 06:35:11PM +0530, Manivannan Sadhasivam wrote:
> > > ...
> > 
> > > 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.
> > 
> > I agree, it would be really nice to share this code.
> > 
> > It looks a little messy to deal with passing around pointers to
> > controller read ops, and we'll still end up with a lot of duplicated
> > code between __pci_find_next_cap() and __cdns_pcie_find_next_cap(),
> > etc.
> > 
> > Maybe someday we'll make a generic way to access non-PCI "config"
> > space like this host controller space and PCIe RCRBs.
> > 
> > Or if you add interfaces that accept read/write ops, maybe the
> > existing pci_find_capability() etc could be refactored on top of them
> > by passing in pci_bus_read_config_word() as the accessor.
> 
> I have already replied to an email, please help review whether it is
> appropriate.

URL to the email on lore.kernel.org?

If you mean this:
https://lore.kernel.org/r/3cadf8d5-c4d8-4941-ae2e-8b00ceb83a8f@163.com,
just post it as a v3 patch with the usual commit log and motivation
for the change, and it will automatically get picked up in patchwork
so it doesn't get forgotten.

Right now the urgency seems fairly low since (IIUC) it doesn't fix an
existing bug in the upstream code.

Bjorn