Re: [PATCH] x86/HVM: replace paging_mode_hap() uses

["Alejandro Vallejo" <alejandro.garciavallejo@xxxxxxx>]

On Mon Sep 21, 2026 at 3:19 PM CEST, Jan Beulich wrote:
> HVM guests cannot run without either HAP or shadow enabled. While
> paging_mode_shadow() is compile-time-constant when SHADOW_PAGING=n,
> paging_mode_hap() isn't. Hence the former is preferred to leverage DCE.
>
> In svm_update_guest_cr() combine two adjacent conditionals.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Yes, please.

  Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>

A couple of nits below. Take them or leave them.

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -113,7 +113,8 @@ static void cf_check svm_update_guest_cr
>      switch ( cr )
>      {
>      case 0:
> -        if ( paging_mode_hap(v->domain) )
> +        value = v->arch.hvm.guest_cr[0];
> +        if ( !paging_mode_shadow(v->domain) )
>          {
>              uint32_t intercepts = vmcb_get_cr_intercepts(vmcb);
>  
> @@ -122,9 +123,7 @@ static void cf_check svm_update_guest_cr
>                   monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
>                 vmcb_set_cr_intercepts(vmcb, intercepts | 
> CR_INTERCEPT_CR3_WRITE);
>          }
> -
> -        value = v->arch.hvm.guest_cr[0];
> -        if ( paging_mode_shadow(v->domain) )
> +        else
>              value |= X86_CR0_PG | X86_CR0_WP;

nit: This would be clearer with the polarity reversed. Check shadow
first and have hap later. It'd also make the diff (marginally) smaller too.

>          vmcb_set_cr0(vmcb, value);
>          break;
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c

[snip]

>      v->arch.hvm.vmx.exception_bitmap = HVM_TRAP_MASK
> -              | (paging_mode_hap(d) ? 0 : (1U << X86_EXC_PF));
> +        | (!paging_mode_shadow(d) ? 0 : (1U << X86_EXC_PF));

nit: Shouldn't | be on the prior line? It was there before, but seeing
how you're adjusting indentation might as well move that char.

In the same vein as before, it'd be a bit clearer with the polarity
inverted (shadow() ? bit : 0 )

Cheers,
Alejandro