Re: [PATCH] x86: Add Kconfig option to require NX bit support

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On Fri, Jun 02, 2023 at 03:22:20PM +0100, Andrew Cooper wrote:
> On 01/06/2023 6:43 pm, Alejandro Vallejo wrote:
> > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> > index 09bebf8635..8414266281 100644
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S
> > @@ -647,11 +653,18 @@ trampoline_setup:
> >          cpuid
> >  1:      mov     %edx, CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM) + 
> > sym_esi(boot_cpu_data)
> >  
> > -        /* Check for NX. Adjust EFER setting if available. */
> > +        /*
> > +         * Check for NX:
> > +         *   - If Xen was compiled requiring it simply assert it's
> > +         *     supported. The trampoline already has the right constant.
> > +         *   - Otherwise, update the trampoline EFER mask accordingly.
> > +         */
> >          bt      $cpufeat_bit(X86_FEATURE_NX), %edx
> > -        jnc     1f
> > +        jnc     no_nx_bit
> > +#if !IS_ENABLED(CONFIG_REQUIRE_NX_BIT)
> >          orb     $EFER_NXE >> 8, 1 + sym_esi(trampoline_efer)
> > -1:
> > +no_nx_bit:
> > +#endif
> 
> It occurs to me...  This will prevent Xen booting in firmware
> configurations where XD-Disable is active, despite Xen having
> unconditional logic to turn XD off later in boot.
In practice setting/clearing that bit was done through a BIOS configuration
knob AFAIR, so I wouldn't be too worried about forcing it open.

> 
> Linux deals with this in verify_cpu() (early asm) along with a FMS check
> protecting the access to MSR_MISC_ENABLE, rather than using rdmsr_safe()
> and catching the #GP.
> 
On a related note, we don't use rdmsr_safe() either. We just hope it exists
on any Intel CPU. It fortunately does on any Intel CPU we care about
because it was introduced shortly before Pentium 4 (Netburst), so we're
fine since we mandate long mode.

> 
> In terms of which CPUs are a problem, we almost got very lucky.  NX is
> part of the AMD64 spec, and all AMD, VIA, Centaur and Intel Atoms have
> this property.  64bit and XD were both added midway through the Pentium
> 4 era, and appear in the Prescott E0 stepping.
> 
> However, it appears that the prior stepping, D0, had 64bit but was only
> unlocked for certain OEMs.  (At the time, Intel were still trying to
> push Itaniaum as the future, and were trying hard not to go the x86_64
> route.)
> 
> Specifically,
> https://en.wikipedia.org/wiki/List_of_Intel_Xeon_processors_(NetBurst-based)#%22Nocona%22_(90_nm)
> is the suspected problem set.
> 
> 
> So, I think this does want to turn into a series, with the first patch
> moving the XD-disable logic into this path,
I agree. Will do.

> after which I think there is
> a strong case to be made for defaulting CONFIG_REQUIRE_NX to yes.
>  
> ~Andrew
A machine with long mode and no NX would be exceedingly rare, that's
for sure.

Alejandro