1. Patchew
  2. linux
  3. [PATCH v2 0/8] KVM: x86: nSVM: Improve PAT virtualization
  4. View patch

[PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat

Jim Mattson posted 8 patches 3 weeks, 2 days ago
There is a newer version of this series
[PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat
Posted by Jim Mattson 3 weeks, 2 days ago 
Add a validity check for g_pat, so that when nested paging is enabled for
vmcb12, an invalid g_pat causes an immediate VMEXIT with exit code
VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
VMRUN/VMEXIT."

Update the signature of __nested_vmcb_check_save() to include a pointer to
a struct vmcb_ctrl_area_cached, since the g_pat validity check depend on
the nested paging control bit.

Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 07a57a43fc3b..e65291434be9 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -369,7 +369,8 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 
 /* Common checks that apply to both L1 and L2 state.  */
 static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
-				     struct vmcb_save_area_cached *save)
+				     struct vmcb_save_area_cached *save,
+				     struct vmcb_ctrl_area_cached *control)
 {
 	if (CC(!(save->efer & EFER_SVME)))
 		return false;
@@ -400,6 +401,10 @@ static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
 	if (CC(!kvm_valid_efer(vcpu, save->efer)))
 		return false;
 
+	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
+	       npt_enabled && !kvm_pat_valid(save->g_pat)))
+		return false;
+
 	return true;
 }
 
@@ -407,8 +412,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	struct vmcb_save_area_cached *save = &svm->nested.save;
+	struct vmcb_ctrl_area_cached *ctl = &svm->nested.ctl;
 
-	return __nested_vmcb_check_save(vcpu, save);
+	return __nested_vmcb_check_save(vcpu, save, ctl);
 }
 
 static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
@@ -1892,7 +1898,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	if (!(save->cr0 & X86_CR0_PG) ||
 	    !(save->cr0 & X86_CR0_PE) ||
 	    (save->rflags & X86_EFLAGS_VM) ||
-	    !__nested_vmcb_check_save(vcpu, &save_cached))
+	    !__nested_vmcb_check_save(vcpu, &save_cached, &ctl_cached))
 		goto out_free;
 
 
-- 
2.52.0.457.g6b5491de43-goog
Re: [PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat
Posted by Yosry Ahmed 2 weeks, 3 days ago 
On Thu, Jan 15, 2026 at 03:21:42PM -0800, Jim Mattson wrote:
> Add a validity check for g_pat, so that when nested paging is enabled for
> vmcb12, an invalid g_pat causes an immediate VMEXIT with exit code
> VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
> VMRUN/VMEXIT."
> 
> Update the signature of __nested_vmcb_check_save() to include a pointer to
> a struct vmcb_ctrl_area_cached, since the g_pat validity check depend on
> the nested paging control bit.
> 
> Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 07a57a43fc3b..e65291434be9 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -369,7 +369,8 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
>  
>  /* Common checks that apply to both L1 and L2 state.  */
>  static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> -				     struct vmcb_save_area_cached *save)
> +				     struct vmcb_save_area_cached *save,
> +				     struct vmcb_ctrl_area_cached *control)
>  {
>  	if (CC(!(save->efer & EFER_SVME)))
>  		return false;
> @@ -400,6 +401,10 @@ static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
>  	if (CC(!kvm_valid_efer(vcpu, save->efer)))
>  		return false;
>  
> +	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
> +	       npt_enabled && !kvm_pat_valid(save->g_pat)))

If this lands after "KVM: nSVM: Drop the non-architectural consistency
check for NP_ENABLE" [1], the npt_enabled check can be dropped, as
SVM_NESTED_CTL_NP_ENABLE will be cleared if the guest cannot use NPTs.

[1]https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/

> +		return false;
> +
>  	return true;
>  }
>  
> @@ -407,8 +412,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	struct vmcb_save_area_cached *save = &svm->nested.save;
> +	struct vmcb_ctrl_area_cached *ctl = &svm->nested.ctl;
>  
> -	return __nested_vmcb_check_save(vcpu, save);
> +	return __nested_vmcb_check_save(vcpu, save, ctl);
>  }
>  
>  static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
> @@ -1892,7 +1898,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  	if (!(save->cr0 & X86_CR0_PG) ||
>  	    !(save->cr0 & X86_CR0_PE) ||
>  	    (save->rflags & X86_EFLAGS_VM) ||
> -	    !__nested_vmcb_check_save(vcpu, &save_cached))
> +	    !__nested_vmcb_check_save(vcpu, &save_cached, &ctl_cached))

save_cached here is from vmcb01, but ctl_cached is from vmcb12, right?
So we're conditioning the PAT validity check on L1's gPAT on NPT being
enabled for L2 IIUC.

Perhaps we should pass 'bool guest_npt' instead of the cached control
area?  Then we can just pass npt_enabled here IIUC.

>  		goto out_free;
>  
>  
> -- 
> 2.52.0.457.g6b5491de43-goog
>
Re: [PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat
Posted by Jim Mattson 5 days, 16 hours ago 
On Wed, Jan 21, 2026 at 5:41 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Thu, Jan 15, 2026 at 03:21:42PM -0800, Jim Mattson wrote:
> > Add a validity check for g_pat, so that when nested paging is enabled for
> > vmcb12, an invalid g_pat causes an immediate VMEXIT with exit code
> > VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
> > VMRUN/VMEXIT."
> >
> > Update the signature of __nested_vmcb_check_save() to include a pointer to
> > a struct vmcb_ctrl_area_cached, since the g_pat validity check depend on
> > the nested paging control bit.
> >
> > Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/kvm/svm/nested.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 07a57a43fc3b..e65291434be9 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -369,7 +369,8 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
> >
> >  /* Common checks that apply to both L1 and L2 state.  */
> >  static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> > -                                  struct vmcb_save_area_cached *save)
> > +                                  struct vmcb_save_area_cached *save,
> > +                                  struct vmcb_ctrl_area_cached *control)
> >  {
> >       if (CC(!(save->efer & EFER_SVME)))
> >               return false;
> > @@ -400,6 +401,10 @@ static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> >       if (CC(!kvm_valid_efer(vcpu, save->efer)))
> >               return false;
> >
> > +     if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
> > +            npt_enabled && !kvm_pat_valid(save->g_pat)))
>
> If this lands after "KVM: nSVM: Drop the non-architectural consistency
> check for NP_ENABLE" [1], the npt_enabled check can be dropped, as
> SVM_NESTED_CTL_NP_ENABLE will be cleared if the guest cannot use NPTs.
>
> [1]https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/

I don't see it today, but I will probably forget to keep checking. :)

> > +             return false;
> > +
> >       return true;
> >  }
> >
> > @@ -407,8 +412,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu)
> >  {
> >       struct vcpu_svm *svm = to_svm(vcpu);
> >       struct vmcb_save_area_cached *save = &svm->nested.save;
> > +     struct vmcb_ctrl_area_cached *ctl = &svm->nested.ctl;
> >
> > -     return __nested_vmcb_check_save(vcpu, save);
> > +     return __nested_vmcb_check_save(vcpu, save, ctl);
> >  }
> >
> >  static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
> > @@ -1892,7 +1898,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >       if (!(save->cr0 & X86_CR0_PG) ||
> >           !(save->cr0 & X86_CR0_PE) ||
> >           (save->rflags & X86_EFLAGS_VM) ||
> > -         !__nested_vmcb_check_save(vcpu, &save_cached))
> > +         !__nested_vmcb_check_save(vcpu, &save_cached, &ctl_cached))
>
> save_cached here is from vmcb01, but ctl_cached is from vmcb12, right?
> So we're conditioning the PAT validity check on L1's gPAT on NPT being
> enabled for L2 IIUC.
>
> Perhaps we should pass 'bool guest_npt' instead of the cached control
> area?  Then we can just pass npt_enabled here IIUC.

I'm going to cache it on its own to avoid confusion.

> >               goto out_free;
> >
> >
> > --
> > 2.52.0.457.g6b5491de43-goog
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.