On 07/29/2013 01:32 PM, Don Dutile wrote: > On 07/29/2013 03:58 PM, Bjorn Helgaas wrote: >> On Sat, Jul 27, 2013 at 9:11 AM, Yinghai Lu<yinghai@xxxxxxxxxx> wrote: >>> After commit dc087f2f6a2925e81831f3016b9cbb6e470e7423 >>> (PCI: Simplify IOV implementation and fix reference count races) >>> VF need to be removed via virtfn_remove to make sure ref to PF >>> is put back. >>> >>> Some driver (like ixgbe) does not call pci_disable_sriov() if >>> sriov is enabled via /sys/.../sriov_numvfs setting. >>> ixgbe does allow driver for PF get detached, but still have VFs >>> around. >> >> Is this something ixgbe should be doing differently? >> >> I'm not 100% sold on the idea of the VFs staying active after the >> driver releases the PF. It seems asymmetric because I think the >> driver has to claim the PF to *enable* the VFs, but we don't disable >> them when releasing the PF. >> >> What's the use case for detaching the PF driver while the VFs are >> active? >> > VF's assigned to (KVM) guest (via device-assignment). > Virtually, it's as if the enet cable is unplugged to the VF in the > guest -- > the device is still there (the PF wasn't unplugged, just the driver > de-configured). > > Pre-sysfs-based configuration, the std way to configure the VFs into > a system was to unload the PF driver, and reload it with a vf-enabling > parameter > (like max_vfs=<n> in the case of ixgbe, igb). > Now, if someone unloaded the PF driver in the host, the unplanned removal > of the PF enabled the VF to crash the host (maybe AlexD can provide the > details how that occurred). > So, the solution was to 'pause' the VF operation and let packets drop > in the guest, and re-enable the VF operation if the PF driver was > re-configured. > > So, as I stated in previous postings, this patch is acceptable if > it doesn't cause a guest crash when a VF is assigned to a KVM guest. > If you tested this case, then please state as such in the posting. > If not, then can AlexD test this case ? > > - Don I actually haven't done much with direct assignment to guests. I believe it was Greg who did that work to fix this issue. I'm adding Jeff Kirsher to the CC. Perhaps he can pull this patch into a copy of the net tree and submit it to our validation team for testing to see if they end up being able to reproduce the kernel panic issue that was originally addressed by allowing the VFs to be persistent. Thanks, Alex >>> But how about PF get removed via /sys or pciehp finally? >>> >>> During hot-remove, VF will still hold one ref to PF and it >>> prevent PF to be removed. >>> That make the next hot-add fails, as old PF dev struct is still around. >>> >>> We need to add pci_disable_sriov() calling during stop PF . >>> >>> Need this one for v3.11 >> >> Don had a concern that there might be a regression here ... I'm a bit >> confused on the details, but you guys need to come to agreement that >> this doesn't make things worse. >> >>> Signed-off-by: Yinghai Lu<yinghai@xxxxxxxxxx> >>> Cc: Jiang Liu<liuj97@xxxxxxxxx> >>> Cc: Alexander Duyck<alexander.h.duyck@xxxxxxxxx> >>> Cc: Donald Dutile<ddutile@xxxxxxxxxx> >>> Cc: Greg Rose<gregory.v.rose@xxxxxxxxx> >>> >>> --- >>> drivers/pci/remove.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> Index: linux-2.6/drivers/pci/remove.c >>> =================================================================== >>> --- linux-2.6.orig/drivers/pci/remove.c >>> +++ linux-2.6/drivers/pci/remove.c >>> @@ -25,6 +25,8 @@ static void pci_stop_dev(struct pci_dev >>> pci_proc_detach_device(dev); >>> pci_remove_sysfs_dev_files(dev); >>> device_del(&dev->dev); >>> + /* remove VF, if PF driver skip that */ >>> + pci_disable_sriov(dev); >>> dev->is_added = 0; >>> } >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
