Re: [PATCH 3/3] KVM: arm64: nv: Punt stage-2 recycling to a vCPU request

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 03, 2024, Oliver Upton wrote:
> On Thu, Oct 03, 2024 at 12:04:06AM +0000, Oliver Upton wrote:
> > Hey,
> > 
> > On Thu, Oct 03, 2024 at 12:31:33AM +0100, Marc Zyngier wrote:
> > > On Wed, 02 Oct 2024 01:23:41 +0100, Sean Christopherson <seanjc@google.com> wrote:
> > > > IIUC, KVM round-robins across 2*nr_vcpus MMUs, and when L1 switches to a different
> > > > VTTBR, it will first drop its reference to the previous MMU.  So at any given time,
> > > > there are nr_vcpus worth of unused MMUs, i.e. a vCPU is guaranteed to be able to
> > > > find an unused slot, even if vCPUs that are scheduled out hold onto their S2 MMU
> > > > reference.
> > > 
> > > It's not about not finding a slot, but about making sure that vcpus
> > > that context switch rapidly between VTTBRs for their own guests can do
> > > so freely without sacrificing the TLBs they have just produced. Not
> > > reusing the TLBs hogged by a vcpu that cannot run is a waste of
> > > resource.

I don't think it's a complete waste.  There's value in not having to unmap and
rebuild an S2 MMU that is likely to be reused in the near future, especially for
a vCPU that was preempted.  E.g. the preempted L2 vCPU is already going to
experience some amount of jitter, forcing it to recycle a different S2 MMU and
then rebuild its S2 MMU is going to exacerbate the jitter.

Jitter aside, for a well-behaved system, it's unlikely a vCPU in the same VM will
be able to switch between L2s (VTTBRs) so fast that it would be a net positive to
recycle the TLB/MMU of a VTTBR that is "active", but scheduled out.  E.g. if the
L0 scheduler doesn't give the scheduled out vCPU a time slice "soon", then the L1
VM is going to be quite unhappy.

> > OTOH, our global TLBs don't model hardware exactly since a vCPU doing
> > rapid context switches trash the TLBs of *all* vCPUs in the system.
> > The cost of reusing an MMU is quite noticeable, since our unmap
> > implementation is slightly crap at the moment, the cost of which shows
> > up both on sides of the reclaim (victim and user).
> 
> Oh, and why unmap is crap:

Heh, isn't unmap by definition crap?  If KVM needs to unmap and rebuild an S2 MMU,
then KVM is already in a slow, sub-optimal situation.  I'm not saying unmap and
rebuild shouldn't be optimized as much as possible, just that weighting heavily
twoards avoiding unmap+rebuild will likely yield better overall performance and
experience, even if it means holding references across certain boundaries.

> > > > At that point, choosing an MMU that no vCPU is using seems more likely to recycle
> > > > a cold/dead MMU than a soon-to-be-reused MMU.
> > > > 
> > > > And the round-robin approach makes it all heavily luck-based anyways.  E.g. if
> > > > a vCPU puts VTTBR A and then loads VTTBR B, B could recycle A's S2 MMU if that
> > > > MMU slot is next up for recycling.
> > > 
> > > Well, we'll have to agree to disagree. It's a terrible hack to add
> > > artificial ties between a vcpu and TLBs.

But those ties already exist, in that nested_mmus_size scales with the number of
L1 vCPUs.
 
> > Still should drop the reference in most other cases, as I do *not* want
> > to entertain vCPUs holding a reference when they've gone out to
> > userspace.

Why not?  The vCPU is still running, keeping its S2 MMU resident is desirable, no?
It's extremely unlikely userspace will terminate or refuse to run the vCPU unless
the entire L1 VM is doomed.

Similarly, if an L1 vCPU is running multiple L2s, i.e. multiple VTTBRs, then it's
desirable to keep references to multiple S2 MMUs as well, e.g. so that switching
between two VTTBRs is guaranteed to get a cache hit on both.

Essentially all I'm suggesting is that instead of having a common pool of 2*vCPUs
TLBs per L1 VMM, have 2 (or however many) TLBs per L1 vCPU, plus maybe N extra
TLBs per L1 VMM.  I.e. mimic the hierarchical design of hardware caches and TLBs
to some extent.

vCPUs can still spill past their dedicated 2 TLBs, e.g. if L1 is only running L2s
in a subset of vCPUs.  Yeah, there will be wasted memory if a vCPU stops running
L2s, as KVM won't know when to drop references to its VTTBRs.  But that's a very
manageable problem (though definitely something for the future), e.g. by detecting
effective TLB pressure and requesting vCPUs to drop references to VTTBRs that
aren't actively being used, or by dropping references if the VM clobbers the
associated stage-2 PTEs that are being shadowed (i.e. if L1 is no longer using
that memory for page tables).

I'm not suggesting an overhaul to fix the preemption bug, but I do think that
effectively acquiring references on-demand is an unnecessarily complex solution.