linux-kernel - Re: [PATCH] fpga: dfl: pci: generalize find_dfls_by

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:   Mon, 15 Nov 2021 10:28:32 -0800
From:   Tom Rix <trix@...hat.com>
To:     Xu Yilun <yilun.xu@...el.com>
Cc:     hao.wu@...el.com, mdf@...nel.org, linux-fpga@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fpga: dfl: pci: generalize find_dfls_by_vsec()


On 11/14/21 5:25 PM, Xu Yilun wrote:
> On Sat, Nov 13, 2021 at 02:12:52PM -0800, trix@...hat.com wrote:
>> From: Tom Rix <trix@...hat.com>
>>
>> find_dfls_by_vsec() is a general dfl function.
>> Although dfl has multiple vendors, only Intel is supported.
>> Move vsec and vendor id to an array variable.
>> Other vendors can append the array to enable their support.
> As Hao mentioned, DVSEC could be a better solution if DFL should be
> present in components by a variety of vendors. This is not finally
> determined, but I think we should not add new features for VSEC now.

Can you expand what you mean by this ?

I am considering the n5010 usecase, the vendor is not intel and will go 
through this dfl function and always fail.

This is broken.

Either the function should be generalized or moved to an intel specific 
call.

Tom

>
> Thanks,
> Yilun
>
>> Signed-off-by: Tom Rix <trix@...hat.com>
>> ---
>>   drivers/fpga/dfl-pci.c | 31 ++++++++++++++++++++++++-------
>>   1 file changed, 24 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
>> index 4d68719e608f..9dc0815c8274 100644
>> --- a/drivers/fpga/dfl-pci.c
>> +++ b/drivers/fpga/dfl-pci.c
>> @@ -136,19 +136,36 @@ static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec)
>>   	return table;
>>   }
>>   
>> -static int find_dfls_by_vsec(struct pci_dev *pcidev, struct dfl_fpga_enum_info *info)
>> +struct dfl_vsec {
>> +	u16 vendor;
>> +	u16 id;
>> +};
>> +
>> +static struct dfl_vsec vsecs[] = {
>> +	{ PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_DFLS },
>> +};
>> +
>> +static int find_dfls_by_vsec(struct pci_dev *pcidev,
>> +			     struct dfl_fpga_enum_info *info)
>>   {
>>   	u32 bir, offset, vndr_hdr, dfl_cnt, dfl_res;
>>   	int dfl_res_off, i, bars, voff = 0;
>>   	resource_size_t start, len;
>>   
>> -	while ((voff = pci_find_next_ext_capability(pcidev, voff, PCI_EXT_CAP_ID_VNDR))) {
>> -		vndr_hdr = 0;
>> -		pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER, &vndr_hdr);
>> +	for (i = 0; i < ARRAY_SIZE(vsecs); i++) {
>> +		if (pcidev->vendor != vsecs[i].vendor)
>> +			continue;
>> +
>> +		while ((voff =
>> +			pci_find_next_ext_capability(pcidev, voff,
>> +						     PCI_EXT_CAP_ID_VNDR))) {
>> +			vndr_hdr = 0;
>> +			pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER,
>> +					      &vndr_hdr);
>>   
>> -		if (PCI_VNDR_HEADER_ID(vndr_hdr) == PCI_VSEC_ID_INTEL_DFLS &&
>> -		    pcidev->vendor == PCI_VENDOR_ID_INTEL)
>> -			break;
>> +			if (PCI_VNDR_HEADER_ID(vndr_hdr) == vsecs[i].id)
>> +				break;
>> +		}
>>   	}
>>   
>>   	if (!voff) {
>> -- 
>> 2.26.3