Re: [RFC PATCH V4] pci: virtio_pci: Add SR-IOV support for virtio_pci devices

[Alexander Duyck]

On Mon, Feb 26, 2018 at 2:38 PM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> On Mon, Feb 26, 2018 at 10:05:31AM -0800, Alexander Duyck wrote:
>> On Mon, Feb 26, 2018 at 9:48 AM, Rustad, Mark D <mark.d.rustad@xxxxxxxxx> wrote:
>> > Alex,
>> >
>> >> On Feb 26, 2018, at 7:26 AM, Alexander Duyck <alexander.duyck@xxxxxxxxx> wrote:
>> >>
>> >> Mark,
>> >>
>> >> In the future please don't put my "Reviewed-by" on a patch that I
>> >> haven't reviewed. I believe I reviewed one of the earlier patches, but
>> >> I hadn't reviewed this version.
>> >
>> > I'm very sorry. I completely spaced doing something about that. I think yours was the first Reviewed-by I ever had in this way. In the future I will remove such things from my changelog right after sending. Thanks for alerting me to what I had failed to do.
>> >
>> >> Also, after thinking about it over the weekend we may want to look at
>> >> just coming up with a truly "generic" solution that is applied to
>> >> SR-IOV capable devices that don't have a SR-IOV capable driver loaded
>> >> on them. That would allow us to handle the uio, vfio, pci-stub, and
>> >> virtio cases all in one fell swoop. I think us going though and
>> >> modifying one patch at a time to do this kind of thing isn't going to
>> >> scale.
>> >
>> > The notion of that kind of troubles me - at least pci-stub does. Having worked on ixgbe a bit, I have to wonder what kind of havoc would ensue if an ixgbe device were assigned to a guest, and an attempt was made to allocate VFs by the pci-stub. The guest could be running any version of the ixgbe driver, possibly even an old one that didn't support SR-IOV. Even if it did support SR-IOV, I don't know how it would respond to mailbox messages when it doesn't think it has VFs.
>>
>> The assumption here is that the root user knows what they are doing.
>
> There are tools that let non-root users load the stub.
>
> People use that to e.g.  prevent a native driver from loading while also
> assuming it won't break the kernel.
>
>
>> We have already had some discussion on this in regards to VFIO. My
>> thought is we look at adding a new PCI sysfs option called
>> "sriov_unmanaged_autoprobe" that would be similar to
>> "sriov_drivers_autoprobe" and is used to determine if we allow for
>> auto probing of the VFs into the host kernel when SR-IOV is enabled.
>
> I'm not sure how a global option can work for use-cases
> such as containers though.

It isn't global. It is per PF. There is a sysfs value per PF called
"sriov_drivers_autoprobe" that currently controls if VFs are
automatically bound by drivers from the kernel. What I am suggesting
is we split that value, and then add a new one called
"sriov_unmanaged_autoprobe" to handle the case where either there is
no driver on the PF or the driver loaded doesn't support SR-IOV. The
default behavior for it would be to be disabled by default so if VFs
are spawned by an unmanaged PF the drivers don't load on them by
default. The root user can toggle the sysfs entry if they want that
behavior to change.

The idea based on a request by Alex Williamson that we prevent VF
drivers from loading in the host if a PF is managed by user-space or a
firmware entity and we don't have a clear way of configuring VFs from
the kernel.

- Alex