On Thu, 19 Mar 2020 06:32:25 +0000 "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote: > > From: Alex Williamson <alex.williamson@xxxxxxxxxx> > > Sent: Thursday, March 12, 2020 5:58 AM > > > > Only minor tweaks since v2, GET and SET on VFIO_DEVICE_FEATURE are > > enforced mutually exclusive except with the PROBE option as suggested > > by Connie, the modinfo text has been expanded for the opt-in to enable > > SR-IOV support in the vfio-pci driver per discussion with Kevin. > > > > I have not incorporated runtime warnings attempting to detect misuse > > of SR-IOV or imposed a session lifetime of a VF token, both of which > > were significant portions of the discussion of the v2 series. Both of > > these also seem to impose a usage model or make assumptions about VF > > resource usage or configuration requirements that don't seem necessary > > except for the sake of generating a warning or requiring an otherwise > > unnecessary and implicit token reinitialization. If there are new > > thoughts around these or other discussion points, please raise them. > > > > Series overview (same as provided with v1): > > > > The synopsis of this series is that we have an ongoing desire to drive > > PCIe SR-IOV PFs from userspace with VFIO. There's an immediate need > > for this with DPDK drivers and potentially interesting future use > > cases in virtualization. We've been reluctant to add this support > > previously due to the dependency and trust relationship between the > > VF device and PF driver. Minimally the PF driver can induce a denial > > of service to the VF, but depending on the specific implementation, > > the PF driver might also be responsible for moving data between VFs > > or have direct access to the state of the VF, including data or state > > otherwise private to the VF or VF driver. > > > > To help resolve these concerns, we introduce a VF token into the VFIO > > PCI ABI, which acts as a shared secret key between drivers. The > > userspace PF driver is required to set the VF token to a known value > > and userspace VF drivers are required to provide the token to access > > the VF device. If a PF driver is restarted with VF drivers in use, it > > must also provide the current token in order to prevent a rogue > > untrusted PF driver from replacing a known driver. The degree to > > which this new token is considered secret is left to the userspace > > drivers, the kernel intentionally provides no means to retrieve the > > current token. > > > > Note that the above token is only required for this new model where > > both the PF and VF devices are usable through vfio-pci. Existing > > models of VFIO drivers where the PF is used without SR-IOV enabled > > or the VF is bound to a userspace driver with an in-kernel, host PF > > driver are unaffected. > > > > The latter configuration above also highlights a new inverted scenario > > that is now possible, a userspace PF driver with in-kernel VF drivers. > > I believe this is a scenario that should be allowed, but should not be > > enabled by default. This series includes code to set a default > > driver_override for VFs sourced from a vfio-pci user owned PF, such > > that the VFs are also bound to vfio-pci. This model is compatible > > with tools like driverctl and allows the system administrator to > > decide if other bindings should be enabled. The VF token interface > > above exists only between vfio-pci PF and VF drivers, once a VF is > > bound to another driver, the administrator has effectively pronounced > > the device as trusted. The vfio-pci driver will note alternate > > binding in dmesg for logging and debugging purposes. > > > > Please review, comment, and test. The example QEMU implementation > > provided with the RFC is still current for this version. Thanks, > > > > Alex > > The whole series looks good to me: > Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx> Thanks! > and confirm one understanding here, since it is not discussed anywhere. For > VM live migration with assigned VF device, it is not necessary to migrate the > VF token itself and actually we don't allow userspace to retrieve it. Instead, > Qemu just follows whatever token requirement on the dest to open the new > VF: could be same or different token as/from src, or even no token if PF > driver runs in kernel on dest. I suppose either combination could work, correct? That's correct. Thanks, Alex > > RFC: > > https://lore.kernel.org/lkml/158085337582.9445.17682266437583505502.stg > > it@xxxxxxxxxx/ > > v1: > > https://lore.kernel.org/lkml/158145472604.16827.15751375540102298130.st > > git@xxxxxxxxxx/ > > v2: > > https://lore.kernel.org/lkml/158213716959.17090.8399427017403507114.stg > > it@xxxxxxxxxx/ > > > > --- > > > > Alex Williamson (7): > > vfio: Include optional device match in vfio_device_ops callbacks > > vfio/pci: Implement match ops > > vfio/pci: Introduce VF token > > vfio: Introduce VFIO_DEVICE_FEATURE ioctl and first user > > vfio/pci: Add sriov_configure support > > vfio/pci: Remove dev_fmt definition > > vfio/pci: Cleanup .probe() exit paths > > > > > > drivers/vfio/pci/vfio_pci.c | 390 > > +++++++++++++++++++++++++++++++++-- > > drivers/vfio/pci/vfio_pci_private.h | 10 + > > drivers/vfio/vfio.c | 20 +- > > include/linux/vfio.h | 4 > > include/uapi/linux/vfio.h | 37 +++ > > 5 files changed, 433 insertions(+), 28 deletions(-) >
