Re: [PATCH V9 1/2] PCI: handle CRS returned by device after FLR

[Sinan Kaya <okaya@codeaurora.org>]

On 8/10/2017 6:32 PM, Sinan Kaya wrote:
>> Alex was concerned that pci_bus_read_dev_vendor_id() could return
>> false ("no device here") with an ID value of ~0 for a functional VF
>> [1].
>>
>> I think that is indeed possible:
>>
>>   - a VF that is ready will return 0xffff as both Vendor ID and Device
>>     ID (SR-IOV r1.1, sec 3.4.1.1, .2), so the very first read in
>>     pci_bus_read_dev_vendor_id() would see 0xffffffff and return
>>     "false".
>>
>>   - a VF that needs more time will return CRS and we'll loop in
>>     pci_bus_read_dev_vendor_id() until it becomes ready, and we'll
>>     return "true".
>>
>> Falling into the code below for the "false" case probably will work,
>> but it's a little bit ugly because (a) we're using two mechanisms to
>> figure out when the device is ready for config requests, and (b) we
>> have to worry about VFs both in pci_bus_read_dev_vendor_id() and here
>> in the caller.
> OK, I'm open to improving the code.
> 
>> Here's an idea to make pci_bus_read_dev_vendor_id() work for both VFs
>> and PFs.  It can't distinguish the 0xffffffff from a VF vs one from a
>> non-existent device, but the caller might be able to pass that
>> information in, e.g., when we're enumerating and don't know whether
>> the device exists, we don't have a pci_dev and would use this:
> How about creating a pci_bus_wait_crs() function with the loop in 
> pci_bus_read_dev_vendor_id() and calling it from both places?
> 

I prototyped both of the options. I found pci_bus_wait_crs() to be cleaner
due to this. 

is_vf looked like another hack like mine to tap into the CRS handling
inside the pci_bus_read_dev_vendor_id().

I think the right thing is to make CRS handling a first class citizen rather
than hiding it and overriding functions with unnecessary parameters.

I'll post V10 in a minute. It passed my testing.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.