public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry.ahmed@linux.dev>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	 Maxim Levitsky <mlevitsk@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 stable@vger.kernel.org
Subject: Re: [PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv()
Date: Tue, 11 Nov 2025 14:30:58 -0800	[thread overview]
Message-ID: <aRO5ItX_--ZDfnfM@google.com> (raw)
In-Reply-To: <6ving6sg3ywr23epd3fmorzhovdom5uaty4ae4itit2amxafql@iui7as55sb55>

On Tue, Nov 11, 2025, Yosry Ahmed wrote:
> On Tue, Nov 11, 2025 at 03:11:37AM +0000, Yosry Ahmed wrote:
> > On Sat, Nov 08, 2025 at 12:45:20AM +0000, Yosry Ahmed wrote:
> > > svm_update_lbrv() is called when MSR_IA32_DEBUGCTLMSR is updated, and on
> > > nested transitions where LBRV is used. It checks whether LBRV enablement
> > > needs to be changed in the current VMCB, and if it does, it also
> > > recalculate intercepts to LBR MSRs.
> > > 
> > > However, there are cases where intercepts need to be updated even when
> > > LBRV enablement doesn't. Example scenario:
> > > - L1 has MSR_IA32_DEBUGCTLMSR cleared.
> > > - L1 runs L2 without LBR_CTL_ENABLE (no LBRV).
> > > - L2 sets DEBUGCTLMSR_LBR in MSR_IA32_DEBUGCTLMSR, svm_update_lbrv()
> > >   sets LBR_CTL_ENABLE in VMCB02 and disables intercepts to LBR MSRs.
> > > - L2 exits to L1, svm_update_lbrv() is not called on this transition.
> > > - L1 clears MSR_IA32_DEBUGCTLMSR, svm_update_lbrv() finds that
> > >   LBR_CTL_ENABLE is already cleared in VMCB01 and does nothing.
> > > - Intercepts remain disabled, L1 reads to LBR MSRs read the host MSRs.
> > > 
> > > Fix it by always recalculating intercepts in svm_update_lbrv().
> > 
> > This actually breaks hyperv_svm_test, because svm_update_lbrv() is
> > called on every nested transition, calling
> > svm_recalc_lbr_msr_intercepts() -> svm_set_intercept_for_msr() and
> > setting svm->nested.force_msr_bitmap_recalc to true.
> > 
> > This breaks the hyperv optimization in nested_svm_vmrun_msrpm() AFAICT.
> > 
> > I think there are two ways to fix this:
> > - Add another bool to svm->nested to track LBR intercepts, and only call
> >   svm_set_intercept_for_msr() if the intercepts need to be updated.
> > 
> > - Update svm_set_intercept_for_msr() itself to do nothing if the
> >   intercepts do not need to be changed, which is more clutter but
> >   applies to other callers as well so could shave cycles elsewhere (see
> >   below).
> > 
> > Sean, Paolo, any preferences?
> > 
> > Here's what updating svm_set_intercept_for_msr() looks like:

I am *very* strongly opposed to modifying svm_set_intercept_for_msr() to deal
with whatever mess LBRs has created.  Whatever the problem is (I haven't read
through all of this yet), it needs to be fixed in the LBR code, not in
svm_set_intercept_for_msr().

Recalculating MSR intercepts is supposed to done only as needed, I don't want to
encourage lazy code that works by optimizing paths that should be rare.

  reply	other threads:[~2025-11-11 22:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251108004524.1600006-1-yosry.ahmed@linux.dev>
2025-11-08  0:45 ` [PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv() Yosry Ahmed
2025-11-11  3:11   ` Yosry Ahmed
2025-11-11 18:55     ` Yosry Ahmed
2025-11-11 22:30       ` Sean Christopherson [this message]
2025-11-08  0:45 ` [PATCH 3/6] KVM: nSVM: Fix and simplify LBR virtualization handling with nested Yosry Ahmed
2025-11-08  0:45 ` [PATCH 4/6] KVM: SVM: Switch svm_copy_lbrs() to a macro Yosry Ahmed
2025-11-09  7:59   ` Paolo Bonzini
2025-11-10 19:23     ` Yosry Ahmed
2025-11-10 19:41       ` Sean Christopherson
2025-11-08  0:45 ` [PATCH 5/6] KVM: SVM: Add missing save/restore handling of LBR MSRs Yosry Ahmed
2025-11-08  9:08   ` Yosry Ahmed

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=aRO5ItX_--ZDfnfM@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=yosry.ahmed@linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox