Re: [PATCH v4 1/5] PCI: Add vendor ID for the PCI SIG

From: Dan Williams <dan.j.williams@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Linux PCI <linux-pci@vger.kernel.org>,
	linux-cxl@vger.kernel.org, Bjorn Helgaas <helgaas@kernel.org>,
	"Weiny, Ira" <ira.weiny@intel.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Ben Widawsky <ben.widawsky@intel.com>,
	Chris Browy <cbrowy@avery-design.com>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	"Schofield, Alison" <alison.schofield@intel.com>,
	Vishal L Verma <vishal.l.verma@intel.com>,
	Fangjian <f.fangjian@huawei.com>, Linuxarm <linuxarm@huawei.com>
Subject: Re: [PATCH v4 1/5] PCI: Add vendor ID for the PCI SIG
Date: Thu, 10 Jun 2021 13:10:56 -0700	[thread overview]
Message-ID: <CAPcyv4hE8M2qmMZYH+rRbYJyCV0LVNsc0hAYcRqOawd6aYUBRw@mail.gmail.com> (raw)
In-Reply-To: <20210610183924.0000491d@Huawei.com>

On Thu, Jun 10, 2021 at 10:39 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Thu, 10 Jun 2021 08:17:23 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > On Mon, May 24, 2021 at 6:41 AM Jonathan Cameron
> > <Jonathan.Cameron@huawei.com> wrote:
> > >
> > > This ID is used in DOE headers to identify protocols that are defined
> > > within the PCI Express Base Specification.
> > >
> > > Specified in Table 7-x2 of the Data Object Exchange ECN (approved 12 March
> > > 2020) available from https://members.pcisig.com/wg/PCI-SIG/document/14143
> > >
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > ---
> > >  include/linux/pci_ids.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > > index 4c3fa5293d76..dcc8b4b14198 100644
> > > --- a/include/linux/pci_ids.h
> > > +++ b/include/linux/pci_ids.h
> > > @@ -149,6 +149,7 @@
> > >  #define PCI_CLASS_OTHERS               0xff
> > >
> > >  /* Vendors and devices.  Sort key: vendor first, device next. */
> > > +#define PCI_VENDOR_ID_PCI_SIG          0x0001
> >
> > Should this not be:
> >
> > PCI_DOE_VENDOR_ID_PCI_SIG?
> >
> > ...because I don't think this value will ever show up at the typical
> > config-offset 0 vendor-id, will it?
>
> Good question.
>
> Whilst I agree it is unlikely to turn up as a conventional vendor-id
> (though I've not found any text ruling it out) it already turns up
> in locations other than DOE.
>
> Many of them aren't software visible, but potentially places
> like SPDM are in which you would have a registry ID of 0x3 (PCI-SIG)
> followed by the PCI vendor ID (this one).  Those are used in SPDM
> vendor defined requests / responses.
>
> That SPDM feature is then used in IDE establishment.
> The IDE ECN (via pcisig.com) has the following:
> "The VendorID field of the VENDOR_DEFINED_REQUEST/
>  VENDOR_DEFINED_RESPONSE must contain the value 0001h, which is assigned to
>  the PCI-SIG."
>
> Which to my reading, isn't quite the same as saying it's a vendor ID,
> but nearly so.
>
> Now, I argued the *_DVSEC_* naming in the CXL one based on the spec saying
> that was all it could be used for but I may well have been wrong longer
> term.
>
> I'm fine with renaming it to the PCI_DOE_* version then dropping the DOE
> when it gets used for something else though if that works for people.
>
> At least this time naming isn't made awkward by legalese.

For fun I did a lookup for vendor-id 1 and it came back "Fry's
Electronics Counterfeit Flash Drive"

https://pcilookup.com/?ven=0001&dev=&action=submit

The potential for it to be used in other places outside of DOE makes
me think the way you have it here is fine.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>