Re: [PATCH v10 02/17] PCI/CXL: Add pcie_is_cxl()

["Bowman, Terry" <terry.bowman@amd.com>]

On 8/9/2025 5:56 AM, Alejandro Lucero Palau wrote:
> 
> On 6/26/25 23:42, Terry Bowman wrote:
>> CXL and AER drivers need the ability to identify CXL devices.
>>
>> Add set_pcie_cxl() with logic checking for CXL Flexbus DVSEC presence. The
>> CXL Flexbus DVSEC presence is used because it is required for all the CXL
>> PCIe devices.[1]
>>
>> Add boolean 'struct pci_dev::is_cxl' with the purpose to cache the CXL
>> Flexbus presence.
>>
>> Add function pcie_is_cxl() to return 'struct pci_dev::is_cxl'.
>>
>> [1] CXL 3.1 Spec, 8.1.1 PCIe Designated Vendor-Specific Extended
>>      Capability (DVSEC) ID Assignment, Table 8-2
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
>> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> ---
>>   drivers/pci/probe.c           | 10 ++++++++++
>>   include/linux/pci.h           |  6 ++++++
>>   include/uapi/linux/pci_regs.h |  8 +++++++-
>>   3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 4b8693ec9e4c..5d3548648d5c 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1691,6 +1691,14 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
>>   		dev->is_thunderbolt = 1;
>>   }
>>   
>> +static void set_pcie_cxl(struct pci_dev *dev)
>> +{
>> +	u16 dvsec = pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
>> +					      PCI_DVSEC_CXL_FLEXBUS);
>> +	if (dvsec)
>> +		dev->is_cxl = 1;
> 
> 
> Hi Terry,
> 
> 
> Should not this check for the bits about port status like IO_Enabled?
> 
> 
> Maybe I'm confused with the goal, but we can know a device is a cxl one 
> without specifically looking at the FLEXBUS capability presence or not. 
> What this capability can tell, and what I think it is more interesting, 
> is if the link was successfully trained as CXL.io. From problems we have 
> had while testing our type2 device, if the training for CXL.io fails, 
> the device will use pcie and this bit will be 0, same if the device can 
> be configured for using pcie only.
> 
> 
> If this is what we really need to know, changing is_cxl to 
> is_cxl_enabled could be clearer.
> 
> 
> I come here after Dan suggesting to use this functionality for ensuring 
> the CXL device functionality is on, and it would require to inspect the 
> status instead of just the capability being present. Maybe I'm confused 
> because I remember some patches from Robert Richter dealing with 
> checking link up for enabling downstream ports, but I think the goal 
> here is different.
> 

Hi Alejandro,

I agree in large part. We need to check for training complete and any change 
in training needs to be reflected in is_cxl(). My understanding is this 
will be be added later in a following patch series. 

Dan, can you add your thoughts ?

-Terry