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

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

Le 15/01/2026 à 16:50, Andrew Cooper a écrit :
> 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.
>

I see this MSR in the BKDG (bit 20 is NX). Are these bits stable across
the AMD CPU generations ?

>> +    }
>> +
>> +    /* 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?

Sounds good to me, I suppose there is no use of NX before this ?

>
> ~Andrew
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech