> From: Alex Williamson > Sent: Friday, March 6, 2020 1:34 AM > > Hi Kevin, > > Sorry for the delay, I've been out on PTO... > > On Tue, 25 Feb 2020 02:33:27 +0000 > "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote: > > > > From: Alex Williamson > > > Sent: Thursday, February 20, 2020 2:54 AM > > > > > > Changes since v1 are primarily to patch 3/7 where the commit log is > > > rewritten, along with option parsing and failure logging based on > > > upstream discussions. The primary user visible difference is that > > > option parsing is now much more strict. If a vf_token option is > > > provided that cannot be used, we generate an error. As a result of > > > this, opening a PF with a vf_token option will serve as a mechanism of > > > setting the vf_token. This seems like a more user friendly API than > > > the alternative of sometimes requiring the option (VFs in use) and > > > sometimes rejecting it, and upholds our desire that the option is > > > always either used or rejected. > > > > > > This also means that the VFIO_DEVICE_FEATURE ioctl is not the only > > > means of setting the VF token, which might call into question whether > > > we absolutely need this new ioctl. Currently I'm keeping it because I > > > can imagine use cases, for example if a hypervisor were to support > > > SR-IOV, the PF device might be opened without consideration for a VF > > > token and we'd require the hypservisor to close and re-open the PF in > > > order to set a known VF token, which is impractical. > > > > > > Series overview (same as provided with v1): > > > > Thanks for doing this! > > > > > > > > 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 > > > > Can you provide a link to the DPDK discussion? > > There's a thread here which proposed an out-of-tree driver that enables > a parallel sr-iov enabling interface for a vfio-pci own device. > Clearly I felt strongly about it ;) > > https://patches.dpdk.org/patch/58810/ > > Also, documentation for making use of an Intel FPGA device with DPDK > requires the PF bound to igb_uio to support enabling SR-IOV: > > https://doc.dpdk.org/guides/bbdevs/fpga_lte_fec.html thanks. it is useful. > > > > 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. > > > > Just a loud thinking. While the motivation of VF token sounds reasonable > > to me, I'm curious why the same concern is not raised in other usages. > > For example, there is no such design in virtio framework, where the > > virtio device could also be restarted, putting in separate process (vhost- > user), > > and even in separate VM (virtio-vhost-user), etc. Of course the para- > > virtualized attribute of virtio implies some degree of trust, but as you > > mentioned many SR-IOV implementations support VF->PF communication > > which also implies some level of trust. It's perfectly fine if VFIO just tries > > to do better than other sub-systems, but knowing how other people > > tackle the similar problem may make the whole picture clearer. 😊 > > > > +Jason. > > We can follow the thread with Jason, but I can't really speak to > whether virtio needs something similar or doesn't provide enough PF > access to be concerned. If they need a similar solution, we can > collaborate, but the extension we're defining here is specifically part > of the vfio-pci ABI, so it might not be easily portable to virtio. > > > > 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. > > > > I'm wondering whether the token idea can be used beyond SR-IOV, e.g. > > (1) we may allow vfio user space to manage Scalable IOV in the future, > > which faces the similar challenge between the PF and mdev; (2) the > > token might be used as a canonical way to replace off-tree acs-override > > workaround, say, allowing the admin to assign devices within the > > same iommu group to different VMs which trust each other. I'm not > > sure how much complexity will be further introduced, but it's greatly > > appreciated if you can help think a bit and if feasible abstract some > > logic in vfio core layer for such potential usages... > > I don't see how this can be used for ACS override. Lacking ACS, we > must assume lack of DMA isolation, which results in our IOMMU grouping. > If we split IOMMU groups, that implies something that doesn't exist. A > user can already create a process that can own the vfio group and pass > vfio devices to other tasks, with the restriction of having a single > DMA address space. If there is DMA isolation, then an mdev solution > might be better, but given the IOMMU integration of SIOV, I'm not sure > why the devices wouldn't simply be placed in separate groups by the > IOMMU driver. Thanks, You are right. I overlooked the single DMA address space limitation. > > Alex > > > > 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 > > > > > > RFC: > > > > https://lore.kernel.org/lkml/158085337582.9445.17682266437583505502.stg > > > it@xxxxxxxxxx/ > > > v1: > > > > https://lore.kernel.org/lkml/158145472604.16827.15751375540102298130.st > > > git@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 | 383 > > > +++++++++++++++++++++++++++++++++-- > > > 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, 426 insertions(+), 28 deletions(-) > >
