Re: [XEN PATCH v2] x86/hvm: Use constants for x86 modes

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.12.2024 10:49, Teddy Astie wrote:
> In many places of x86 HVM code, constants integer are used to indicate in 
> what mode is
> running the CPU (real, vm86, 16-bits, 32-bits, 64-bits). However, these 
> constants are
> are written directly as integer which hides the actual meaning of these modes.
> 
> This patch introduces X86_MODE_* macros and replace those occurences with it.
> 
> Signed-off-by Teddy Astie <teddy.astie@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with further style adjustments (see below) and ideally with ...

> ---
> v2:
> Formatting changes (alignment, ...)
> Renamed v86 to vm86. (Jan)

... this extended to ...

> @@ -2952,11 +2954,11 @@ static const char *guest_x86_mode_to_str(int mode)
>  {
>      switch ( mode )
>      {
> -    case 0:  return "Real";
> -    case 1:  return "v86";
> -    case 2:  return "16bit";
> -    case 4:  return "32bit";
> -    case 8:  return "64bit";
> +    case X86_MODE_REAL:   return "Real";
> +    case X86_MODE_VM86:   return "v86";

... the string literal here.

> @@ -112,23 +113,23 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>  
>      switch ( mode )
>      {
> -    case 8:
> +    case X86_MODE_64BIT:
>          eax = regs->rax;
>          fallthrough;
> -    case 4:
> -    case 2:
> +    case X86_MODE_32BIT:
> +    case X86_MODE_16BIT:
>          if ( currd->arch.monitor.guest_request_userspace_enabled &&
>              eax == __HYPERVISOR_hvm_op &&
> -            (mode == 8 ? regs->rdi : regs->ebx) == 
> HVMOP_guest_request_vm_event )
> +            (mode == X86_MODE_64BIT ? regs->rdi : regs->ebx) == 
> HVMOP_guest_request_vm_event )

This line is too long now.

> @@ -2073,7 +2073,8 @@ int nvmx_handle_vmx_insn(struct cpu_user_regs *regs, 
> unsigned int exit_reason)
>  
>      if ( !(curr->arch.hvm.guest_cr[4] & X86_CR4_VMXE) ||
>           !nestedhvm_enabled(curr->domain) ||
> -         (vmx_guest_x86_mode(curr) < (hvm_long_mode_active(curr) ? 8 : 2)) ||
> +         (vmx_guest_x86_mode(curr) < (hvm_long_mode_active(curr) ? 
> X86_MODE_64BIT
> +                                                                 : 
> X86_MODE_16BIT)) ||

As are these two.

Likely easy enough to adjust while committing.

Jan