All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Oliver Upton <oliver.upton@linux.dev>
Cc: Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.linux.dev, Joey Gouly <joey.gouly@arm.com>,
	 Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>
Subject: Re: [PATCH 3/3] KVM: arm64: nv: Punt stage-2 recycling to a vCPU request
Date: Thu, 3 Oct 2024 11:23:36 -0700	[thread overview]
Message-ID: <Zv7hKD_6Pvhg4ULY@google.com> (raw)
In-Reply-To: <Zv7Z4D0L3bnxJi8h@linux.dev>

On Thu, Oct 03, 2024, Oliver Upton wrote:
> On Thu, Oct 03, 2024 at 09:45:40AM -0700, Sean Christopherson wrote:
> 
> [...[
> 
> > > > 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.
> 
> Not really, the unmap plumbing is used for applying the intent of a
> guest TLBI too. Sub-optimal or not, it is exactly what the VM asked for,
> and it'd be in our interest to handle the unmap as expeditiously as
> possible.

Sorry, I meant "unnecessary unmap".

> > > > 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?
> 
> How could we possibly know what the intent of userspace is? The VMM
> could just as well throw that vCPU fd on ice for an eternity.
> 
> For example, you could have a PSCI implementation that lives in
> userspace. Guest does CPU_OFF and the VMM decides to terminate the
> backing thread and keep the FD around for the next CPU_ON.

Yes, but we need to play the odds.  I.e. make the common case fast/efficient.
KVM obviously needs to not fallover or crater performance in the presence of edge
cases, but IMO, disallowing a vCPU from pinning a vCPU because it _might_ go
offline is the wrong tradeoff.

> Since KVM still views that fd as 'runnable', it'd sit on the reference
> that vCPU holds indefinitely. On top of that, it adds complexity to the
> implementation since we would need more refcount cleanup flows to handle
> these straggler references.

But only one flow, vCPU destruction, is mandatory.  Anything beyond that is pure
optimization.

> > 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.
> 
> Making TLBs private to the L1 vCPU is almost guaranteed to be a net loss
> in performance.

I'm not saying make TLBs private, I'm saying allow each vCPU to "pin" (i.e. hold
a reference) up to N TLBs/MMUs, regardless of "where" that vCPU is in the flow
of things.  Versus the proposed behavior of pinning TLBs only when it's absolutely
mandatory to do so for functional correctness.

Holding a reference across preemption would be the first step towards that model.

  reply	other threads:[~2024-10-03 18:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-01  0:17 [PATCH 0/3] KVM: arm64: nv: Fixes for stage-2 MMU recycling Oliver Upton
2024-10-01  0:17 ` [PATCH 1/3] KVM: arm64: Treat stage-2 MMUs as refcounted generally Oliver Upton
2024-10-01  0:17 ` [PATCH 2/3] KVM: arm64: nv: Do not block when unmapping stage-2 if disallowed Oliver Upton
2024-10-01  0:17 ` [PATCH 3/3] KVM: arm64: nv: Punt stage-2 recycling to a vCPU request Oliver Upton
2024-10-01 19:05   ` Sean Christopherson
2024-10-01 20:41     ` Oliver Upton
2024-10-01 23:28       ` Sean Christopherson
2024-10-01 23:49         ` Marc Zyngier
2024-10-02  0:06           ` Oliver Upton
2024-10-02  0:23             ` Sean Christopherson
2024-10-02 23:31               ` Marc Zyngier
2024-10-03  0:04                 ` Oliver Upton
2024-10-03  0:12                   ` Oliver Upton
2024-10-03 16:45                     ` Sean Christopherson
2024-10-03 17:52                       ` Oliver Upton
2024-10-03 18:23                         ` Sean Christopherson [this message]
2024-10-03 22:03                           ` Oliver Upton
2024-10-01 23:23   ` Marc Zyngier
2024-10-02  0:06     ` Oliver Upton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zv7hKD_6Pvhg4ULY@google.com \
    --to=seanjc@google.com \
    --cc=joey.gouly@arm.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=suzuki.poulose@arm.com \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.