[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86: re-enable NX if disabled



On 04/12/15 16:31, Jan Beulich wrote:
> I noticed Linux 4.4 doing this universally now, and I think it's a good
> idea to override such anti-security BIOS settings (we certainly have no
> compatibility problem due to NX being enabled).

I had a plan to do the same, so definitely +1.

>
> Secondary changes:
> - no need to check supported extended CPUID level for leaves 80000000
>   and 80000001 (required on x86-64)
> - no need to update c->cpuid_level in early_init_intel() (done anyway
>   in generic_identify())
> - alignment of trampoline data items
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/boot/trampoline.S
> +++ b/xen/arch/x86/boot/trampoline.S
> @@ -32,9 +32,13 @@ GLOBAL(trampoline_realmode_entry)
>          lmsw    %ax                       # CR0.PE = 1 (enter protected mode)
>          ljmpl   $BOOT_CS32,$bootsym_rel(trampoline_protmode_entry,6)
>  
> +        .balign 8
> +        .word   0
>  idt_48: .word   0, 0, 0 # base = limit = 0
> +        .word   0
>  gdt_48: .word   6*8-1
>          .long   bootsym_rel(trampoline_gdt,4)
> +
>  trampoline_gdt:
>          /* 0x0000: unused */
>          .quad   0x0000000000000000
> @@ -56,6 +60,9 @@ trampoline_gdt:
>          .long   trampoline_gdt + BOOT_PSEUDORM_DS + 2 - .
>          .popsection
>  
> +GLOBAL(trampoline_misc_enable_off)
> +        .quad   0
> +

For clarity, I would name this trampoline_misc_enable_mask and invert
its representation to make the asm easier.

>  GLOBAL(cpuid_ext_features)
>          .long   0
>  
> @@ -84,6 +91,21 @@ trampoline_protmode_entry:
>          add     bootsym_rel(trampoline_xen_phys_start,4,%eax)
>          mov     %eax,%cr3
>  
> +        /* Adjust IA32_MISC_ENABLE if needed. */

Possibly worth stating that this is to allow us to enable NX before
attempting to use pagetables with NX set.

> +        mov     bootsym_rel(trampoline_misc_enable_off,4,%esi)
> +        mov     bootsym_rel(trampoline_misc_enable_off+4,4,%edi)
> +        mov     %esi,%eax
> +        or      %edi,%eax
> +        jz      1f
> +        mov     $MSR_IA32_MISC_ENABLE,%ecx
> +        rdmsr
> +        not     %esi
> +        not     %edi
> +        and     %esi,%eax
> +        and     %edi,%edx
> +        wrmsr
> +1:
> +
>          /* Set up EFER (Extended Feature Enable Register). */
>          mov     bootsym_rel(cpuid_ext_features,4,%edi)
>          movl    $MSR_EFER,%ecx
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -256,15 +256,15 @@ static void __cpuinit generic_identify(s
>  
>       /* AMD-defined flags: level 0x80000001 */
>       c->extended_cpuid_level = cpuid_eax(0x80000000);
> -     if ( (c->extended_cpuid_level & 0xffff0000) == 0x80000000 ) {
> -             if ( c->extended_cpuid_level >= 0x80000001 )
> -                     cpuid(0x80000001, &tmp, &tmp,
> -                           
> &c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)],
> -                           
> &c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]);
> +     cpuid(0x80000001, &tmp, &tmp,
> +           &c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)],
> +           &c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]);
> +     if (c == &boot_cpu_data)
> +             bootsym(cpuid_ext_features) =
> +                     c->x86_capability[cpufeat_word(X86_FEATURE_NX)];
>  
> -             if ( c->extended_cpuid_level >= 0x80000004 )
> -                     get_model_name(c); /* Default name */
> -     }
> +     if (c->extended_cpuid_level >= 0x80000004)
> +             get_model_name(c); /* Default name */
>  
>       /* Intel-defined flags: level 0x00000007 */
>       if ( c->cpuid_level >= 0x00000007 )
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -162,19 +162,29 @@ static void __devinit early_init_intel(s
>       if (c->x86 == 15 && c->x86_cache_alignment == 64)
>               c->x86_cache_alignment = 128;
>  
> -     /* Unmask CPUID levels if masked: */
> +     /* Unmask CPUID levels and NX if masked: */
>       if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
> -             u64 misc_enable;
> +             u64 misc_enable, disable;
>  
>               rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>  
> -             if (misc_enable & MSR_IA32_MISC_ENABLE_LIMIT_CPUID) {
> -                     misc_enable &= ~MSR_IA32_MISC_ENABLE_LIMIT_CPUID;
> -                     wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> -                     c->cpuid_level = cpuid_eax(0);
> -                     if (opt_cpu_info || c == &boot_cpu_data)
> -                             printk(KERN_INFO "revised cpuid level: %d\n",
> -                                    c->cpuid_level);
> +             disable = misc_enable & (MSR_IA32_MISC_ENABLE_LIMIT_CPUID |
> +                                      MSR_IA32_MISC_ENABLE_XD_DISABLE);
> +             if (disable) {
> +                     wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable & ~disable);
> +                     bootsym(trampoline_misc_enable_off) |= disable;
> +             }
> +
> +             if (disable & MSR_IA32_MISC_ENABLE_LIMIT_CPUID)
> +                     printk(KERN_INFO "revised cpuid level: %d\n",
> +                            cpuid_eax(0));
> +             if (disable & MSR_IA32_MISC_ENABLE_XD_DISABLE) {
> +                     u64 efer;
> +
> +                     rdmsrl(MSR_EFER, efer);
> +                     wrmsrl(MSR_EFER, efer | EFER_NX);

{read,write}_efer(), which also update this_cpu(efer).

> +                     printk(KERN_INFO
> +                            "re-enabled NX (Execute Disable) protection\n");
>               }

Because of the asm adjustment of $MSR_IA32_MISC_ENABLE, this code will
only ever run on the BSP.

As such, it would better live somewhere like early_cpu_detect() (or a
brand new early_cpu_workaround()), making it __init.

With that, the adjustment of bootsym(cpuid_ext_features) can move as
well.  (As part of feature levelling, I had considered changing it to a
boolean indicating whether NX should be used, but that is definitely a
separate piece of cleanup.)

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.