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

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

[Ross Lagerwall]

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 may now change behind Xen's back, sync it on VMEXIT so that
the emulator sees the correct value.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/svm.c  | 7 +++++--
 xen/arch/x86/hvm/svm/vmcb.c | 8 +++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 5f5d903d872d..8da879a5af67 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;
@@ -2517,6 +2518,7 @@ void asmlinkage svm_vmexit_handler(void)
     hvm_sanitize_regs_fields(
         regs, !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l));
 
+    v->arch.hvm.guest_cr[0] = vmcb_get_cr0(vmcb);
     v->arch.hvm.guest_cr[2] = vmcb_get_cr2(vmcb);
     if ( paging_mode_hap(v->domain) )
         v->arch.hvm.guest_cr[3] = v->arch.hvm.hw_cr[3] = vmcb_get_cr3(vmcb);
@@ -2882,7 +2884,8 @@ void asmlinkage svm_vmexit_handler(void)
         break;
 
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
-    case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
+    case VMEXIT_CR1_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..7ee91937b10c 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -56,7 +56,7 @@ static int construct_vmcb(struct vcpu *v)
         GENERAL1_INTERCEPT_HLT         | GENERAL1_INTERCEPT_INVLPG      |
         GENERAL1_INTERCEPT_INVLPGA     | GENERAL1_INTERCEPT_IOIO_PROT   |
         GENERAL1_INTERCEPT_MSR_PROT    | GENERAL1_INTERCEPT_SHUTDOWN_EVT|
-        GENERAL1_INTERCEPT_TASK_SWITCH;
+        GENERAL1_INTERCEPT_TASK_SWITCH | GENERAL1_INTERCEPT_CR0_SEL_WRITE;
     vmcb->_general2_intercepts =
         GENERAL2_INTERCEPT_VMRUN       | GENERAL2_INTERCEPT_VMMCALL     |
         GENERAL2_INTERCEPT_VMLOAD      | GENERAL2_INTERCEPT_VMSAVE      |
@@ -76,11 +76,13 @@ static int construct_vmcb(struct vcpu *v)
     /* Intercept all debug-register writes. */
     vmcb->_dr_intercepts = ~0u;
 
-    /* Intercept all control-register accesses except for CR2 and CR8. */
+    /* Intercept all control-register accesses except for CR2, CR8 and
+     * CR0 (covered by selective write). */
     vmcb->_cr_intercepts = ~(CR_INTERCEPT_CR2_READ |
                              CR_INTERCEPT_CR2_WRITE |
                              CR_INTERCEPT_CR8_READ |
-                             CR_INTERCEPT_CR8_WRITE);
+                             CR_INTERCEPT_CR8_WRITE |
+                             CR_INTERCEPT_CR0_WRITE);
 
     svm->vmcb_sync_state = vmcb_needs_vmload;
 
-- 
2.53.0

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

[Jan Beulich]

On 28.08.2026 16:19, Ross Lagerwall wrote:
> Xen does not need to track when the TS or MP bits change so opt to
> intercept CR0 writes selectively.

Not anymore, which may want expressing here (or else it looks as if this
would have been possible from the start).

> 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.

Yet if L1 sets CR0_WRITE, since intercept masks are ORed together (if
I'm not mistaken), ...

> @@ -2882,7 +2884,8 @@ void asmlinkage svm_vmexit_handler(void)
>          break;
>  
>      case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
> -    case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
> +    case VMEXIT_CR1_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") )

... we may still see VMEXIT_CR0_WRITE here (i.e. its handling cannot be
removed).

> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -56,7 +56,7 @@ static int construct_vmcb(struct vcpu *v)
>          GENERAL1_INTERCEPT_HLT         | GENERAL1_INTERCEPT_INVLPG      |
>          GENERAL1_INTERCEPT_INVLPGA     | GENERAL1_INTERCEPT_IOIO_PROT   |
>          GENERAL1_INTERCEPT_MSR_PROT    | GENERAL1_INTERCEPT_SHUTDOWN_EVT|
> -        GENERAL1_INTERCEPT_TASK_SWITCH;
> +        GENERAL1_INTERCEPT_TASK_SWITCH | GENERAL1_INTERCEPT_CR0_SEL_WRITE;

I understand GENERAL1_INTERCEPT_SHUTDOWN_EVT is an existing outlier here,
but can we please not add more? This expression is sorted by bit position,
with said exception. (Yes, that'll be more churn, but I think that's
acceptable here.)

> @@ -76,11 +76,13 @@ static int construct_vmcb(struct vcpu *v)
>      /* Intercept all debug-register writes. */
>      vmcb->_dr_intercepts = ~0u;
>  
> -    /* Intercept all control-register accesses except for CR2 and CR8. */
> +    /* Intercept all control-register accesses except for CR2, CR8 and
> +     * CR0 (covered by selective write). */
>      vmcb->_cr_intercepts = ~(CR_INTERCEPT_CR2_READ |
>                               CR_INTERCEPT_CR2_WRITE |
>                               CR_INTERCEPT_CR8_READ |
> -                             CR_INTERCEPT_CR8_WRITE);
> +                             CR_INTERCEPT_CR8_WRITE |
> +                             CR_INTERCEPT_CR0_WRITE);

In both comment and code I think it would be nice if numeric sorting was
retained. In the comment you also want to mirror what the code does (it
only excludes writes, not reads). Finally - nit: Comment style.

Jan

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

[Ross Lagerwall]

On 9/3/26 2:02 PM, Jan Beulich wrote:
> On 28.08.2026 16:19, Ross Lagerwall wrote:
>> Xen does not need to track when the TS or MP bits change so opt to
>> intercept CR0 writes selectively.
> 
> Not anymore, which may want expressing here (or else it looks as if this
> would have been possible from the start).
> 
>> 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.
> 
> Yet if L1 sets CR0_WRITE, since intercept masks are ORed together (if
> I'm not mistaken), ...
> 
>> @@ -2882,7 +2884,8 @@ void asmlinkage svm_vmexit_handler(void)
>>           break;
>>   
>>       case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
>> -    case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
>> +    case VMEXIT_CR1_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") )
> 
> ... we may still see VMEXIT_CR0_WRITE here (i.e. its handling cannot be
> removed).
> 

No, it shouldn't get here in that case. If L1 sets CR0_WRITE, it will enter
nestedsvm_check_intercepts() and then hit the NESTEDHVM_VMEXIT_INJECT case.

Ross

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

[Jan Beulich]

On 03.09.2026 15:35, Ross Lagerwall wrote:
> On 9/3/26 2:02 PM, Jan Beulich wrote:
>> On 28.08.2026 16:19, Ross Lagerwall wrote:
>>> Xen does not need to track when the TS or MP bits change so opt to
>>> intercept CR0 writes selectively.
>>
>> Not anymore, which may want expressing here (or else it looks as if this
>> would have been possible from the start).
>>
>>> 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.
>>
>> Yet if L1 sets CR0_WRITE, since intercept masks are ORed together (if
>> I'm not mistaken), ...
>>
>>> @@ -2882,7 +2884,8 @@ void asmlinkage svm_vmexit_handler(void)
>>>           break;
>>>   
>>>       case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
>>> -    case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
>>> +    case VMEXIT_CR1_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") )
>>
>> ... we may still see VMEXIT_CR0_WRITE here (i.e. its handling cannot be
>> removed).
> 
> No, it shouldn't get here in that case. If L1 sets CR0_WRITE, it will enter
> nestedsvm_check_intercepts() and then hit the NESTEDHVM_VMEXIT_INJECT case.

Hmm, okay. Yet then I still think the original case label shouldn't be
removed, as we don't exclude the CR2 or CR8 cases either.

Jan

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

[Andrew Cooper]

On 03/09/2026 2:02 pm, Jan Beulich wrote:
>> 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.
> Yet if L1 sets CR0_WRITE, since intercept masks are ORed together (if
> I'm not mistaken),

L0's choice of intercepts needs combining with L1's choice of intercepts.

Mostly this is OR-ing/unioning, but not exclusively, just like levelling
for migrate isn't a straight interception.

~Andrew

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

[Andrew Cooper]

On 03/09/2026 3:31 pm, Andrew Cooper wrote:
> On 03/09/2026 2:02 pm, Jan Beulich wrote:
>>> 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.
>> Yet if L1 sets CR0_WRITE, since intercept masks are ORed together (if
>> I'm not mistaken),
> L0's choice of intercepts needs combining with L1's choice of intercepts.
>
> Mostly this is OR-ing/unioning, but not exclusively, just like levelling
> for migrate isn't a straight interception.

Sorry, intersection.

~Andrew