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

[Sean Christopherson <seanjc@google.com>]

On Mon, Jun 24, 2024, Oliver Upton wrote:
> 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
> > :\

Ya, as written today, KVM doesn't assign a VMID when the VM is created, and instead
allocates VMIDs on-demand when a vCPU is run.

The KVM changes in this series allow "pinning" the currently assigned VMID, i.e.
tries to address that further complication.  But because of the on-demand
allocation, there might not be a currently assigned VMID for VM, or the VMID might
be stale, i.e. re-assigned to a different VM.

Thus, kvm_arm_pinned_vmid_get() can effectively trigger VMID allocations, and
thus cgroup charging and failure.

If I'm reading the ARM code correctly, the intent is to cycle through VMIDs as  
necessary so that it's possible for every actively running VM to have a VMID.
And maybe also to also minimize the number of TLB + I$ invalidations?

> > 
> > > 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.

Oof, I assumed using VMID=0 just caused a loss of performance, but this makes
it sound like the IOMMU mappings will fault?

> 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).

Heh, I assumed VMID=0 is the host MMU, Intel and AMD have the same effective
behavior :-)