wp_works_ok cleanups again
Subsystems (3)
Andy Lutomirski
maintainerv2 cover wp_works_ok cleanups again
Hi Ingo-
Here it is rebased and split up.
--Andy
Andy Lutomirski (2):
x86/boot/32: Delete cpuinfo_x86::wp_works_ok
x86/boot/32: Rewrite test_wp_bit()
arch/x86/include/asm/processor.h | 2 -- arch/x86/kernel/cpu/proc.c | 5 ++--- arch/x86/kernel/setup.c | 2 -- arch/x86/mm/init_32.c | 44 ++++++++-------------------------------- arch/x86/xen/enlighten.c | 1 - 5 files changed, 11 insertions(+), 43 deletions(-)
Signature · 1 line
2.9.3
Andy Lutomirski
maintainerPATCH v2 1/2 x86/boot/32: Delete cpuinfo_x86::wp_works_ok
Linux refuses to boot if WP doesn't work okay, so tracking whether it works serves no purpose. The only use I can see at all for wp_works_ok is that it lets Xen bypass test_wp_bit(). If this is truly needed, it could be more cleanly handled using X86_FEATURE_XENPV, but it looks like Xen can handle test_wp_bit() correctly without special cases at all.
--- arch/x86/include/asm/processor.h | 2 -- arch/x86/kernel/cpu/proc.c | 5 ++--- arch/x86/kernel/setup.c | 2 -- arch/x86/mm/init_32.c | 13 +++++-------- arch/x86/xen/enlighten.c | 1 - 5 files changed, 7 insertions(+), 16 deletions(-)
arch/x86/include/asm/processor.h +0 -2
@@ -90,8 +90,6 @@ struct cpuinfo_x86 {__u8 x86_model;__u8 x86_mask;#ifdef CONFIG_X86_32 -char wp_works_ok;/* It doesn't on 386's */ -/* Problems on some 486Dx4's and old 386's: */ char rfu;char pad0;
arch/x86/kernel/cpu/proc.c +2 -3
@@ -31,14 +31,13 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)"fpu\t\t: %s\n" "fpu_exception\t: %s\n" "cpuid level\t: %d\n" -"wp\t\t: %s\n" , +"wp\t\t: yes\n" ,static_cpu_has_bug (X86_BUG_FDIV ) ?"yes" :"no" ,static_cpu_has_bug (X86_BUG_F00F ) ?"yes" :"no" ,static_cpu_has_bug (X86_BUG_COMA ) ?"yes" :"no" ,static_cpu_has (X86_FEATURE_FPU ) ?"yes" :"no" ,static_cpu_has (X86_FEATURE_FPU ) ?"yes" :"no" , - c->cpuid_level, - c->wp_works_ok ? "yes" : "no"); + c->cpuid_level); }#else static void show_cpuinfo_misc (struct seq_file *m,struct cpuinfo_x86 *c)
arch/x86/kernel/setup.c +0 -2
@@ -175,11 +175,9 @@ static struct resource bss_resource = {#ifdef CONFIG_X86_32 /* cpu data as detected by the assembly code in head.S */ struct cpuinfo_x86 new_cpu_data = { - .wp_works_ok =-1 , };/* common cpu data for all cpus */ struct cpuinfo_x86 boot_cpu_data__read_mostly = { - .wp_works_ok =-1 , };EXPORT_SYMBOL (boot_cpu_data);
arch/x86/mm/init_32.c +5 -8
@@ -731,15 +731,13 @@ static void __init test_wp_bit(void)/* Any page-aligned address will do, the test is non-destructive */ __set_fixmap (FIX_WP_TEST ,__pa (&swapper_pg_dir),PAGE_KERNEL_RO ); - boot_cpu_data.wp_works_ok =do_test_wp_bit (); -clear_fixmap (FIX_WP_TEST ); - -if (!boot_cpu_data.wp_works_ok) { +if (!do_test_wp_bit ()) {printk (KERN_CONT "No.\n" );panic ("Linux doesn't support CPUs with broken WP." ); - }else { -printk (KERN_CONT "Ok.\n" ); } +clear_fixmap (FIX_WP_TEST ); + +printk (KERN_CONT "Ok.\n" ); }void __init mem_init (void )
@@ -821,8 +819,7 @@ void __init mem_init(void)BUG_ON (VMALLOC_START >=VMALLOC_END );BUG_ON ((unsigned long )high_memory >VMALLOC_START ); -if (boot_cpu_data.wp_works_ok <0 ) -test_wp_bit (); +test_wp_bit (); }#ifdef CONFIG_MEMORY_HOTPLUG
arch/x86/xen/enlighten.c +0 -1
@@ -1595,7 +1595,6 @@ asmlinkage __visible void __init xen_start_kernel(void)/* set up basic CPUID stuff */ cpu_detect (&new_cpu_data);set_cpu_cap (&new_cpu_data,X86_FEATURE_FPU ); - new_cpu_data.wp_works_ok =1 ; new_cpu_data.x86_capability[CPUID_1_EDX ] =cpuid_edx (1 );#endif
Signature · 1 line
2.9.3
Borislav Petkov
maintainerRe: [PATCH v2 1/2] x86/boot/32: Delete cpuinfo_x86::wp_works_ok
Quoted 8 lines · Andy Lutomirski
Linux refuses to boot if WP doesn't work okay, so tracking whether it works serves no purpose. The only use I can see at all for
wp_works_okis that it lets Xen bypasstest_wp_bit(). If this is truly needed, it could be more cleanly handled usingX86_FEATURE_XENPV, but it looks like Xen can handletest_wp_bit()correctly without special cases at all.Signed-off-by Andy Lutomirski <[email protected]>
What's up?
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Re: [PATCH v2 1/2] x86/boot/32: Delete cpuinfo_x86::wp_works_ok
Quoted 8 lines · Borislav Petkov
Quoted 7 lines · Andy Lutomirski
Linux refuses to boot if WP doesn't work okay, so tracking whether it works serves no purpose. The only use I can see at all for
wp_works_okis that it lets Xen bypasstest_wp_bit(). If this is truly needed, it could be more cleanly handled usingX86_FEATURE_XENPV, but it looks like Xen can handletest_wp_bit()correctly without special cases at all.
Quoted 6 lines
Quoted 1 line
Signed-off-by Andy Lutomirski <[email protected]>What's up?
Wow, I based on tip/x86/mm per Ingo's request, but maybe that was the wrong branch, and apparently Mathias did the same thing in the mean time. Whoops. I'll rebase again.
Quoted 5 lines
--
Regards/Gruss,
Boris.Good mailing practices for 400: avoid top-posting and trim the reply.
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Re: [PATCH v2 1/2] x86/boot/32: Delete cpuinfo_x86::wp_works_ok
Quoted 11 lines · Andy Lutomirski
Quoted 10 lines · Borislav Petkov
Quoted 8 lines · Andy Lutomirski
Linux refuses to boot if WP doesn't work okay, so tracking whether it works serves no purpose. The only use I can see at all for
wp_works_okis that it lets Xen bypasstest_wp_bit(). If this is truly needed, it could be more cleanly handled usingX86_FEATURE_XENPV, but it looks like Xen can handletest_wp_bit()correctly without special cases at all.Signed-off-by Andy Lutomirski <[email protected]>
Quoted 8 lines
Quoted 4 lines
What's up?
Wow, I based on tip/x86/mm per Ingo's request, but maybe that was the wrong branch, and apparently Mathias did the same thing in the mean time. Whoops. I'll rebase again.
Oops, I didn't realize the duplication either. The splitting up of the patch that I requested made the merge easier I suspect - albeit that's an unintended side effect.
Thanks,
Ingo
Andy Lutomirski
maintainerPATCH v2 2/2 x86/boot/32: Rewrite test_wp_bit()
In reply to Andy LutomirskiThis code seems to be very old and has gotten only minor updates. It's overcomplicated and has a bunch of comments that are, at best, of purely historical interest. Nowadays we have a shiny function probe_kernel_write() that does more or less exactly what we need. Use it.
I switched the page that we test from swapper_pg_dir to empty_zero_page because writing zero to empty_zero_page is more obviously safe than writing to the paging structures. (It's extremely unlikely that any of this would cause problems in practice because the write will fail on any supported CPU.)
--- arch/x86/mm/init_32.c | 35 ++++++----------------------------- 1 file changed, 6 insertions(+), 29 deletions(-)
arch/x86/mm/init_32.c +6 -29
@@ -56,8 +56,6 @@unsigned long highstart_pfn, highend_pfn; -static noinline int do_test_wp_bit(void); -bool __read_mostly __vmalloc_start_set = false;/*
@@ -726,15 +724,18 @@ void __init paging_init(void)*/ static void __init test_wp_bit (void ) { +char z =0 ; +printk (KERN_INFO "Checking if this processor honours the WP bit even in supervisor mode..." ); -/* Any page-aligned address will do, the test is non-destructive */ -__set_fixmap (FIX_WP_TEST ,__pa (&swapper_pg_dir),PAGE_KERNEL_RO ); -if (!do_test_wp_bit ()) { +__set_fixmap (FIX_WP_TEST ,__pa_symbol (empty_zero_page),PAGE_KERNEL_RO ); + +if (probe_kernel_write ((char *)fix_to_virt (FIX_WP_TEST ), &z,1 ) ==0 ) {printk (KERN_CONT "No.\n" );panic ("Linux doesn't support CPUs with broken WP." ); } +clear_fixmap (FIX_WP_TEST );printk (KERN_CONT "Ok.\n" );
@@ -847,30 +848,6 @@ int arch_remove_memory(u64 start, u64 size)#endif #endif -/* -* This function cannot be __init, since exceptions don't work in that -* section. Put this after the callers, so that it cannot be inlined. -*/ -static noinline int do_test_wp_bit(void) -{ -char tmp_reg; -int flag; - - __asm__ __volatile__( - " movb %0, %1 \n" - "1: movb %1, %0 \n" - " xorl %2, %2 \n" - "2: \n" - _ASM_EXTABLE(1b,2b) - :"=m" (*(char *)fix_to_virt(FIX_WP_TEST)), - "=q" (tmp_reg), - "=r" (flag) - :"2" (1) - :"memory"); - -return flag; -} -int kernel_set_to_readonly__read_mostly ;void set_kernel_text_rw (void )
Signature · 1 line
2.9.3