Re: [PATCH] xen: Move NX handling to a dedicated place

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/01/2026 3:17 pm, Julian Vetter wrote:
> Currently the CONFIG_REQUIRE_NX prevents booting XEN, if NX is disabled
> in the BIOS. AMD doesn't have a software-accessible MSR to re-enable it,
> so there is nothing we can do. The system is going to die anyway. But on
> Intel NX might just be hidden via IA32_MISC_ENABLE.XD_DISABLE. But the
> function to re-enable it is called after the check + panic in
> efi_arch_cpu. So, this patch removes the early check and moves the
> entire NX handling into a dedicated place.
>
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>

Sorry I didn't get around to doing the prep work I promised.

This is going along the right lines, but there are a few complexities still.

Also you'll want to split the patch into a series.  More on that when
we've sorted out a few other details.

> diff --git a/xen/arch/x86/boot/trampoline.S b/xen/arch/x86/boot/trampoline.S
> index a92e399fbe..8e8d50cbdf 100644
> --- a/xen/arch/x86/boot/trampoline.S
> +++ b/xen/arch/x86/boot/trampoline.S
> @@ -144,10 +144,9 @@ gdt_48:
>  GLOBAL(trampoline_misc_enable_off)
>          .quad   0
>  
> -/* EFER OR-mask for boot paths.  SCE conditional on PV support, NX added 
> when available. */
> +/* EFER OR-mask for boot paths.  SCE conditional on PV support. */

The comment wants to stay as-was.  NX does get added when available.

>  GLOBAL(trampoline_efer)
> -        .long   EFER_LME | (EFER_SCE * IS_ENABLED(CONFIG_PV)) | \
> -                (EFER_NXE * IS_ENABLED(CONFIG_REQUIRE_NX))
> +        .long   EFER_LME | (EFER_SCE * IS_ENABLED(CONFIG_PV))
>  
>  GLOBAL(trampoline_xen_phys_start)
>          .long   0
> diff --git a/xen/arch/x86/include/asm/setup.h 
> b/xen/arch/x86/include/asm/setup.h
> index b01e83a8ed..16f53725ca 100644
> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -70,4 +70,6 @@ extern bool opt_dom0_msr_relaxed;
>  
>  #define max_init_domid (0)
>  
> +void nx_init(void);
> +
>  #endif
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 27c63d1d97..608720b717 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1119,6 +1119,50 @@ static struct domain *__init create_dom0(struct 
> boot_info *bi)
>      return d;
>  }
>  
> +void __init nx_init(void)

This should be static if it's only used in a single file.  However, see
later for doing it a bit differently.

> +{
> +    uint64_t misc_enable;
> +    uint32_t eax, ebx, ecx, edx;
> +
> +    if ( !boot_cpu_has(X86_FEATURE_NX) )
> +    {
> +        /* Intel: try to unhide NX by clearing XD_DISABLE */
> +        cpuid(0, &eax, &ebx, &ecx, &edx);
> +        if ( ebx == X86_VENDOR_INTEL_EBX &&
> +             ecx == X86_VENDOR_INTEL_ECX &&
> +             edx == X86_VENDOR_INTEL_EDX )
> +        {
> +            rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +            if ( misc_enable & MSR_IA32_MISC_ENABLE_XD_DISABLE )
> +            {
> +                misc_enable &= ~MSR_IA32_MISC_ENABLE_XD_DISABLE;
> +                wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> +
> +                /* Re-read CPUID after having cleared XD_DISABLE */
> +                boot_cpu_data.x86_capability[FEATURESET_e1d] = 
> cpuid_edx(0x80000001U);
> +
> +                /* Adjust misc_enable_off for secondary startup and wakeup 
> code */
> +                bootsym(trampoline_misc_enable_off) |= 
> MSR_IA32_MISC_ENABLE_XD_DISABLE;
> +                printk(KERN_INFO "re-enabled NX (Execute Disable) 
> protection\n");
> +            }
> +        }
> +        /* AMD: nothing we can do - NX must be enabled in BIOS */

The BIOS is only hiding the CPUID bit.  It's not blocking the use of NX.

You want to do a wrmsr_safe() trying to set EFER.NXE, and if it
succeeds, set the NX bit in MSR_K8_EXT_FEATURE_MASK to "unhide" it in
regular CPUID.  This is a little more tricky to arrange because it needs
doing on each CPU, not just the BSP.

> +    }
> +
> +    /* Enable EFER.NXE only if NX is available */
> +    if ( boot_cpu_has(X86_FEATURE_NX) )
> +    {
> +        if ( !(read_efer() & EFER_NXE) )
> +            write_efer(read_efer() | EFER_NXE);
> +
> +        /* Adjust trampoline_efer for secondary startup and wakeup code */
> +        bootsym(trampoline_efer) |= EFER_NXE;
> +    }
> +
> +    if ( IS_ENABLED(CONFIG_REQUIRE_NX) && !boot_cpu_has(X86_FEATURE_NX) )
> +        panic("This build of Xen requires NX support\n");
> +}
> +
>  /* How much of the directmap is prebuilt at compile time. */
>  #define PREBUILT_MAP_LIMIT (1 << L2_PAGETABLE_SHIFT)
>  
> @@ -1159,6 +1203,8 @@ void asmlinkage __init noreturn __start_xen(void)
>      rdmsrl(MSR_EFER, this_cpu(efer));
>      asm volatile ( "mov %%cr4,%0" : "=r" (info->cr4) );
>  
> +    nx_init();
> +
>      /* Enable NMIs.  Our loader (e.g. Tboot) may have left them disabled. */
>      enable_nmis();
>  

This is too early, as can be seen by the need to make a cpuid() call
rather than using boot_cpu_data.

The cleanup I wanted to do was to create/rework early_cpu_init() to get
things in a better order, so the panic() could go at the end here.  The
current split we've got of early/regular CPU init was inherited from
Linux and can be collapsed substantially.

The intel "unlocking" wants to move back into early_init_intel(), along
with intel_unlock_cpuid_leaves().  (This is where it used to live before
REQUIRE_NX was added).

The AMD side probe wants to live in early_amd_init()  (not that there is
one right now), but the re-enabling of the NX bit in CPUID needs to also
be in amd_init() so it gets applied to APs too.

Does this make sense?

~Andrew