public inbox for linux-kernel@vger.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>,
	Jonathan Corbet <corbet@lwn.net>,
	 Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Maxim Levitsky <mlevitsk@redhat.com>,
	kvm@vger.kernel.org,  linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: VMX: Add quirk to allow L1 to set FREEZE_IN_SMM in vmcs12
Date: Tue, 3 Feb 2026 18:00:07 -0800	[thread overview]
Message-ID: <aYKoJ74MWboBuE_M@google.com> (raw)
In-Reply-To: <CALMp9eQx7EVim4iYGbAhoHrei2YmTra6oxtdmKaY7bw-M0PHbw@mail.gmail.com>

On Thu, Jan 22, 2026, Jim Mattson wrote:
> On Tue, Jan 13, 2026 at 7:47 PM Jim Mattson <jmattson@google.com> wrote:
> > On Tue, Jan 13, 2026 at 4:42 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Tue, Jan 13, 2026, Jim Mattson wrote:
> > > > Add KVM_X86_QUIRK_VMCS12_FREEZE_IN_SMM to allow L1 to set
> > > > IA32_DEBUGCTL.FREEZE_IN_SMM in vmcs12 when using nested VMX.  Prior to
> > > > commit 6b1dd26544d0 ("KVM: VMX: Preserve host's
> > > > DEBUGCTLMSR_FREEZE_IN_SMM while running the guest"), L1 could set
> > > > FREEZE_IN_SMM in vmcs12 to freeze PMCs during physical SMM coincident
> > > > with L2's execution.  The quirk is enabled by default for backwards
> > > > compatibility; userspace can disable it via KVM_CAP_DISABLE_QUIRKS2 if
> > > > consistency with WRMSR(IA32_DEBUGCTL) is desired.
> > >
> > > It's probably worth calling out that KVM will still drop FREEZE_IN_SMM in vmcs02
> > >
> > >         if (vmx->nested.nested_run_pending &&
> > >             (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS)) {
> > >                 kvm_set_dr(vcpu, 7, vmcs12->guest_dr7);
> > >                 vmx_guest_debugctl_write(vcpu, vmcs12->guest_ia32_debugctl &
> > >                                                vmx_get_supported_debugctl(vcpu, false)); <====
> > >         } else {
> > >                 kvm_set_dr(vcpu, 7, vcpu->arch.dr7);
> > >                 vmx_guest_debugctl_write(vcpu, vmx->nested.pre_vmenter_debugctl);
> > >         }
> > >
> > > both from a correctness standpoint and so that users aren't mislead into thinking
> > > the quirk lets L1 control of FREEZE_IN_SMM while running L2.
> >
> > Yes, it's probably worth pointing out that the VM is now subject to
> > the whims of the L0 administrators.
> >
> > While that makes some sense for the legacy vPMU, where KVM is just
> > another client of host perf, perhaps the decision should be revisited
> > in the case of the MPT vPMU, where KVM owns the PMU while the vCPU is
> > in VMX non-root operation.

Eh, running guests with FREEZE_IN_SMM=0 seems absolutely crazy from a security
perspective.  If an admin wants to disable FREEZE_IN_SMM, they get to keep the
pieces.  And KVM definitely isn't going to override the admin, e.g. to allow the
guest to profile host SMM.

> > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > > > index 0521b55d47a5..bc8f0b3aa70b 100644
> > > > --- a/arch/x86/kvm/vmx/nested.c
> > > > +++ b/arch/x86/kvm/vmx/nested.c
> > > > @@ -3298,10 +3298,24 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
> > > >       if (CC(vmcs12->guest_cr4 & X86_CR4_CET && !(vmcs12->guest_cr0 & X86_CR0_WP)))
> > > >               return -EINVAL;
> > > >
> > > > -     if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS) &&
> > > > -         (CC(!kvm_dr7_valid(vmcs12->guest_dr7)) ||
> > > > -          CC(!vmx_is_valid_debugctl(vcpu, vmcs12->guest_ia32_debugctl, false))))
> > > > -             return -EINVAL;
> > > > +     if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS) {
> > > > +             u64 debugctl = vmcs12->guest_ia32_debugctl;
> > > > +
> > > > +             /*
> > > > +              * FREEZE_IN_SMM is not virtualized, but allow L1 to set it in
> > > > +              * L2's DEBUGCTL under a quirk for backwards compatibility.
> > > > +              * Prior to KVM taking ownership of the bit to ensure PMCs are
> > > > +              * frozen during physical SMM, L1 could set FREEZE_IN_SMM in
> > > > +              * vmcs12 to freeze PMCs during physical SMM coincident with
> > > > +              * L2's execution.
> > > > +              */
> > > > +             if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_VMCS12_FREEZE_IN_SMM))
> > > > +                     debugctl &= ~DEBUGCTLMSR_FREEZE_IN_SMM;
> > > > +
> > > > +             if (CC(!kvm_dr7_valid(vmcs12->guest_dr7)) ||
> > > > +                 CC(!vmx_is_valid_debugctl(vcpu, debugctl, false)))
> > >
> > > I'm mildly tempted to say we should quirk the entire consistency check instead of
> > > limiting it to FREEZE_IN_SMM, purely so that we don't have to add yet another quirk
> > > if a different setup breaks on a different bit.  I suppose we could limit the quirk
> > > to bits that could have been plausibly set in hardware, because otherwise VM-Entry
> > > using L2 would VM-Fail, but that's still quite a few bits.
> > >
> > > I'm definitely not opposed to a targeted quirk though.
> >
> > I have no preference.

After mulling over the options from time to time, I think our best be is to quirk
only FREEZE_IN_SMM, but very explicity scope the quirk to just the consistency
check.  E.g. maybe KVM_X86_QUIRK_VMCS12_FREEZE_IN_SMM_CC?  That should help alert
readers to the fact that the quirk bypasses the check, but L2 will still see
FREEZE_IN_SMM=0 (e.g. in the unlikely scenario L1 disables interception of
DEBUGCTL).

As for why just FREEZE_IN_SMM, in addition to the fact that FREEZE_IN_SMM is the
only bit that broke anyone (as far as we know, /knock wood), it's also the only
bit that is host-owned.  I.e. unless the host admin likes SMM mucking with things,
skipping the consistency check isn't terrible from a functionality perspective
(KVM doesn't honor the bit for emulated SMM, but that's QEMU's problem :-D).

> Would you like me to post a v2?

Yes please.

  reply	other threads:[~2026-02-04  2:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-13 22:53 [PATCH] KVM: VMX: Add quirk to allow L1 to set FREEZE_IN_SMM in vmcs12 Jim Mattson
2026-01-14  0:42 ` Sean Christopherson
2026-01-14  3:47   ` Jim Mattson
2026-01-22 21:26     ` Jim Mattson
2026-02-04  2:00       ` Sean Christopherson [this message]
2026-02-05  0:42         ` Jim Mattson
2026-02-05  1:18           ` Sean Christopherson
2026-02-05  4:11             ` Jim Mattson
2026-02-05 14:47               ` Sean Christopherson
2026-02-05 17:43                 ` Jim Mattson
2026-02-05 18:16                   ` Sean Christopherson

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=aYKoJ74MWboBuE_M@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox