linux-kernel - Re: [PATCH kernel v3] PCI/DOE: Support discovery version 2

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]

Date: Thu, 7 Mar 2024 17:09:36 +1100
From: Alexey Kardashevskiy <aik@....com>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>,
 linux-pci@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
 Ira Weiny <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 Lukas Wunner <lukas@...ner.de>
Subject: Re: [PATCH kernel v3] PCI/DOE: Support discovery version 2



On 7/3/24 16:06, Kuppuswamy Sathyanarayanan wrote:
> 
> On 3/6/24 6:20 PM, Alexey Kardashevskiy wrote:
>> PCIe r6.1, sec 6.30.1.1 defines a "DOE Discovery Version" field in
>> the DOE Discovery Request Data Object Contents (3rd DW) as:
>>
>> 15:8 DOE Discovery Version – must be 02h if the Capability Version in
>> the Data Object Exchange Extended Capability is 02h or greater.
> 
> Does this change fix any functional issue? If yes, please add Fixes:
> tag and probably CC stable?

The issue is that before this patch the DOE driver was following PCIe 
r6.0 and it was not working on a device with DOE cap v2 added in r6.1. 
Is it "fixes"?


>>
>> Add support for the version on devices with the DOE v2 capability.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@....com>
>> ---
> Change wise looks fine to me.
> 
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>

Thanks!

>> Changes:
>> v3:
>> * updated subject line
>> * dropped "DISCOVER_" from the new field macro
>>
>> v2:
>> * added the section number to the commit log
>> ---
>>   include/uapi/linux/pci_regs.h |  1 +
>>   drivers/pci/doe.c             | 11 ++++++++---
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>> index a39193213ff2..fbca743b2b86 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -1144,6 +1144,7 @@
>>   #define PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH		0x0003ffff
>>   
>>   #define PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX		0x000000ff
>> +#define PCI_DOE_DATA_OBJECT_DISC_REQ_3_VER		0x0000ff00
>>   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID		0x0000ffff
>>   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL		0x00ff0000
>>   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX	0xff000000
>> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
>> index 61f0531d2b1d..c94c2b0767f3 100644
>> --- a/drivers/pci/doe.c
>> +++ b/drivers/pci/doe.c
>> @@ -381,11 +381,13 @@ static void pci_doe_task_complete(struct pci_doe_task *task)
>>   	complete(task->private);
>>   }
>>   
>> -static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u16 *vid,
>> +static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 capver, u8 *index, u16 *vid,
>>   			     u8 *protocol)
>>   {
>>   	u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
>> -				    *index);
>> +				    *index) |
>> +			 FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_VER,
>> +				    (capver >= 2) ? 2 : 0);
>>   	__le32 request_pl_le = cpu_to_le32(request_pl);
>>   	__le32 response_pl_le;
>>   	u32 response_pl;
>> @@ -419,13 +421,16 @@ static int pci_doe_cache_protocols(struct pci_doe_mb *doe_mb)
>>   {
>>   	u8 index = 0;
>>   	u8 xa_idx = 0;
>> +	u32 hdr = 0;
>> +
>> +	pci_read_config_dword(doe_mb->pdev, doe_mb->cap_offset, &hdr);
>>   
>>   	do {
>>   		int rc;
>>   		u16 vid;
>>   		u8 prot;
>>   
>> -		rc = pci_doe_discovery(doe_mb, &index, &vid, &prot);
>> +		rc = pci_doe_discovery(doe_mb, PCI_EXT_CAP_VER(hdr), &index, &vid, &prot);
>>   		if (rc)
>>   			return rc;
>>   
> 

-- 
Alexey