Re: [PATCH] x86/efi: Remove NX check from efi-boot.h

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Le 27/11/2025 à 15:33, Julian Vetter a écrit :
> Currently Intel CPUs in EFI mode with the "Execute Disable Bit" disabled
> and the 'CONFIG_REQUIRE_NX=y' fail to boot, because this check is
> performed before trampoline_setup is called, which determines if NX is
> supported or if it's hidden by 'MSR_IA32_MISC_ENABLE[34] = 1' (if so,
> re-enables NX).
>
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
> ---
>   xen/arch/x86/efi/efi-boot.h | 12 ------------
>   1 file changed, 12 deletions(-)
>
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 0194720003..8dfd549f12 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -748,18 +748,6 @@ static void __init efi_arch_cpu(void)
>       if ( (eax >> 16) == 0x8000 && eax > 0x80000000U )
>       {
>           caps[FEATURESET_e1d] = cpuid_edx(0x80000001U);
> -
> -        /*
> -         * This check purposefully doesn't use cpu_has_nx because
> -         * cpu_has_nx bypasses the boot_cpu_data read if Xen was compiled
> -         * with CONFIG_REQUIRE_NX
> -         */
> -        if ( IS_ENABLED(CONFIG_REQUIRE_NX) &&
> -             !boot_cpu_has(X86_FEATURE_NX) )
> -            blexit(L"This build of Xen requires NX support");
> -
> -        if ( cpu_has_nx )
> -            trampoline_efer |= EFER_NXE;

I don't think we want to skip setting EFER_NXE. As it would mean not
using NX at all (unless I missed something).

If cpu_policy doesn't have nx, it is likely going to cause issues e.g in
VMs which will not see NX and potentially refuse to boot. I don't really
know in which order things are initialized, but it probably wants to be
considered.

Perhaps, we want to do something like detecting the
MSR_IA32_MISC_ENABLE[34] then adjusting the cpu_policy appropriately
after patching it ?

>       }
>   }
>



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech