linux-kernel - Re: [PATCH 2/5] PCI: dwc: Add new APIs to remove standard and extended Capability

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: <20251226210741.GA4141010@bhelgaas>
Date: Fri, 26 Dec 2025 15:07:41 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Qiang Yu <qiang.yu@....qualcomm.com>
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Manivannan Sadhasivam <mani@...nel.org>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Jingoo Han <jingoohan1@...il.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	Wenbin Yao <wenbin.yao@....qualcomm.com>
Subject: Re: [PATCH 2/5] PCI: dwc: Add new APIs to remove standard and
 extended Capability

On Sun, Nov 09, 2025 at 10:59:41PM -0800, Qiang Yu wrote:
> On some platforms, certain PCIe Capabilities may be present in hardware
> but are not fully implemented as defined in PCIe spec. These incomplete
> capabilities should be hidden from the PCI framework to prevent unexpected
> behavior.
> 
> Introduce two APIs to remove a specific PCIe Capability and Extended
> Capability by updating the previous capability's next offset field to skip
> over the unwanted capability. These APIs allow RC drivers to easily hide
> unsupported or partially implemented capabilities from software.

It's super annoying to have to do this, but I suppose from the
hardware point of view these things *could* work depending on the
design of the platform outside the device.  So the designers probably
assume platform firmware knows those details and hides things that
aren't supported.  But in the DT case, there likely *is* no platform
firmware, so the native RC driver has to do it instead.

> Co-developed-by: Wenbin Yao <wenbin.yao@....qualcomm.com>
> Signed-off-by: Wenbin Yao <wenbin.yao@....qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@....qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 53 ++++++++++++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h |  2 ++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 5585d3ed74316bd218572484f6320019db8d6a10..24f8e9959cb81ca41e91d27057cc115d32e8d523 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -234,6 +234,59 @@ u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
>  
> +void dw_pcie_remove_capability(struct dw_pcie *pci, u8 cap)
> +{
> +	u8 cap_pos, pre_pos, next_pos;
> +	u16 reg;
> +
> +	cap_pos = PCI_FIND_NEXT_CAP(dw_pcie_read_cfg, PCI_CAPABILITY_LIST, cap,
> +				 &pre_pos, pci);
> +	if (!cap_pos)
> +		return;
> +
> +	reg = dw_pcie_readw_dbi(pci, cap_pos);
> +	next_pos = (reg & 0xff00) >> 8;
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +	if (pre_pos == PCI_CAPABILITY_LIST)
> +		dw_pcie_writeb_dbi(pci, PCI_CAPABILITY_LIST, next_pos);
> +	else
> +		dw_pcie_writeb_dbi(pci, pre_pos + 1, next_pos);
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_remove_capability);
> +
> +void dw_pcie_remove_ext_capability(struct dw_pcie *pci, u8 cap)
> +{
> +	int cap_pos, next_pos, pre_pos;
> +	u32 pre_header, header;
> +
> +	cap_pos = PCI_FIND_NEXT_EXT_CAP(dw_pcie_read_cfg, 0, cap, &pre_pos, pci);
> +	if (!cap_pos)
> +		return;
> +
> +	header = dw_pcie_readl_dbi(pci, cap_pos);

Blank line here to match style of other comments.

> +	/*
> +	 * If the first cap at offset PCI_CFG_SPACE_SIZE is removed,
> +	 * only set it's capid to zero as it cannot be skipped.

If setting the capid to zero works here, why won't it work for all
capabilities?  If setting capid to zero is enough, we wouldn't need to
mess with keeping track of the previous capability, and we could drop
the first patch.

s/it's/its/

> +	 */
> +	if (cap_pos == PCI_CFG_SPACE_SIZE) {
> +		dw_pcie_dbi_ro_wr_en(pci);
> +		dw_pcie_writel_dbi(pci, cap_pos, header & 0xffff0000);
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +		return;
> +	}
> +
> +	pre_header = dw_pcie_readl_dbi(pci, pre_pos);
> +	next_pos = PCI_EXT_CAP_NEXT(header);
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +	dw_pcie_writel_dbi(pci, pre_pos,
> +			  (pre_header & 0xfffff) | (next_pos << 20));
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_remove_ext_capability);
> +
>  static u16 __dw_pcie_find_vsec_capability(struct dw_pcie *pci, u16 vendor_id,
>  					  u16 vsec_id)
>  {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e995f692a1ecd10130d3be3358827f801811387f..b68dbc528001b63448db8b1a93bf56a5e53bd33e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -552,6 +552,8 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
>  
>  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
>  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
> +void dw_pcie_remove_capability(struct dw_pcie *pci, u8 cap);
> +void dw_pcie_remove_ext_capability(struct dw_pcie *pci, u8 cap);
>  u16 dw_pcie_find_rasdes_capability(struct dw_pcie *pci);
>  u16 dw_pcie_find_ptm_capability(struct dw_pcie *pci);
>  
> 
> -- 
> 2.34.1
>