On Mon, Jun 07, 2021 at 09:41:48AM -0600, Alex Williamson wrote: > You're calling this an admin knob, which to me suggests a global module > option, so are you trying to implement both an administrator and a user > policy? ie. the user can create scenarios where access to wbinvd might > be justified by hardware/IOMMU configuration, but can be limited by the > admin? Could be a per-device sysfs too. I'm not really sure what is useful here. > For example I proposed that the ioasidfd would bear the responsibility > of a wbinvd ioctl and therefore validate the user's access to enable > wbinvd emulation w/ KVM, so I'm assuming this module option lives > there. Right, this is what I was thinking > What then is "automatic" mode? The user cannot create a non-coherent > IOASID with a non-coherent device if the IOMMU supports no-snoop > blocking? Do they get a failure? Does it get silently promoted to > coherent? "automatic" was just a way to keep the API the same as today. Today if the IOMMU can block no-snoop then vfio disables wbinvd. To get the same level of security automatic mode would detect that vfio would have blocked wbinvd because the IOMMU can do it, and then always block it. It makes sense if there is an admin knob, as the admin could then move to an explict enable/disable to get functionality they can't get today. > In "disable" mode, I think we're just narrowing the restriction > further, a non-coherent capable device cannot be used except in a > forced coherent IOASID. I wouldn't say "cannot be used" - just you can't get access to wbinvd. It is up to qemu if it wants to proceed or not. There is no issue with allowing the use of no-snoop and blocking wbinvd, other than some drivers may malfunction. If the user is certain they don't have malfunctioning drivers then no issue to go ahead. The current vfio arrangement (automatic) maximized compatability. The enable/disable options provide for max performance and max security as alternative targets. > > It is the strenth of Paolo's model that KVM should not be able to do > > optionally less, not more than the process itself can do. > > I think my previous reply was working towards those guidelines. I feel > like we're mostly in agreement, but perhaps reading past each other. Yes, I think I said we were agreeing :) > Nothing here convinced me against my previous proposal that the > ioasidfd bears responsibility for managing access to a wbinvd ioctl, > and therefore the equivalent KVM access. Whether wbinvd is allowed or > no-op'd when the use has access to a non-coherent device in a > configuration where the IOMMU prevents non-coherent DMA is maybe still > a matter of personal preference. I think it makes the software design much simpler if the security check is very simple. Possessing a suitable device in an ioasid fd container is enough to flip on the feature and we don't need to track changes from that point on. We don't need to revoke wbinvd if the ioasid fd changes, for instance. Better to keep the kernel very simple in this regard. Seems agreeable enough that there is something here to explore in patches when the time comes Jason