On 2/21/2017 3:51 PM, Alex Williamson wrote: > On Tue, 21 Feb 2017 12:04:24 -0500 > Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote: > >> Hi Alex, >> >> I'm coming back to work on this. >> >> On 10/3/2016 1:37 AM, Sinan Kaya wrote: >>> An endpoint is allowed to issue CRS following an FLR request to indicate >>> that it is not ready to accept new requests. Changing the polling mechanism >>> in FLR wait function to go read the vendor ID instead of the command/status >>> register. A CRS indication will only be given if the address to be read is >>> vendor ID. >>> >>> Signed-off-by: Sinan Kaya <okaya@xxxxxxxxxxxxxx> >>> --- >>> drivers/pci/pci.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >>> index c8749b9..7580b00 100644 >>> --- a/drivers/pci/pci.c >>> +++ b/drivers/pci/pci.c >>> @@ -3725,7 +3725,8 @@ static void pci_flr_wait(struct pci_dev *dev) >>> >>> do { >>> msleep(100); >>> - pci_read_config_dword(dev, PCI_COMMAND, &id); >> >> Your comment here puzzled me. >> >> https://patchwork.kernel.org/patch/8331851/ >> >> "Self nak on this one, didn't account for VFs not implementing the first >> dword. Thanks," >> >> I'm trying to add Configuration Request Retry Status (CRS) support to FLR >> with this patch. >> >> Basically, the root port will return 0xFFFF0001 only when a config read >> request is sent to the vendor ID register and CRS visibility is set. >> The SW needs to poll until this special read ID disappears. See the >> implementation note on Configuration Request Retry Status in PCIE >> specification for details. >> >> pci_bus_read_dev_vendor_id implements this loop for us. >> >> You are saying that there are VFs that do not implement vendor ID register. >> Can you give some history on this? > > SR-IOV spec rev 1.1, 3.4.1.1 & 3.4.1.2, Vendor ID and Device ID fields > for the VF return 0xFFFF when read. The "Virtualization Intermediary" > is supposed to use the vendor ID from the PF and the device ID defined > in the PF SR-IOV capability. Interesting. Since lspci was showing the correct vendor id and device id, I assumed that it is coming from offset 0. Maybe, the right thing is to figure out if this is a virtual function or not. If it is a physical function, check the CRS first before reading the command register in the existing loop. > >> >>> + pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &id, >>> + 60 * 1000); >>> } while (i++ < 10 && id == ~0); > > pci_bus_read_dev_vendor_id() seems like it will return false with an > id value of ~0 for a functional VF, so this loop will spin longer than > necessary and report an invalid error. Patch 1/2 from this series > would cause pci_dev_restore() to be a no-op on VFs. Thanks, > > Alex > > -- 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. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html