Re: [PATCH] PCI: handle pci_sriov_set_totalvfs(dev, 0)

[Alexander Duyck <alexander.h.duyck@intel.com>]

On 07/31/2014 09:57 AM, Edward Cree wrote:
> On 31/07/14 17:40, Alexander Duyck wrote:
>> If this PF-IOV mode is enabled what would be the layout of the PF
>> devices?  It seems like you should be able to scan for multiple PFs all
>> showing up on the same bus with a certain stride if you wanted to detect it.
> Alas, it's (once again) more complicated than that.  There's another
> mode, NIC partitioning, which has multiple PFs per port but connected
> with a VLAN aggregator rather than a v-switch, which means that each PF
> then can support VFs (the firmware limitation is that v-switches can't
> be stacked).
> So having multiple PFs on the port doesn't necessarily mean we can't do
> SR-IOV.

Okay, so this is purely a vswitch issue then.  If it is allocated for
the PF-IOV it isn't available for SR-IOV.  I think I understand now.

>>>> In addition this solution would
>>>> also resolve the fact that the driver wouldn't actually have to be
>>>> loaded for it to work so if someone were to load a driver that didn't
>>>> contain the fix they would be blocked from enabling SR-IOV as well.
>>> The current driver fails to probe the PF because it assumes the vswitch
>>> creation failure is fatal.  There should never be a driver that knows it
>>> can live without the vswitch but doesn't know that that breaks SR-IOV.
>>>
>>> -Edward
>> Is the vswitch a hard requirement for something other than SR-IOV?  If
>> not then maybe you should consider modifying the driver so it simply
>> disabled SR-IOV if you cannot allocate the vswitch instead of blocking
>> probe.
>>
> That's exactly what I'm trying to do with pci_sriov_set_totalvfs(dev,
> 0); that's the whole point.

It sounds reasonable.  My only real concern is the fact that the error
reported when attempting to enable the interface will be a ERANGE when
really a more appropriate error return might be something like a EBUSY
since it is the vswitch that is occupied.

You might want to take a look at modifying sriov_numvfs_store so that if
the only option is 0 you return EBUSY for all non-zero values requested.
 This way it will be more in sync with how we handle the case of
attempting to update the number of VFs when SR-IOV is already enabled.

Thanks,

Alex