linux-kernel - Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search

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: <20250115152742.yhb57c6cbbwrnjcg@thinkpad>
Date: Wed, 15 Jan 2025 20:57:42 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Shradha Todi <shradha.t@...sung.com>, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, lpieralisi@...nel.org, kw@...ux.com,
	robh@...nel.org, bhelgaas@...gle.com, jingoohan1@...il.com,
	Jonathan.Cameron@...wei.com, fan.ni@...sung.com,
	a.manzanares@...sung.com, pankaj.dubey@...sung.com,
	quic_nitegupt@...cinc.com, quic_krichai@...cinc.com,
	gost.dev@...sung.com
Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific
 capability search

On Wed, Dec 11, 2024 at 08:43:30AM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 11, 2024 at 05:15:50PM +0530, Shradha Todi wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@...nel.org>
> > > Sent: 06 December 2024 21:43
> > > To: Shradha Todi <shradha.t@...sung.com>
> > > Cc: linux-kernel@...r.kernel.org; linux-pci@...r.kernel.org; manivannan.sadhasivam@...aro.org; lpieralisi@...nel.org;
> > > kw@...ux.com; robh@...nel.org; bhelgaas@...gle.com; jingoohan1@...il.com; Jonathan.Cameron@...wei.com;
> > > fan.ni@...sung.com; a.manzanares@...sung.com; pankaj.dubey@...sung.com; quic_nitegupt@...cinc.com;
> > > quic_krichai@...cinc.com; gost.dev@...sung.com
> > > Subject: Re: [PATCH v4 1/2] PCI: dwc: Add support for vendor specific capability search
> > > 
> > > On Fri, Dec 06, 2024 at 01:14:55PM +0530, Shradha Todi wrote:
> > > > Add vendor specific extended configuration space capability search API
> > > > using struct dw_pcie pointer for DW controllers.
> > > >
> > > > Signed-off-by: Shradha Todi <shradha.t@...sung.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-designware.c | 16 ++++++++++++++++
> > > > drivers/pci/controller/dwc/pcie-designware.h |  1 +
> > > >  2 files changed, 17 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 6d6cbc8b5b2c..41230c5e4a53 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -277,6 +277,22 @@ static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> > > >  	return 0;
> > > >  }
> > > >
> > > > +u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci, u8 vsec_cap)
> > > 
> > > To make sure that we find a VSEC ID that corresponds to the
> > > expected vendor, I think this interface needs to be the same
> > > as pci_find_vsec_capability().  In particular, it needs to take a
> > > "u16 vendor"
> >
> > As per my understanding, Synopsys is the vendor here when we talk
> > about vsec capabilities.  VSEC cap IDs are fixed for each vendor
> > (eg: For Synopsys Designware controllers, 0x2 is always RAS CAP, 0x4
> > is always PTM responder and so on).
> 
> For VSEC, the vendor that matters is the one identified at 0x0 in
> config space.  That's why pci_find_vsec_capability() checks the
> supplied "vendor" against "dev->vendor".
> 
> > So no matter if the DWC IP is being integrated by Samsung, NVDIA or
> > Qcom, the vendor specific CAP IDs will remain constant. Now since
> > this function is being written as part of designware file, the
> > control will reach here only when the PCIe IP is DWC. So, we don't
> > really require a vendor ID to be checked here. EG: If 0x2 VSEC ID is
> > present in any DWC controller, it means RAS is supported. Please
> > correct me if I'm wrong.
> 
> In this case, the Vendor ID is typically Samsung, NVIDIA, Qcom, etc.,
> even though it may contain Synopsys DWC IP.  Each vendor assigns VSEC
> IDs independently, so VSEC ID 0x2 may mean something different to
> Samsung than it does to NVIDIA or Qcom.
> 
> PCIe r6.0, sec 7.9.5 has the details, but the important part is this:
> 
>   With a PCI Express Function, the structure and definition of the
>   vendor-specific Registers area is determined by the vendor indicated
>   by the Vendor ID field located at byte offset 00h in PCI-compatible
>   Configuration Space.
> 
> There IS a separate DVSEC ("Designated Vendor-Specific") Capability;
> see sec 7.9.6.  That one does include a DVSEC Vendor ID in the
> Capability itself, and this would make more sense for this situation.
> 
> If Synopsys assigned DVSEC ID 0x2 from the Synopsys namespace for RAS,
> then devices from Samsung, NVIDIA, Qcom, etc., could advertise a DVSEC
> Capability that contained a DVSEC Vendor ID of PCI_VENDOR_ID_SYNOPSYS
> with DVSEC ID 0x2, and all those devices could easily locate it.
> 
> Unfortunately Samsung et al used VSEC instead of DVSEC, so we're stuck
> with having to specify the device vendor and the VSEC ID assigned by
> that vendor, and those VSEC IDs might be different per vendor.
> 

Atleast on Qcom platforms, VSEC_ID is 0x2 for RAS. But this is not guaranteed to
be the same as per the PCIe spec as you mentioned. Though, I think it is safe to
go with it since we have seen the same IDs on 2 platforms (my gut feeling is
that it is going to be the same on other DWC vendor platforms as well). If we
encounter different IDs, then we can add vendor id check.

- Mani

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