| 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
| ||
|
Message-ID: <aVDhFq0Qu6pVbVd5@hu-qianyu-lv.qualcomm.com>
Date: Sat, 27 Dec 2025 23:49:42 -0800
From: Qiang Yu <qiang.yu@....qualcomm.com>
To: Bjorn Helgaas <helgaas@...nel.org>
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 Fri, Dec 26, 2025 at 03:07:41PM -0600, Bjorn Helgaas wrote:
> 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.
>
Setting the capability ID to zero is simpler and works for all
capabilities. However, it makes users confused to see a capability with
capability ID zero, especially when using debug tools like lspci to list
the configuration space. I think it works functionally, but it's not a
clean approach. Setting capid of the cap at offset PCI_CFG_SPACE_SIZE
to zero is a last resort.
-Qiang Yu
> 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
> >
Powered by blists - more mailing lists