Re: [PATCH v2 1/3] PCI: Add check for CXL Secondary Bus Reset

From: Bjorn Helgaas <helgaas@kernel.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>,
	linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org,
	ira.weiny@intel.com, vishal.l.verma@intel.com,
	alison.schofield@intel.com, Jonathan.Cameron@huawei.com,
	dave@stgolabs.net, bhelgaas@google.com, lukas@wunner.de
Subject: Re: [PATCH v2 1/3] PCI: Add check for CXL Secondary Bus Reset
Date: Tue, 2 Apr 2024 12:23:23 -0500	[thread overview]
Message-ID: <20240402172323.GA1818777@bhelgaas> (raw)
In-Reply-To: <6605bef53c82b_1fb31e29481@dwillia2-xfh.jf.intel.com.notmuch>

On Thu, Mar 28, 2024 at 12:03:17PM -0700, Dan Williams wrote:
> Bjorn Helgaas wrote:
> > On Wed, Mar 27, 2024 at 04:57:40PM -0700, Dave Jiang wrote:
> > > On 3/27/24 2:26 PM, Bjorn Helgaas wrote:
> > > > On Mon, Mar 25, 2024 at 04:58:01PM -0700, Dave Jiang wrote:
> ...

> > > > IIUC, this should be just a PCI SIG-defined "Vendor ID", e.g.,
> > > > "PCI_VENDOR_ID_CXL", that doesn't need the "DVSEC" qualifier in the
> > > > name, and would normally be defined in include/linux/pci_ids.h.
> > > > 
> > > > But I don't see CXL in pci_ids.h, and I don't see either "CXL" or the
> > > > value "1e98" in the PCI SIG list at
> > > > https://pcisig.com/membership/member-companies.
> > > > 
> > > I'll create a new patch and move to include/linux/pci_ids.h first
> > > for this define and change to PCI_VENDOR_ID_CXL. The value is
> > > defined in CXL spec r3.1 sec 8.1.1.
> > 
> > I saw the CXL mentions of 0x1e98, but IMO that's not an authoritative
> > source; no vendor is allowed to just squat on a Vendor ID value simply
> > by mentioning it in their own internal specs.  That would obviously
> > lead to madness.
> > 
> > The footnote in CXL r3.1, sec 3.1.2, about how the 1E98h value is only
> > for use in DVSEC etc., is really weird.
> > 
> > IIUC, the PCI SIG controls the Vendor ID namespace, so I'm still
> > really confused about why it is not reserved there.  I emailed the PCI
> > SIG, but the footnote suggests to me that there some kind of history
> > here that I don't know.
> > 
> > Anyway, I think all we can do here is to put the definition in
> > include/linux/pci_ids.h as you did and hope 0x1e98 is never allocated
> > to another vendor.
> 
> Oh, true, I think this should be PCI_DVSEC_VENDOR_ID_CXL, because afaics
> it is still possible that 0x1e98 be used as a non-DVSEC vendor-id in
> some future device.
> 
> In other words I think the CXL specification usage of 0x1e98 is scoped
> as "DVSEC Vendor ID", not "Vendor ID".
> 
> However that would mean that a future 0x1e98 device could not publish
> DVSECs without colliding with CXL DVSECs.

FWIW, I pinged administration@pcisig.com and got the response that
"1E98h is not a VID in our system, but 1E98 has already been reserved
by CXL."

I wish there were a clearer public statement of this reservation, but
I interpret the response to mean that CXL is not a "Vendor", maybe due
to some strict definition of "Vendor," but that PCI-SIG will not
assign 0x1e98 to any other vendor.

So IMO we should add "#define PCI_VENDOR_ID_CXL 0x1e98" so that if we
ever *do* see such an assignment, we'll be more likely to flag it as
an issue.

Bjorn