On Fri, Mar 20, 2009 at 03:53:12AM +0800, Matthew Wilcox wrote: > On Wed, Mar 11, 2009 at 03:25:42PM +0800, Yu Zhao wrote: > > +config PCI_IOV > > + bool "PCI IOV support" > > + depends on PCI > > + help > > + PCI-SIG I/O Virtualization (IOV) Specifications support. > > + Single Root IOV: allows the creation of virtual PCI devices > > + that share the physical resources from a real device. > > + > > + When in doubt, say N. > > It's certainly shorter than my text, which is nice. But I think it > still has too much spec-ese and not enough explanation. How about: > > help > I/O Virtualization is a PCI feature supported by some devices > which allows them to create virtual devices which share their > physical resources. > > If unsure, say N. Yes, it's more user-friendly. > > + list_for_each_entry(pdev, &dev->bus->devices, bus_list) > > + if (pdev->is_physfn) > > + break; > > + if (list_empty(&dev->bus->devices) || !pdev->is_physfn) > > + pdev = NULL; > > This is still wrong. If the 'break' condition is not hit, pdev is > pointing to garbage, not to the last pci_dev in the list. Yes, you are right. I should think it over after you commented on it last time. So it looks like we need to make it as: ctrl = 0; list_for_each_entry(pdev, &dev->bus->devices, bus_list) if (pdev->is_physfn) goto found; pdev = NULL; if (pci_ari_enabled(dev->bus)) ctrl |= PCI_SRIOV_CTRL_ARI; found: pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, ctrl); ... > > @@ -270,6 +278,7 @@ struct pci_dev { > > struct list_head msi_list; > > #endif > > struct pci_vpd *vpd; > > + struct pci_sriov *sriov; /* SR-IOV capability related */ > > Should be ifdeffed? Yes, will do. Thank you for reviewing it. The patch series was applied on Xen Domain0 tree 2 days ago, and I'll carry your comments back to Xen tree too. -- 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