linux-kernel - Re: [RFC PATCH 2/2] PCI: Add basic Compute eXpress Link DVSEC decode

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

Message-ID: <55BAB3D3-5C7F-455A-96E6-7C5672510021@linux.intel.com>
Date:   Fri, 15 May 2020 15:28:17 -0700
From:   "Sean V Kelley" <sean.v.kelley@...ux.intel.com>
To:     "Bjorn Helgaas" <helgaas@...nel.org>
Cc:     bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/2] PCI: Add basic Compute eXpress Link DVSEC decode

Hi,

On 15 May 2020, at 14:04, Bjorn Helgaas wrote:

> On Fri, May 15, 2020 at 10:55:28AM -0700, Sean V Kelley wrote:
>> Compute eXpress Link is a new CPU interconnect created with
>> workload accelerators in mind. The interconnect relies on PCIe 
>> Electrical
>> and Physical interconnect for communication. CXL devices enumerate to 
>> the
>> OS as an ACPI-described PCIe Root Complex Integrated Endpoint.
>>
>> This patch introduces the bare minimum support by simply looking for 
>> and
>> caching the DVSEC CXL Extended Capability. Currently, only CXL.io 
>> (which
>> is mandatory to be configured by BIOS) is enabled. In future, we will
>> also add support for CXL.cache and CXL.mem.
>
> This looks fine, but AFAICT, it doesn't *do* anything yet (except
> print a few things to dmesg).  We don't normally merge code until it
> adds some new functionality.  So just FYI that I'll wait until that
> new functionality comes along and then merge this as part of that
> series.  But let me know if I'm missing something.
>

Correct.  Understood. I’ve additional changes for CXL.mem/.cache.  
I’ll queue those up.

>> +	dev_info(&dev->dev, "CXL: Cache%c IO%c Mem%c Viral%c HDMCount 
>> %d\n",
>> +		 (cap & PCI_CXL_CACHE) ? '+' : '-',
>> +		 (cap & PCI_CXL_IO) ? '+' : '-',
>> +		 (cap & PCI_CXL_MEM) ? '+' : '-',
>> +		 (cap & PCI_CXL_VIRAL) ? '+' : '-',
>> +		 PCI_CXL_HDM_COUNT(cap));
>
> These could use pci_info() and FLAG(), as in pcie_init().

Will do.

>
>> +	dev_info(&dev->dev, "CXL: cap ctrl status ctrl2 status2 lock\n");
>> +	dev_info(&dev->dev, "CXL: %04x %04x %04x %04x %04x %04x\n",
>> +		 cap, ctrl, status, ctrl2, status2, lock);
>> +}
>
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -315,6 +315,7 @@ struct pci_dev {
>>  	u16		aer_cap;	/* AER capability offset */
>>  	struct aer_stats *aer_stats;	/* AER stats for this device */
>>  #endif
>> +	u16		cxl_cap;	/* CXL capability offset */
>
> Wrap in #ifdef PCI_CXL.

Will do.

Thanks,

Sean