|
[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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |