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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/01/2026 4:32 pm, Teddy Astie wrote:
> Le 15/01/2026 à 16:50, Andrew Cooper a écrit :
>> On 15/01/2026 3:17 pm, Julian Vetter wrote:
>>> +{
>>> +    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 ?

Urgh.  Almost, but not quite, and I've apparently lost a patch.

" x86/amd: Fixes for levelling setup"

The K8 RevD and earlier have their masking MSRs at different indices.

Perhaps instead of editing the masking MSRs, just setup_force_cpu_cap(),
which confines the logic to 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?
> Sounds good to me, I suppose there is no use of NX before this ?

NX predates 64bit on AMD CPUs.

~Andrew