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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/01/2026 10:34 am, Julian Vetter wrote:
> On 1/15/26 4:50 PM, Andrew Cooper wrote:
>> 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.
> Yes, you're right.
>> 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.
> Ok, yes, I have modified the AMD side to use MSR_K8_EXT_FEATURE_MASK to 
> "unhide" it.

Great.  And contrary to the other thread, this really must modify the
mask MSRs rather than use setup_force_cpu_cap(), because we still need
it to be visible to PV guest kernels which can't see Xen's choice of
setup_force_cpu_cap().

>
>>> +    }
>>> +
>>> +    /* 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.
> I have tried to add the logic into the early_init_{intel,amd}() 
> functions. But it seems this is already too late in the boot chain. This 
> is why I put into an extra function which is called earlier. Because it 
> seems there are already pages with PAGE_NX being used on the way to 
> early_init_{intel,amd}(). Because when I put my code into 
> early_init_intel I get a fault and a reboot. What do you suggest?

Have you got the backtrace available?

It's probably easiest if I prototype the split I'd like to see, and you
integrate with that.

~Andrew