[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0468715595718af34a8a3551663cffa79dd3ce2e@linux.dev>
Date: Fri, 06 Feb 2026 18:29:05 +0000
From: "Yosry Ahmed" <yosry.ahmed@...ux.dev>
To: "Sean Christopherson" <seanjc@...gle.com>, "Jim Mattson"
<jmattson@...gle.com>
Cc: "Paolo Bonzini" <pbonzini@...hat.com>, "Thomas Gleixner"
<tglx@...nel.org>, "Ingo Molnar" <mingo@...hat.com>, "Borislav Petkov"
<bp@...en8.de>, "Dave Hansen" <dave.hansen@...ux.intel.com>,
x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, "Shuah Khan"
<shuah@...nel.org>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for
nested NPT
February 6, 2026 at 10:23 AM, "Sean Christopherson" <seanjc@...gle.com> wrote:
>
> On Thu, Feb 05, 2026, Jim Mattson wrote:
>
> >
> > When nested NPT is enabled in vmcb12, copy the (cached and validated)
> > vmcb12 g_pat field to the guest PAT register. Under KVM, the guest PAT
> > register lives in the vmcb02 g_pat field.
> >
> > When NPT is enabled, but nested NPT is disabled, copy L1's IA32_PAT MSR to
> > the vmcb02 g_pat field, since L2 shares the IA32_PAT MSR with L1.
> >
> > When NPT is disabled, the vmcb02 g_pat field is ignored by hardware.
> >
> Uber nit, the "vmcb02" qualifier can be dropped, i.e.
>
> When NPT is disabled, the g_pat field is ignored by hardware.
>
> Scoping it to vmcb02 makes it sound like there's a special rule about vmcb02.
>
> >
> > Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
> > Signed-off-by: Jim Mattson <jmattson@...gle.com>
> > ---
> > arch/x86/kvm/svm/nested.c | 16 +++++++++++++---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 1d4ff6408b34..1ff2ede96094 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -646,9 +646,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> > struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
> > struct kvm_vcpu *vcpu = &svm->vcpu;
> >
> > - nested_vmcb02_compute_g_pat(svm);
> > - vmcb_mark_dirty(vmcb02, VMCB_NPT);
> > -
> > /* Load the nested guest state */
> > if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
> > new_vmcb12 = true;
> > @@ -656,6 +653,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> > svm->nested.force_msr_bitmap_recalc = true;
> > }
> >
> > + if (npt_enabled) {
> > + if (nested_npt_enabled(svm)) {
> > + if (unlikely(new_vmcb12 ||
> > + vmcb_is_dirty(vmcb12, VMCB_NPT))) {
> > + vmcb02->save.g_pat = svm->nested.gpat;
> > + vmcb_mark_dirty(vmcb02, VMCB_NPT);
> > + }
> > + } else {
> > + vmcb02->save.g_pat = vcpu->arch.pat;
> > + vmcb_mark_dirty(vmcb02, VMCB_NPT);
> > + }
> > + }
> >
> To reduce indentation, how about this? There's a consistency check for
> nested_npt_enabled() vs. npt_enabled, so it's guaranteed to do the right thing.
You mean the one that goes away after this patch: https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/?
>
> if (nested_npt_enabled(svm)) {
> if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_NPT))) {
> vmcb02->save.g_pat = svm->nested.gpat;
> vmcb_mark_dirty(vmcb02, VMCB_NPT);
> }
> } else if (npt_enabled) {
> vmcb02->save.g_pat = vcpu->arch.pat;
> vmcb_mark_dirty(vmcb02, VMCB_NPT);
> }
>
> >
> > +
> > if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_SEG))) {
> > vmcb02->save.es = vmcb12->save.es;
> > vmcb02->save.cs = vmcb12->save.cs;
> > --
> > 2.53.0.rc2.204.g2597b5adb4-goog
> >
>
Powered by blists - more mailing lists