Re: [PATCH v3 2/8] KVM: x86: nSVM: Cache and validate vmcb12 g_pat

[Sean Christopherson <seanjc@google.com>]

On Fri, Feb 06, 2026, Jim Mattson wrote:
> On Fri, Feb 6, 2026 at 11:12 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Feb 06, 2026, Jim Mattson wrote:
> > > On Fri, Feb 6, 2026 at 10:23 AM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
> > > >
> > > > February 6, 2026 at 10:19 AM, "Sean Christopherson" <seanjc@google.com> wrote:
> > AFAICT, the only "problem" is that g_pat in the serialization payload will be
> > garbage when restoring state from an older KVM.  But that's totally fine, precisely
> > because L1's PAT isn't restored from vmcb01 on nested #VMEXIT, it's always resident
> > in vcpu->arch.pat.  So can't we just do this to avoid a spurious -EINVAL?
> >
> >         /*
> >          * Validate host state saved from before VMRUN (see
> >          * nested_svm_check_permissions).
> >          */
> >         __nested_copy_vmcb_save_to_cache(&save_cached, save);
> >
> >         /*
> >          * Stuff gPAT in L1's save state, as older KVM may not have saved L1's
> >          * gPAT.  L1's PAT, i.e. hPAT for the vCPU, is *always* tracked in
> >          * vcpu->arch.pat, i.e. gPAT is a reflection of vcpu->arch.pat, not the
> >          * other way around.
> >          */
> >         save_cached.g_pat = vcpu->arch.pat;
> 
> Your comment is a bit optimistic. Qemu, for instance, hasn't restored
> MSRs yet, so vcpu->arch.pat will actually be the current vCPU's PAT
> (in the case of snapshot restore, some future PAT).

Yeah, FWIW, I was _trying_ account for that by not explicitly saying that arch.pat
is the "new" L1 state, but it's difficult to dance around :-/

> But, in any case, it should be a valid PAT.
>
> >         if (!(save->cr0 & X86_CR0_PG) ||
> >             !(save->cr0 & X86_CR0_PE) ||
> >             (save->rflags & X86_EFLAGS_VM) ||
> >             !nested_vmcb_check_save(vcpu, &ctl_cached, &save_cached))
> 
> Wrong ctl_cached. Those are the vmcb02 controls, but we are checking
> the vmcb01 save state.

*sigh*

> I think it would be better to add a boolean argument, "check_gpat,"
> which will be false at this call site and nested_npt_enabled(vcpu) at
> the other call site.

Yeah, agreed.  Because even though arch.pat should be valid, IIUC there isn't a
consistent check on hPAT because it's never reloaded.