All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	 stable@vger.kernel.org, David Woodhouse <dwmw2@infradead.org>,
	 Stefan Teodorescu <fane@google.com>
Subject: Re: [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry
Date: Tue, 3 Nov 2026 12:59:50 -0800	[thread overview]
Message-ID: <aupLRleTJSIDTE9M@google.com> (raw)
In-Reply-To: <CALMp9eTdY3=1BrOYgov852VeAy3+Os7bvhWogvB0YF-JQrmtig@mail.gmail.com>

On Tue, Nov 03, 2026, Jim Mattson wrote:
> On Mon, Nov 2, 2026 at 4:10 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Nov 02, 2026, Jim Mattson wrote:
> > > KVM needs to do better.
> >
> > Meh, KVM is doing pretty darn good, all things considered.  Yeah, this code sucks.
> > But I'm not convinced the alternatives suck less.  We've gotten the set of missed
> > consistency checks down to effectively zero, module the TPR mess that I do test
> > by running warn_on_missed_cc=true.
> 
> I don't understand the TPR mess. The SDM says, "With the exception of
> fields corresponding to virtualized APIC registers (defined in Section
> 32.1.1), software may modify the virtual-APIC page referenced by the
> current VMCS of a logical processor in VMX non-root operation. (This
> is an exception to the general requirement given in Section 27.11.4.)"

Oh, I wasn't aware of that.  I agree, that gives us an "out" for vTPR, mostly.

> I would argue that this prohibits changes to VTPR between KVM's
> software VM-entry checks and hardware VM-entry to VMCS02. If the
> hardware VM-entry fails because VTPR has changed, section 27.11.4
> gives KVM the freedom to implement whatever "unpredictable behavior"
> we like. Yes, that probably does include loading MSR values from the
> VM-exit MSR-load list, but why complicate things?
> 
> Something like returning to userspace with
> KVM_INTERNAL_ERROR_EMULATION following a prohibited write to an active
> VTPR seems far easier--and cleaner. What am I missing?

My biggest concern is that KVM could kill L1 if an L2 coerces L1 into trying
VMLAUNCH/VMRESUME with invalid state.  KVM simply can't *know* that an unexpected
VM-Fail was due to the vTPR wrinkle.  Even if KVM re-checked state, it's possible
for the vTPR to be "restored" to a good value.

I.e. if KVM exits to userspace with KVM_INTERNAL_ERROR_EMULATION, then there's a
non-zero chance of a false positive.  KVM could exit to userspace if and only if
it caught the guest red-handed, but then we'd still need this horrific code for
the other cases, so that seems rather pointless.

warn_on_missed_cc helps, but in practice it can't detect cases for which we don't
have test coverage.  So *if* there was a bug that could be exploited by L2 to DoS
L1, odds are good warn_on_missed_cc didn't catch that hypothetical bug either.

Maybe with AI we can be 100% confident that KVM performs all of the VM-Fail
consistency checks?  Again, I'm not opposed to alternative approaches, but at the
same time I don't feel a sense of urgency.

  reply	other threads:[~2026-11-03 20:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 23:14 [PATCH 0/7] KVM: a bunch of fixes Paolo Bonzini
2026-11-02 23:14 ` [PATCH 1/7] KVM: nSVM: Check validity of vmcs01 save area copied from SMRAM on RSM Paolo Bonzini
2026-11-02 23:45   ` sashiko-bot
2026-11-02 23:14 ` [PATCH 2/7] KVM: x86/mmu: Conditionally resched when retrying gPTE walk on A/D update failure Paolo Bonzini
2026-11-02 23:14 ` [PATCH 3/7] KVM: x86: Conditionally resched when restarting REP string emulation Paolo Bonzini
2026-11-02 23:14 ` [PATCH 4/7] KVM: nVMX: Process each host MSR entry at most once when unwinding failed VM-Entry Paolo Bonzini
2026-11-02 23:39   ` Jim Mattson
2026-11-03  0:10     ` Sean Christopherson
2026-11-03  0:34       ` Jim Mattson
2026-11-03 19:40       ` Jim Mattson
2026-11-03 20:59         ` Sean Christopherson [this message]
2026-11-03 22:52           ` Jim Mattson
2026-11-04  0:12             ` Sean Christopherson
2026-11-02 23:14 ` [PATCH 5/7] KVM: x86/mmu: Cap number of parent PTEs per SP at 1000 (the arbitrary rmap limit) Paolo Bonzini
2026-11-02 23:14 ` [PATCH 6/7] KVM: x86: Unconditionally reset MMU context when emulating INIT Paolo Bonzini
2026-11-02 23:14 ` [PATCH 7/7] KVM: x86: Flush guest TLB when emulating a RESET to strictly honor the spec Paolo Bonzini

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=aupLRleTJSIDTE9M@google.com \
    --to=seanjc@google.com \
    --cc=dwmw2@infradead.org \
    --cc=fane@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    /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.