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

[Oliver Upton]

On Mon, Jun 24, 2024 at 03:01:48PM -0300, Jason Gunthorpe wrote:
> On Mon, Jun 24, 2024 at 10:54:37AM -0700, Sean Christopherson wrote:
> > > > 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.
> 
> I mean KVM will have already allocated and charged the cgroup for it's
> use of the VMID. The IOMMU side just has to match it, no second
> allocation of a VMID.
> 
> We wouldn't charge a cgroup for iommu and kvm sharing the same vmid.

I think the concern remains that an operator may want to limit the blast
radius of some runaway VMID allocation in a system. But you're right, a
well-intentioned VMM should wind up with a single charge for all the
stage-2's that used the VMID allocation.

> > > 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.
> 
> Ok, right, yes, the expectation is that KVM allocates a VMID at some
> point and it stays fixed for the life of that kvm.
> 
> If KVM can change VMID on the fly then that is a further complication
> :\
> 
> > Hmm, kvm_arm_pinned_vmid_get() doesn't fail, it just falls back to VMID=0.  Which
> > seems odd.

This is bleeding a bit of implementation detail where VMID=0 is known to
be reserved (thus invalid), it'd probably be better if the
implementation actually just returned an error.

VMID=0 is associated with the host's MMU context, which is relevant when
running {n,h}VHE mode, as the VMID tags TLB entries even if stage-2
translation is disabled (HCR_EL2.VM = 0).

-- 
Thanks,
Oliver