Re: [RFC PATCH v2 4/7] iommufd: Associate kvm pointer to iommufd ctx

[Sean Christopherson <seanjc@google.com>]

On Mon, Jun 24, 2024, Jason Gunthorpe wrote:
> On Mon, Jun 24, 2024 at 09:53:00AM -0700, Sean Christopherson wrote:
> > If kvm_pinned_vmid_{get,put}() are implemented directly by KVM ARM, then I don't
> > have any immediate concerns, as KVM ARM is a long, long way from being able to
> > isolate KVM from the core kernel.  
> 
> I think that is a reasonable thing, I also don't really see VMID as
> being general. We will have to figure out how to ensure that the KVM
> FD we got is an ARM KVM FD..

Isn't the caller in ARM specific code?  I was assuming kvm_pinned_vmid_{get,put}()
would simply not exist for non-ARM builds.

> > That said, I find the on-demand pinning to be very odd.  IIUC, if KVM runs out
> > of pinnable VMIDs, attaching a device to the KVM+iommu will fail.  Failing an
> > iommufd operation because of a (potentially transient) KVM resource issue is
> > rather unpleasant.
> 
> It is kind of subtle, but the only thing that will consume VMIDs is
> IOMMUFD operations that are working with nested translation but not
> providing KVMs. This is a pretty small blast radius - ie a specific
> qemu will fail to start - that I think we can tolerate it.
> 
> More normal iommu operation will not require VMIDs so things like
> driver attaching/etc is fine.
> 
> > And assuming that pinnable VMIDs are a somewhat scarce resource, it wouldn't
> > suprise me if someone wanted to add cgroup integration, e.g. similar to the
> > misc cgroup that's used to manage SEV(-ES) ASIDs on KVM AMD (IIUC, an SEV ASID
> > is analagous to an ARM VMID).
> 
> Yeah, but if someone is using such a cgroup then I expect they will
> also have an up to date VMM that doesn't trigger this VMID allocation
> in the first place...

I suspect we're talking about two different things.  Either that, or I am really
lost.

> > Rather than on-demand pinning, would it make sense to have KVM provide an ioctl()
> > (or capability, or VM type) to let userspace pin a VM's VMID?  That would allow
> > for a much saner failure mode, and I suspect would be cleaner in general for iommufd.
> 
> The point of this mechanism is to support using this iommufd feature
> without a KVM at all. We could instead prevent this directly 100% of
> the time, but it means that HW with this BTM capability would not run
> the legacy VMMs at all, so I'm not that keen on it..
> 
> When a KVM is present then the iommu needs to adopt the VMID of KVM,
> and that should have a mechanism to ensure the VMID is valid so long
> as the IOMMU is using it (eg because the KVM FD is open)

Right, and that's what I'm referring to as "on-demand pinning".  For the IOMMU
to adopt a KVM VMID, the VMID needs to be pinned (or KVM would need to notify
the IOMMU every time the VMID changed), i.e. every KVM+IOMMU pair pins a VMID
that is managed by KVM.

Hmm, kvm_arm_pinned_vmid_get() doesn't fail, it just falls back to VMID=0.  Which
seems odd.