On Wed, Nov 20, 2019 at 09:38:35AM -0400, Jason Gunthorpe wrote: > On Tue, Nov 19, 2019 at 10:59:20PM -0500, Jason Wang wrote: > > > > > The interface between vfio and userspace is > > > > based on virtio which is IMHO much better than > > > > a vendor specific one. userspace stays vendor agnostic. > > > > > > Why is that even a good thing? It is much easier to provide drivers > > > via qemu/etc in user space then it is to make kernel upgrades. We've > > > learned this lesson many times. > > > > For upgrades, since we had a unified interface. It could be done > > through: > > > > 1) switch the datapath from hardware to software (e.g vhost) > > 2) unload and load the driver > > 3) switch teh datapath back > > > > Having drivers in user space have other issues, there're a lot of > > customers want to stick to kernel drivers. > > So you want to support upgrade of kernel modules, but runtime > upgrading the userspace part is impossible? Seems very strange to me. It's still true, you have to kill userspace to update a shared library. Not to mention that things like rust encourage static builds so you can't update a library even if you were willing to risk doing that. > > > This is why we have had the philosophy that if it doesn't need to be > > > in the kernel it should be in userspace. > > > > Let me clarify again. For this framework, it aims to support both > > kernel driver and userspce driver. For this series, it only contains > > the kernel driver part. What it did is to allow kernel virtio driver > > to control vDPA devices. Then we can provide a unified interface for > > all of the VM, containers and bare metal. For this use case, I don't > > see a way to leave the driver in userspace other than injecting > > traffic back through vhost/TAP which is ugly. > > Binding to the other kernel virtio drivers is a reasonable > justification, but none of this comes through in the patch cover > letters or patch commit messages. Yea this could have been communicated better. > > > > That has lots of security and portability implications and isn't > > > > appropriate for everyone. > > > > > > This is already using vfio. It doesn't make sense to claim that using > > > vfio properly is somehow less secure or less portable. > > > > > > What I find particularly ugly is that this 'IFC VF NIC' driver > > > pretends to be a mediated vfio device, but actually bypasses all the > > > mediated device ops for managing dma security and just directly plugs > > > the system IOMMU for the underlying PCI device into vfio. > > > > Well, VFIO have multiple types of API. The design is to stick the VFIO > > DMA model like container work for making DMA API work for userspace > > driver. > > Well, it doesn't, that model, for security, is predicated on vfio > being the exclusive owner of the device. For instance if the kernel > driver were to perform DMA as well then security would be lost. The assumption at least IFC driver makes is that the kernel driver does no DMA. > > > I suppose this little hack is what is motivating this abuse of vfio in > > > the first place? > > > > > > Frankly I think a kernel driver touching a PCI function for which vfio > > > is now controlling the system iommu for is a violation of the security > > > model, and I'm very surprised AlexW didn't NAK this idea. > > > > > > Perhaps it is because none of the patches actually describe how the > > > DMA security model for this so-called mediated device works? :( > > > > > > Or perhaps it is because this submission is split up so much it is > > > hard to see what is being proposed? (I note this IFC driver is the > > > first user of the mdev_set_iommu_device() function) > > > > Are you objecting the mdev_set_iommu_deivce() stuffs here? > > I'm questioning if it fits the vfio PCI device security model, yes. If you look at the IFC patch you'll find it doesn't do DMA, that's what makes it secure. > > > > It is kernel's job to abstract hardware away and present a unified > > > > interface as far as possible. > > > > > > Sure, you could create a virtio accelerator driver framework in our > > > new drivers/accel I hear was started. That could make some sense, if > > > we had HW that actually required/benefited from kernel involvement. > > > > The framework is not designed specifically for your card. It tries to be > > generic to support every types of virtio hardware devices, it's not > > tied to any bus (e.g PCI) and any vendor. So it's not only a question > > of how to slice a PCIE ethernet device. > > That doesn't explain why this isn't some new driver subsystem and > instead treats vfio as a driver multiplexer. > > Jason That motivation's missing. -- MST
