[PATCH v4 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode

Jim Mattson posted 8 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
Posted by Jim Mattson 1 month, 2 weeks ago
When running an L2 guest and writing to MSR_IA32_CR_PAT, the host PAT value
is stored in both vmcb01's g_pat field and vmcb02's g_pat field, but the
clean bit was only being cleared for vmcb02.

Introduce the helper vmcb_set_gpat() which sets vmcb->save.g_pat and marks
the VMCB dirty for VMCB_NPT. Use this helper in both svm_set_msr() for
updating vmcb01 and in nested_vmcb02_compute_g_pat() for updating vmcb02,
ensuring both VMCBs' NPT fields are properly marked dirty.

Fixes: 4995a3685f1b ("KVM: SVM: Use a separate vmcb for the nested L2 guest")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 2 +-
 arch/x86/kvm/svm/svm.c    | 3 +--
 arch/x86/kvm/svm/svm.h    | 9 +++++----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index d80b1bde6630..b72a1f3c4144 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -707,7 +707,7 @@ void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
 		return;
 
 	/* FIXME: merge g_pat from vmcb01 and vmcb12.  */
-	svm->nested.vmcb02.ptr->save.g_pat = svm->vmcb01.ptr->save.g_pat;
+	vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
 }
 
 static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 364915f42e13..529cbac57814 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2924,10 +2924,9 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		if (ret)
 			break;
 
-		svm->vmcb01.ptr->save.g_pat = data;
+		vmcb_set_gpat(svm->vmcb01.ptr, data);
 		if (is_guest_mode(vcpu))
 			nested_vmcb02_compute_g_pat(svm);
-		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
 		break;
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr->host_initiated &&
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 0bb93879abfe..9850ed01e16e 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -434,14 +434,15 @@ static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
 	vmcb->control.clean &= ~(1 << bit);
 }
 
-static inline bool vmcb_is_dirty(struct vmcb *vmcb, int bit)
+static inline bool vmcb12_is_dirty(struct vmcb_ctrl_area_cached *control, int bit)
 {
-        return !test_bit(bit, (unsigned long *)&vmcb->control.clean);
+	return !test_bit(bit, (unsigned long *)&control->clean);
 }
 
-static inline bool vmcb12_is_dirty(struct vmcb_ctrl_area_cached *control, int bit)
+static inline void vmcb_set_gpat(struct vmcb *vmcb, u64 data)
 {
-	return !test_bit(bit, (unsigned long *)&control->clean);
+	vmcb->save.g_pat = data;
+	vmcb_mark_dirty(vmcb, VMCB_NPT);
 }
 
 static __always_inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
-- 
2.53.0.239.g8d8fc8a987-goog
Re: [PATCH v4 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
Posted by Yosry Ahmed 1 month, 2 weeks ago
On Thu, Feb 12, 2026 at 07:58:49AM -0800, Jim Mattson wrote:
> When running an L2 guest and writing to MSR_IA32_CR_PAT, the host PAT value
> is stored in both vmcb01's g_pat field and vmcb02's g_pat field, but the
> clean bit was only being cleared for vmcb02.
> 
> Introduce the helper vmcb_set_gpat() which sets vmcb->save.g_pat and marks
> the VMCB dirty for VMCB_NPT. Use this helper in both svm_set_msr() for
> updating vmcb01 and in nested_vmcb02_compute_g_pat() for updating vmcb02,
> ensuring both VMCBs' NPT fields are properly marked dirty.
> 
> Fixes: 4995a3685f1b ("KVM: SVM: Use a separate vmcb for the nested L2 guest")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 2 +-
>  arch/x86/kvm/svm/svm.c    | 3 +--
>  arch/x86/kvm/svm/svm.h    | 9 +++++----
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index d80b1bde6630..b72a1f3c4144 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -707,7 +707,7 @@ void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
>  		return;
>  
>  	/* FIXME: merge g_pat from vmcb01 and vmcb12.  */
> -	svm->nested.vmcb02.ptr->save.g_pat = svm->vmcb01.ptr->save.g_pat;
> +	vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
>  }
>  
>  static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 364915f42e13..529cbac57814 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2924,10 +2924,9 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  		if (ret)
>  			break;
>  
> -		svm->vmcb01.ptr->save.g_pat = data;
> +		vmcb_set_gpat(svm->vmcb01.ptr, data);
>  		if (is_guest_mode(vcpu))
>  			nested_vmcb02_compute_g_pat(svm);
> -		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
>  		break;
>  	case MSR_IA32_SPEC_CTRL:
>  		if (!msr->host_initiated &&
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 0bb93879abfe..9850ed01e16e 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -434,14 +434,15 @@ static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
>  	vmcb->control.clean &= ~(1 << bit);
>  }
>  
> -static inline bool vmcb_is_dirty(struct vmcb *vmcb, int bit)

Huh, I assume the removed of vmcb_is_dirty() was not intentional?

> +static inline bool vmcb12_is_dirty(struct vmcb_ctrl_area_cached *control, int bit)
>  {
> -        return !test_bit(bit, (unsigned long *)&vmcb->control.clean);
> +	return !test_bit(bit, (unsigned long *)&control->clean);
>  }
>  
> -static inline bool vmcb12_is_dirty(struct vmcb_ctrl_area_cached *control, int bit)
> +static inline void vmcb_set_gpat(struct vmcb *vmcb, u64 data)
>  {
> -	return !test_bit(bit, (unsigned long *)&control->clean);
> +	vmcb->save.g_pat = data;
> +	vmcb_mark_dirty(vmcb, VMCB_NPT);
>  }
>  
>  static __always_inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
> -- 
> 2.53.0.239.g8d8fc8a987-goog
>
Re: [PATCH v4 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
Posted by Sean Christopherson 1 month, 2 weeks ago
On Fri, Feb 13, 2026, Yosry Ahmed wrote:
> On Thu, Feb 12, 2026 at 07:58:49AM -0800, Jim Mattson wrote:
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 0bb93879abfe..9850ed01e16e 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -434,14 +434,15 @@ static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
> >  	vmcb->control.clean &= ~(1 << bit);
> >  }
> >  
> > -static inline bool vmcb_is_dirty(struct vmcb *vmcb, int bit)
> 
> Huh, I assume the removed of vmcb_is_dirty() was not intentional?

Regardless of whether or not it was intentional, IMO it's a good change.  KVM
should never check vmcb12 directly, and I can't think of a legitimate case where
KVM should condition its behavior on vmcb0{1,2} being clean/dirty.

Unless a v5 is needed, I'll split it to a separate patch when applying.
Re: [PATCH v4 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
Posted by Yosry Ahmed 1 month, 2 weeks ago
On Fri, Feb 13, 2026 at 07:26:34AM -0800, Sean Christopherson wrote:
> On Fri, Feb 13, 2026, Yosry Ahmed wrote:
> > On Thu, Feb 12, 2026 at 07:58:49AM -0800, Jim Mattson wrote:
> > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > > index 0bb93879abfe..9850ed01e16e 100644
> > > --- a/arch/x86/kvm/svm/svm.h
> > > +++ b/arch/x86/kvm/svm/svm.h
> > > @@ -434,14 +434,15 @@ static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
> > >  	vmcb->control.clean &= ~(1 << bit);
> > >  }
> > >  
> > > -static inline bool vmcb_is_dirty(struct vmcb *vmcb, int bit)
> > 
> > Huh, I assume the removed of vmcb_is_dirty() was not intentional?
> 
> Regardless of whether or not it was intentional, IMO it's a good change.  KVM
> should never check vmcb12 directly, and I can't think of a legitimate case where
> KVM should condition its behavior on vmcb0{1,2} being clean/dirty.

Funny enough, I removed all usages of vmcb_is_dirty() in my series, I
just didn't drop it:

https://lore.kernel.org/kvm/20260206190851.860662-24-yosry.ahmed@linux.dev/

So Jim was cleaning up after me :)

> 
> Unless a v5 is needed, I'll split it to a separate patch when applying.
Re: [PATCH v4 1/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
Posted by Jim Mattson 1 month, 2 weeks ago
On Fri, Feb 13, 2026 at 7:33 AM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
> Funny enough, I removed all usages of vmcb_is_dirty() in my series, I
> just didn't drop it:
>
> https://lore.kernel.org/kvm/20260206190851.860662-24-yosry.ahmed@linux.dev/
>
> So Jim was cleaning up after me :)

Sorry; I removed it when handling the merge conflicts, thought about
moving it to a separate patch, and then forgot about it.