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] [day] [month] [year] [list]

Message-ID: <20241211144330.GA3290532@bhelgaas>
Date: Wed, 11 Dec 2024 08:43:30 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
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 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.

Bjorn