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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2026 14:23, 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 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@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with two remarks (which I may take the liberty of carrying out while
committing):

> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -50,13 +50,13 @@ static int construct_vmcb(struct vcpu *v)
>      struct vmcb_struct *vmcb = svm->vmcb;
>  
>      vmcb->_general1_intercepts =
> -        GENERAL1_INTERCEPT_INTR        | GENERAL1_INTERCEPT_NMI         |
> -        GENERAL1_INTERCEPT_SMI         | GENERAL1_INTERCEPT_INIT        |
> -        GENERAL1_INTERCEPT_CPUID       | GENERAL1_INTERCEPT_INVD        |
> -        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_INTR          | GENERAL1_INTERCEPT_NMI           |
> +        GENERAL1_INTERCEPT_SMI           | GENERAL1_INTERCEPT_INIT          |
> +        GENERAL1_INTERCEPT_CR0_SEL_WRITE | GENERAL1_INTERCEPT_CPUID         |
> +        GENERAL1_INTERCEPT_INVD          | GENERAL1_INTERCEPT_HLT           |
> +        GENERAL1_INTERCEPT_INVLPG        | GENERAL1_INTERCEPT_INVLPGA       |
> +        GENERAL1_INTERCEPT_IOIO_PROT     | GENERAL1_INTERCEPT_MSR_PROT      |
> +        GENERAL1_INTERCEPT_TASK_SWITCH   | GENERAL1_INTERCEPT_SHUTDOWN_EVT;

I think it would be nice to avoid moving the |-s out, to keep ...

>      vmcb->_general2_intercepts =
>          GENERAL2_INTERCEPT_VMRUN       | GENERAL2_INTERCEPT_VMMCALL     |
>          GENERAL2_INTERCEPT_VMLOAD      | GENERAL2_INTERCEPT_VMSAVE      |

... aligning with the ones here.

> @@ -76,8 +76,12 @@ 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. */
> -    vmcb->_cr_intercepts = ~(CR_INTERCEPT_CR2_READ |
> +    /*
> +     * Intercept all control-register accesses except for CR0 writes (use
> +     * selective write instead), and CR2 and CR8 reads/writes.
> +     */

Imo slightly more precise as "... (using selective write intercept instead) ..."

Jan

> +    vmcb->_cr_intercepts = ~(CR_INTERCEPT_CR0_WRITE |
> +                             CR_INTERCEPT_CR2_READ |
>                               CR_INTERCEPT_CR2_WRITE |
>                               CR_INTERCEPT_CR8_READ |
>                               CR_INTERCEPT_CR8_WRITE);