[v3] x86/svm: Intercept CR0 writes selectively

[PATCH v3] x86/svm: Intercept CR0 writes selectively

Posted by Ross Lagerwall 1 week, 6 days ago

Since 3356d685dbda ("x86/svm: Remove lazy FPU support"), Xen does not
need to track when the TS or MP bits change so opt to intercept CR0
writes selectively. Aside from potentially reducing a few VMEXITs, this
fixes a nested virt bug where L1 intercepts CR0_SEL_WRITE and L0
intercepts CR0_WRITE. The hardware prioritizes CR0_WRITE and so L1 never
sees any CR0 writes.

Since CR0 TS/MP bits may now change behind Xen's back, sync CR0 on
VMEXIT so that the emulator sees the correct values.

Shadow mode continues to use the full CR0_WRITE intercept since with
Shadow the CR0 in the VMCB is not the same as the value Xen tracks on
behalf of the guest and allowing the guest to change one of them
directly would be fragile.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---

In v3: Change HAP only, not Shadow

 xen/arch/x86/hvm/svm/svm.c  | 7 ++++++-
 xen/arch/x86/hvm/svm/vmcb.c | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 5f5d903d872d..4e0f3282251f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1640,7 +1640,8 @@ static void svm_vmexit_do_cr_access(
 {
     int gp, cr, dir, rc;
 
-    cr = vmcb->exitcode - VMEXIT_CR0_READ;
+    cr = (vmcb->exitcode == VMEXIT_CR0_SEL_WRITE)
+         ? 16 : (vmcb->exitcode - VMEXIT_CR0_READ);
     dir = (cr > 15);
     cr &= 0xf;
     gp = vmcb->ei.mov_cr.gpr;
@@ -2519,7 +2520,10 @@ void asmlinkage svm_vmexit_handler(void)
 
     v->arch.hvm.guest_cr[2] = vmcb_get_cr2(vmcb);
     if ( paging_mode_hap(v->domain) )
+    {
+        v->arch.hvm.guest_cr[0] = vmcb_get_cr0(vmcb);
         v->arch.hvm.guest_cr[3] = v->arch.hvm.hw_cr[3] = vmcb_get_cr3(vmcb);
+    }
 
     if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
         vcpu_guestmode = 1;
@@ -2883,6 +2887,7 @@ void asmlinkage svm_vmexit_handler(void)
 
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
     case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
+    case VMEXIT_CR0_SEL_WRITE:
         if ( cpu_has_svm_decode && vmcb->ei.mov_cr.mov_insn )
             svm_vmexit_do_cr_access(vmcb, regs);
         else if ( !hvm_emulate_one_insn(x86_insn_is_cr_access, "CR access") )
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 975a1eaef806..d069280a4da8 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -154,6 +154,13 @@ static int construct_vmcb(struct vcpu *v)
         vmcb->_cr_intercepts &=
             ~(CR_INTERCEPT_CR3_READ|CR_INTERCEPT_CR3_WRITE);
 
+        /*
+         * Xen is not interested in changes to the MP and TS bits so use
+         * CR0_SEL_WRITE to avoid unnecessary intercepts.
+         */
+        vmcb->_cr_intercepts &= ~CR_INTERCEPT_CR0_WRITE;
+        vmcb->_general1_intercepts |= GENERAL1_INTERCEPT_CR0_SEL_WRITE;
+
         /*
          * No point in intercepting INVLPG if we don't have shadow pagetables
          * that need to be fixed up.
-- 
2.53.0

Re: [PATCH v3] x86/svm: Intercept CR0 writes selectively

Posted by Jan Beulich 1 week, 3 days ago

On 11.09.2026 11:51, Ross Lagerwall wrote:
> Since 3356d685dbda ("x86/svm: Remove lazy FPU support"), Xen does not
> need to track when the TS or MP bits change so opt to intercept CR0
> writes selectively. Aside from potentially reducing a few VMEXITs, this
> fixes a nested virt bug where L1 intercepts CR0_SEL_WRITE and L0
> intercepts CR0_WRITE. The hardware prioritizes CR0_WRITE and so L1 never
> sees any CR0 writes.
> 
> Since CR0 TS/MP bits may now change behind Xen's back, sync CR0 on
> VMEXIT so that the emulator sees the correct values.
> 
> Shadow mode continues to use the full CR0_WRITE intercept since with
> Shadow the CR0 in the VMCB is not the same as the value Xen tracks on
> behalf of the guest and allowing the guest to change one of them
> directly would be fragile.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Independently, ...

> @@ -2519,7 +2520,10 @@ void asmlinkage svm_vmexit_handler(void)
>  
>      v->arch.hvm.guest_cr[2] = vmcb_get_cr2(vmcb);
>      if ( paging_mode_hap(v->domain) )
> +    {
> +        v->arch.hvm.guest_cr[0] = vmcb_get_cr0(vmcb);
>          v->arch.hvm.guest_cr[3] = v->arch.hvm.hw_cr[3] = vmcb_get_cr3(vmcb);
> +    }

... we really want to change to !paging_mode_shadow() here and ...

> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -154,6 +154,13 @@ static int construct_vmcb(struct vcpu *v)
>          vmcb->_cr_intercepts &=
>              ~(CR_INTERCEPT_CR3_READ|CR_INTERCEPT_CR3_WRITE);
>  
> +        /*
> +         * Xen is not interested in changes to the MP and TS bits so use
> +         * CR0_SEL_WRITE to avoid unnecessary intercepts.
> +         */
> +        vmcb->_cr_intercepts &= ~CR_INTERCEPT_CR0_WRITE;
> +        vmcb->_general1_intercepts |= GENERAL1_INTERCEPT_CR0_SEL_WRITE;
> +
>          /*
>           * No point in intercepting INVLPG if we don't have shadow pagetables
>           * that need to be fixed up.

... here, as that's compile-time constant when SHADOW_PAGING=n, while
paging_mode_hap() is compile-time constant only when HVM=n (i.e.
useless here).

Jan