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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.09.2026 15:45, Alejandro Vallejo wrote:
> 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>

Thanks.

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

Not sure about diff size, but I deliberately didn't want to inverse
polarities anywhere. Switching the predicate I was hoping to be
sufficiently uncontroversial (except, as Andrew points out, it having
a slightly negative doc effect in a few places). Inverting polarities
of if/else-like constructs, otoh, can affect code gen, and we would
likely not want to favor shadow over HAP now that shadow is off by
default.

Jan