Re: [PATCH v10 1/7] PCI: initialize and release SR-IOV capability

Randy Dunlap
2009-03-06T22:03:33Z

Matthew Wilcox wrote:
> On Fri, Feb 20, 2009 at 02:54:42PM +0800, Yu Zhao wrote:
>> +config PCI_IOV
>> + bool "PCI IOV support"
>> + depends on PCI
>> + select PCI_MSI
>
> My understanding is that having 'select' of a config symbol that the
> user can choose is bad. I think we should probably make this 'depends
> on PCI_MSI'.

Ack.

> PCI MSI can also be disabled at runtime (and Fedora do by default).
> Since SR-IOV really does require MSI, we need to put in a runtime check
> to see if pci_msi_enabled() is false.
>
> We don't depend on PCIEPORTBUS (a horribly named symbol). Should we?
> SR-IOV is only supported for PCI Express machines. I'm not sure of the
> right answer here, but I thought I should raise the question.
>
>> + help
>> + PCI-SIG I/O Virtualization (IOV) Specifications support.
>> + Single Root IOV: allows the Physical Function driver to enable
>> + the hardware capability, so the Virtual Function is accessible
>> + via the PCI Configuration Space using its own Bus, Device and
>> + Function Numbers. Each Virtual Function also has the PCI Memory
>> + Space to map the device specific register set.

Too spec. and implementation specific for users.

> I'm not convinced this is the most helpful we could be to the user who's
> configuring their own kernel. How about something like this? (Randy, I
> particularly look to you to make my prose less turgid).
>
> help
> IO Virtualisation is a PCI feature supported by some devices
z ;)
> which allows you to create virtual PCI devices and assign them
> to guest OSes. This option needs to be selected in the host
> or Dom0 kernel, but does not need to be selected in the guest
> or DomU kernel. If you don't know whether your hardware supports
> it, you can check by using lspci to look for the SR-IOV capability.
>
> If you have no idea what any of that means, it is safe to
> answer 'N' here.

That's certainly more readable and user-friendly.
I don't know what else it needs. Looks good to me.

~Randy