Re: [PATCH 3/5] VMX: don't run with CR4.VMXE set when VMX could not be enabled

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Nov 16, 2023 at 02:32:07PM +0100, Jan Beulich wrote:
> While generally benign, doing so is still at best misleading.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Using set_in_cr4() seems favorable over updating mmu_cr4_features
> despite the resulting redundant CR4 update. But I certainly could be
> talked into going the alternative route.

Hm, yes I wondered the same, overall I guess it's safer to just use
set_in_cr4() and do the redundant CR4 write. It's just for the BSP
anyway.

> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2959,7 +2959,7 @@ static bool __init has_if_pschange_mc(vo
>  
>  const struct hvm_function_table * __init start_vmx(void)
>  {
> -    set_in_cr4(X86_CR4_VMXE);
> +    write_cr4(read_cr4() | X86_CR4_VMXE);
>  
>      if ( vmx_vmcs_init() )
>      {
> @@ -2967,6 +2967,9 @@ const struct hvm_function_table * __init
>          return NULL;

Do we want to also clear VMXE from CR4 here?

Thanks, Roger.