linux-kernel - Re: [PATCH v13 02/25] PCI/CXL: Introduce pcie_is

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

Message-ID: <a5d4f527-1a66-4c1e-9311-b8f3fe3badf1@amd.com>
Date: Mon, 8 Dec 2025 09:26:29 -0600
From: "Bowman, Terry" <terry.bowman@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
 alison.schofield@...el.com, dan.j.williams@...el.com, bhelgaas@...gle.com,
 shiju.jose@...wei.com, ming.li@...omail.com,
 Smita.KoralahalliChannabasappa@....com, rrichter@....com,
 dan.carpenter@...aro.org, PradeepVineshReddy.Kodamati@....com,
 lukas@...ner.de, Benjamin.Cheatham@....com,
 sathyanarayanan.kuppuswamy@...ux.intel.com, linux-cxl@...r.kernel.org,
 alucerop@....com, ira.weiny@...el.com, linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org
Subject: Re: [PATCH v13 02/25] PCI/CXL: Introduce pcie_is_cxl()

On 12/5/2025 6:45 PM, Bjorn Helgaas wrote:
> On Mon, Nov 03, 2025 at 06:09:38PM -0600, Terry Bowman wrote:
>> CXL and AER drivers need the ability to identify CXL devices.
>>
>> Introduce set_pcie_cxl() with logic checking for CXL.mem or CXL.cache
>> status in the CXL Flexbus DVSEC status register. 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
>> CXL.cache and CXl.mem status.
>>
>> In the case the device is an EP or USP, call set_pcie_cxl() on behalf of
>> the parent downstream device. Once a device is created there is
>> possibilty the parent training or CXL state was updated as well. This
>> will make certain the correct parent CXL state is cached.
> 
> See question at comment below.
> 
> s/on behalf of the parent downstream device/for the parent bridge/
> s/possibilty/possibility/
> 

Ok

>> +++ b/drivers/pci/probe.c
>> @@ -1709,6 +1709,33 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
>>  		dev->is_thunderbolt = 1;
>>  }
>>  
>> +static void set_pcie_cxl(struct pci_dev *dev)
>> +{
>> +	struct pci_dev *parent;
>> +	u16 dvsec = pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
>> +					      PCI_DVSEC_CXL_FLEXBUS_PORT);
>> +	if (dvsec) {
>> +		u16 cap;
>> +
>> +		pci_read_config_word(dev, dvsec + PCI_DVSEC_CXL_FLEXBUS_STATUS_OFFSET, &cap);
>> +
>> +		dev->is_cxl = FIELD_GET(PCI_DVSEC_CXL_FLEXBUS_STATUS_CACHE_MASK, cap) ||
>> +			FIELD_GET(PCI_DVSEC_CXL_FLEXBUS_STATUS_MEM_MASK, cap);
> 
> Wrap to fit in 80 columns like the rest of the file.
> 

Ok

> Not sure the "_MASK" and "_OFFSET" on the end of all these #defines is
> really needed.  Other items in pci_regs.h typically don't include
> them, and these names get really long.  
> 

These were moved over from local CXL header. As a result they are not 
consistent in the usage of offset and mask. Would you like for this to 
be made the same ? This would be to change all points where used.


>> +	}
>> +
>> +	if (!pci_is_pcie(dev) ||
>> +	    !(pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT ||
>> +	      pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM))
>> +		return;
> 
> Maybe could do the pci_is_pcie() check first, before the
> pci_find_dvsec_capability(), so we could avoid that search, e.g.,
> 

Good idea.

>   if (!pci_is_pcie(dev))
>     return;
> 
>   dvsec = pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL, ...);
> 
>> +	/*
>> +	 * Update parent's CXL state because alternate protocol training
>> +	 * may have changed
> 
> What is the event that changes alternate protocol training?  The
> commit log says "once a device is created", but I don't know what that
> means in terms of hardware.
> 

There is potential an upstream device (switch) was hotpluged and in the 
case of the alt training retries may not be correctly cached in pci_dev::is_cxl.

>> +	 */
>> +	parent = pci_upstream_bridge(dev);
>> +	set_pcie_cxl(parent);
>> +}

Thanks for reviewing. 

Note, this is the wrong series (the last 2 patches in this series failed to send). I resent here:

https://lore.kernel.org/linux-cxl/20251104170305.4163840-1-terry.bowman@amd.com/

Regards,
Terry