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

[Jan Beulich <jbeulich@xxxxxxxx>]

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