On 20 Apr 2020, at 10:47, Bjorn Helgaas wrote:
Hi Jay, thanks for taking a look at this!
On Mon, Apr 20, 2020 at 09:21:27AM -0700, Sean V Kelley wrote:
On 18 Apr 2020, at 1:36, Jay Fang wrote:
On 2020/4/15 8:47, Sean V Kelley wrote:
[1] https://www.computeexpresslink.org/
Signed-off-by: Sean V Kelley <sean.v.kelley@xxxxxxxxxxxxxxx>
---
lib/header.h | 20 +++
+
+static int
+is_cxl_cap(struct device *d, int where)
+{
+ u32 hdr;
+ u16 w;
+
+ if (!config_fetch(d, where + PCI_DVSEC_HEADER1, 8))
+ return 0;
+
+ /* Check for supported Vendor */
+ hdr = get_conf_long(d, where + PCI_DVSEC_HEADER1);
+ w = BITS(hdr, 0, 16);
+ if (w != PCI_VENDOR_ID_INTEL)
I don't think here checking is quite right. Does only Intel support
CXL?
Other Vendors should also be considered.
In the absence of currently available hardware, I was attempting to
limit
false positive noise. I’m happy to avoid the check on the vendor
if there
were to exist a definitive supported list. Bjorn suggested that I
check for
vendor ID with DVSEC ID for now. As hardware enters the market, I
can
surely revise this with an update when the CXL group publishes a
list.
The Vendor ID check cannot be avoided. Vendor IDs are allocated by
the PCI-SIG, but the DVSEC ID is vendor-defined so there cannot be a
global "CXL capability" DVSEC ID.
CXL *could* work through the PCI-SIG and define a new CXL Extended
Capability that could make this generic. But apparently they've
chosen to use the DVSEC mechanism instead.
+ /* Check for Designated Vendor-Specific ID */
+ hdr = get_conf_long(d, where + PCI_DVSEC_HEADER2);
+ w = BITS(hdr, 0, 16);
+ if (w == PCI_DVSEC_ID)
However, this is slightly wrong. The name "PCI_DVSEC_ID" implies that
there can only be one DVSEC ID and it is vendor-independent, but
neither is true.
This value is allocated by Intel, so we must check for the Intel
Vendor ID first. And Intel may define several DVSEC capabilities for
different purposes, so the name should also mention CXL.
So this should be "PCI_DVSEC_INTEL_CXL" or something similar.
But that doesn't mean other vendors need to define their own DVSEC
IDs for CXL. The spec (PCIe r5.0, sec 7.9.6) says:
[The DVSEC Capability] allows PCI Express component vendors to use
the Extended Capability mechanism to expose vendor-specific
registers that can be present in components by a variety of
vendors.
Any vendor that implements this CXL DVSEC the same way Intel does can
tag it with PCI_VENDOR_ID_INTEL and PCI_DVSEC_INTEL_CXL.
Good point. I will revise.
Thanks,
Sean
Note that there are two types of vendor-specific capabilities:
1) Vendor-Specific Extended Capability (VSEC). The structure and
definition are defined by the *Function* Vendor ID (from offset 0 in
config space) and the VSEC ID in the capability.
2) Designated Vendor-Specific Extended Capability (DVSEC). The
structure and definition are defined by the *DVSEC* Vendor ID (from
the DVSEC capability, *not* the vendor of the Function) and the
DVSEC ID in the capability.
This CXL capability is the latter, of course.
Bjorn
