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

[Randy Dunlap]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/