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

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: <da77e691-d1a1-5c69-c23e-69656a82f8f5@codeaurora.org>
Date:   Fri, 11 Aug 2017 12:54:47 -0400
From:   Sinan Kaya <okaya@...eaurora.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     linux-pci@...r.kernel.org, timur@...eaurora.org,
        alex.williamson@...hat.com, linux-arm-msm@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH V9 1/2] PCI: handle CRS returned by device after FLR

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.