[PATCH 0/6] struct cpuinfo_x86 related cleanups

[PATCH 0/6] struct cpuinfo_x86 related cleanups

[Mathias Krause]

This small series slims down the x86 specific struct cpuinfo_x86
(patches 1 and 2). It's kind of a continuation of Boris' work from 2013.

Beside the x86 specific part it also cleans up other arches that had to
have a wp_works_ok variable / define back in the old v1.1 times. But
those times are long gone, so we can get rid of that ancient hackery.

Therefore patches 4, 5 and 6 are independent of the x86 specific changes
and could be taken by the individual arch maintainers if they prefer to.

Please apply,
Mathias

Mathias Krause (6):
  x86: drop unneded members of struct cpuinfo_x86
  x86/cpu: drop wp_works_ok member of struct cpuinfo_x86
  x86/cpu: proc - remove "wp" status line in cpuinfo
  sparc: remove unused wp_works_ok macro
  cris: remove unused wp_works_ok macro
  m68k: paging_init - remove dead code

 arch/cris/include/arch-v10/arch/processor.h |    3 ---
 arch/m68k/mm/sun3mmu.c                      |    3 ---
 arch/sparc/include/asm/processor_32.h       |    6 ------
 arch/sparc/include/asm/processor_64.h       |    4 ----
 arch/x86/include/asm/processor.h            |   11 ++---------
 arch/x86/kernel/cpu/proc.c                  |    9 +++------
 arch/x86/kernel/setup.c                     |   11 ++++-------
 arch/x86/mm/init_32.c                       |    9 +++++----
 arch/x86/xen/enlighten.c                    |    1 -
 9 files changed, 14 insertions(+), 43 deletions(-)

-- 
1.7.10.4

[PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

[Mathias Krause]

Those member serve no purpose -- not even fill padding for alignment or
such. So just get rid of them.

Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/include/asm/processor.h |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index e6cfe7ba2d65..bf7cb1e00ce7 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -80,7 +80,7 @@ enum tlb_infos {
 
 /*
  *  CPU type and hardware bug flags. Kept separately for each CPU.
- *  Members of this structure are referenced in head.S, so think twice
+ *  Members of this structure are referenced in head_32.S, so think twice
  *  before touching them. [mj]
  */
 
@@ -91,11 +91,6 @@ struct cpuinfo_x86 {
 	__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;
-	char			pad1;
 #else
 	/* Number of 4K pages in DTLB/ITLB combined(in pages): */
 	int			x86_tlbsize;
-- 
1.7.10.4

[PATCH 2/6] x86/cpu: drop wp_works_ok member of struct cpuinfo_x86

[Mathias Krause]

Remove the wp_works_ok member of struct cpuinfo_x86. It's an
optimization back from Linux v0.99 times where we had no fixup support
yet and did the CR0.WP test via special code in the page fault handler.
The < 0 test was an optimization to not do the special casing for each
NULL ptr access violation but just for the first one doing the WP test.
Today it serves no real purpose as the test no longer needs special code
in the page fault handler and the only call side -- mem_init() -- calls
it just once, anyway. However, Xen pre-initializes it to 1, to skip the
test.

Doing the test again for Xen should be no issue at all, as even the
commit introducing skipping the test (commit d560bc61575e ("x86, xen:
Suppress WP test on Xen")) mentioned it being ban aid only. And, in
fact, testing the patch on Xen showed nothing breaks.

The pre-fixup times are long gone and with the removal of the fallback
handling code in commit a5c2a893dbd4 ("x86, 386 removal: Remove
CONFIG_X86_WP_WORKS_OK") the kernel requires a working CR0.WP anyway.
So just get rid of the "optimization" and do the test unconditionally.

Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Cc: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/include/asm/processor.h |    4 +---
 arch/x86/kernel/cpu/proc.c       |    5 ++---
 arch/x86/kernel/setup.c          |   11 ++++-------
 arch/x86/mm/init_32.c            |    9 +++++----
 arch/x86/xen/enlighten.c         |    1 -
 5 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index bf7cb1e00ce7..7b15b29e8a66 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -89,9 +89,7 @@ struct cpuinfo_x86 {
 	__u8			x86_vendor;	/* CPU vendor */
 	__u8			x86_model;
 	__u8			x86_mask;
-#ifdef CONFIG_X86_32
-	char			wp_works_ok;	/* It doesn't on 386's */
-#else
+#ifdef CONFIG_X86_64
 	/* Number of 4K pages in DTLB/ITLB combined(in pages): */
 	int			x86_tlbsize;
 #endif
diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index 18ca99f2798b..6df621ae62a7 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -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)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 4cfba947d774..ffc2791ab256 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -173,14 +173,11 @@ int default_check_phys_apicid_present(int phys_apicid)
 
 
 #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,
-};
+/* cpu data as detected by the assembly code in head_32.S */
+struct cpuinfo_x86 new_cpu_data;
+
 /* common cpu data for all cpus */
-struct cpuinfo_x86 boot_cpu_data __read_mostly = {
-	.wp_works_ok = -1,
-};
+struct cpuinfo_x86 boot_cpu_data __read_mostly;
 EXPORT_SYMBOL(boot_cpu_data);
 
 unsigned int def_to_bigsmp;
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 928d657de829..e0fd0c8b9ad1 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -716,15 +716,17 @@ void __init paging_init(void)
  */
 static void __init test_wp_bit(void)
 {
+	int wp_works_ok;
+
 	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);
-	boot_cpu_data.wp_works_ok = do_test_wp_bit();
+	wp_works_ok = do_test_wp_bit();
 	clear_fixmap(FIX_WP_TEST);
 
-	if (!boot_cpu_data.wp_works_ok) {
+	if (!wp_works_ok) {
 		printk(KERN_CONT "No.\n");
 		panic("Linux doesn't support CPUs with broken WP.");
 	} else {
@@ -811,8 +813,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
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 51ef95232725..f37b297bdedc 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1682,7 +1682,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
 
-- 
1.7.10.4

[PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Mathias Krause]

As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
correctly. This makes a process reading this file always see "wp : yes"
here -- otherwise there would be no process to begin with ;)

As this status line in /proc/cpuinfo serves no purpose for quite some
time now, get rid of it.

Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@linux.intel.com>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/kernel/cpu/proc.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index 6df621ae62a7..c6c5217a7980 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
 		   "coma_bug\t: %s\n"
 		   "fpu\t\t: %s\n"
 		   "fpu_exception\t: %s\n"
-		   "cpuid level\t: %d\n"
-		   "wp\t\t: yes\n",
+		   "cpuid level\t: %d\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",
@@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
 	seq_printf(m,
 		   "fpu\t\t: yes\n"
 		   "fpu_exception\t: yes\n"
-		   "cpuid level\t: %d\n"
-		   "wp\t\t: yes\n",
+		   "cpuid level\t: %d\n",
 		   c->cpuid_level);
 }
 #endif
-- 
1.7.10.4

[PATCH 4/6] sparc: remove unused wp_works_ok macro

[Mathias Krause]

It's unused for ages, used to be required for ksyms.c back in the v1.1
times.

Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/sparc/include/asm/processor_32.h |    6 ------
 arch/sparc/include/asm/processor_64.h |    4 ----
 2 files changed, 10 deletions(-)

diff --git a/arch/sparc/include/asm/processor_32.h b/arch/sparc/include/asm/processor_32.h
index 365d4cb267b4..dd27159819eb 100644
--- a/arch/sparc/include/asm/processor_32.h
+++ b/arch/sparc/include/asm/processor_32.h
@@ -18,12 +18,6 @@
 #include <asm/signal.h>
 #include <asm/page.h>
 
-/*
- * The sparc has no problems with write protection
- */
-#define wp_works_ok 1
-#define wp_works_ok__is_a_macro /* for versions in ksyms.c */
-
 /* Whee, this is STACK_TOP + PAGE_SIZE and the lowest kernel address too...
  * That one page is used to protect kernel from intruders, so that
  * we can make our access_ok test faster
diff --git a/arch/sparc/include/asm/processor_64.h b/arch/sparc/include/asm/processor_64.h
index 6448cfc8292f..b58ee9018433 100644
--- a/arch/sparc/include/asm/processor_64.h
+++ b/arch/sparc/include/asm/processor_64.h
@@ -18,10 +18,6 @@
 #include <asm/ptrace.h>
 #include <asm/page.h>
 
-/* The sparc has no problems with write protection */
-#define wp_works_ok 1
-#define wp_works_ok__is_a_macro /* for versions in ksyms.c */
-
 /*
  * User lives in his very own context, and cannot reference us. Note
  * that TASK_SIZE is a misnomer, it really gives maximum user virtual
-- 
1.7.10.4

[PATCH 5/6] cris: remove unused wp_works_ok macro

[Mathias Krause]

It had no use since it's introduction in v2.4.1.2. Get rid of it.

Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Mikael Starvik <starvik@axis.com>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/cris/include/arch-v10/arch/processor.h |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/cris/include/arch-v10/arch/processor.h b/arch/cris/include/arch-v10/arch/processor.h
index 93feb2a487d8..58f75bee1d6c 100644
--- a/arch/cris/include/arch-v10/arch/processor.h
+++ b/arch/cris/include/arch-v10/arch/processor.h
@@ -7,9 +7,6 @@
  */
 #define current_text_addr() ({void *pc; __asm__ ("move.d $pc,%0" : "=rm" (pc)); pc; })
 
-/* CRIS has no problems with write protection */
-#define wp_works_ok 1
-
 /* CRIS thread_struct. this really has nothing to do with the processor itself, since
  * CRIS does not do any hardware task-switching, but it's here for legacy reasons.
  * The thread_struct here is used when task-switching using _resume defined in entry.S.
-- 
1.7.10.4

[PATCH 6/6] m68k: paging_init - remove dead code

[Mathias Krause]

The macro TEST_VERIFY_AREA can never be defined as there's no
wp_works_ok variable. So just remove the dead code.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/m68k/mm/sun3mmu.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/m68k/mm/sun3mmu.c b/arch/m68k/mm/sun3mmu.c
index b5b7d53f7283..177d776de1a0 100644
--- a/arch/m68k/mm/sun3mmu.c
+++ b/arch/m68k/mm/sun3mmu.c
@@ -44,9 +44,6 @@ void __init paging_init(void)
 	unsigned long zones_size[MAX_NR_ZONES] = { 0, };
 	unsigned long size;
 
-#ifdef TEST_VERIFY_AREA
-	wp_works_ok = 0;
-#endif
 	empty_zero_page = alloc_bootmem_pages(PAGE_SIZE);
 
 	address = PAGE_OFFSET;
-- 
1.7.10.4

Re: [PATCH 4/6] sparc: remove unused wp_works_ok macro

[David Miller]

From: Mathias Krause <minipli@googlemail.com>
Date: Sun, 12 Feb 2017 22:12:10 +0100

> It's unused for ages, used to be required for ksyms.c back in the v1.1
> times.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 6/6] m68k: paging_init - remove dead code

[Geert Uytterhoeven]

On Sun, Feb 12, 2017 at 10:12 PM, Mathias Krause <minipli@googlemail.com> wrote:
> The macro TEST_VERIFY_AREA can never be defined as there's no
> wp_works_ok variable. So just remove the dead code.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>

Thanks, applied with rewritten one-line summary to match subsystem
style, and queued for v4.11.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/6] cris: remove unused wp_works_ok macro

[Jesper Nilsson]

On Sun, Feb 12, 2017 at 10:12:11PM +0100, Mathias Krause wrote:
> It had no use since it's introduction in v2.4.1.2. Get rid of it.

Agreed.

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

> Cc: Mikael Starvik <starvik@axis.com>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
>  arch/cris/include/arch-v10/arch/processor.h |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/cris/include/arch-v10/arch/processor.h b/arch/cris/include/arch-v10/arch/processor.h
> index 93feb2a487d8..58f75bee1d6c 100644
> --- a/arch/cris/include/arch-v10/arch/processor.h
> +++ b/arch/cris/include/arch-v10/arch/processor.h
> @@ -7,9 +7,6 @@
>   */
>  #define current_text_addr() ({void *pc; __asm__ ("move.d $pc,%0" : "=rm" (pc)); pc; })
>  
> -/* CRIS has no problems with write protection */
> -#define wp_works_ok 1
> -
>  /* CRIS thread_struct. this really has nothing to do with the processor itself, since
>   * CRIS does not do any hardware task-switching, but it's here for legacy reasons.
>   * The thread_struct here is used when task-switching using _resume defined in entry.S.
> -- 
> 1.7.10.4

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

[Borislav Petkov]

On Sun, Feb 12, 2017 at 10:12:07PM +0100, Mathias Krause wrote:
> Those member serve no purpose -- not even fill padding for alignment or
> such. So just get rid of them.

Well, almost. You need the wp_works_ok removal patch too, otherwise you
have the 3 bytes hole below.

But the wp_works_ok goes away too so I guess that's fine.

$ pahole -C cpuinfo_x86 vmlinux
struct cpuinfo_x86 {
        __u8                       x86;                  /*     0     1 */
        __u8                       x86_vendor;           /*     1     1 */
        __u8                       x86_model;            /*     2     1 */
        __u8                       x86_mask;             /*     3     1 */
        char                       wp_works_ok;          /*     4     1 */
        __u8                       x86_virt_bits;        /*     5     1 */
        __u8                       x86_phys_bits;        /*     6     1 */
        __u8                       x86_coreid_bits;      /*     7     1 */
        __u8                       cu_id;                /*     8     1 */

        /* XXX 3 bytes hole, try to pack */

        __u32                      extended_cpuid_level; /*    12     4 */
        int                        cpuid_level;          /*    16     4 */
        __u32                      x86_capability[19];   /*    20    76 */
        /* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
        char                       x86_vendor_id[16];    /*    96    16 */
        char                       x86_model_id[64];     /*   112    64 */
        /* --- cacheline 2 boundary (128 bytes) was 48 bytes ago --- */
        int                        x86_cache_size;       /*   176     4 */
        int                        x86_cache_alignment;  /*   180     4 */
        int                        x86_cache_max_rmid;   /*   184     4 */
        int                        x86_cache_occ_scale;  /*   188     4 */
        /* --- cacheline 3 boundary (192 bytes) --- */
        int                        x86_power;            /*   192     4 */
        long unsigned int          loops_per_jiffy;      /*   196     4 */
        u16                        x86_max_cores;        /*   200     2 */
        u16                        apicid;               /*   202     2 */
        u16                        initial_apicid;       /*   204     2 */
        u16                        x86_clflush_size;     /*   206     2 */
        u16                        booted_cores;         /*   208     2 */
        u16                        phys_proc_id;         /*   210     2 */
        u16                        logical_proc_id;      /*   212     2 */
        u16                        cpu_core_id;          /*   214     2 */
        u16                        cpu_index;            /*   216     2 */

        /* XXX 2 bytes hole, try to pack */

        u32                        microcode;            /*   220     4 */

        /* size: 224, cachelines: 4, members: 30 */
        /* sum members: 219, holes: 2, sum holes: 5 */
        /* last cacheline: 32 bytes */
};

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Borislav Petkov]

On Sun, Feb 12, 2017 at 10:12:09PM +0100, Mathias Krause wrote:
> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
> correctly. This makes a process reading this file always see "wp : yes"
> here -- otherwise there would be no process to begin with ;)
> 
> As this status line in /proc/cpuinfo serves no purpose for quite some
> time now, get rid of it.

Right, sure, except /proc/cpuinfo's format is kind of an ABI and scripts
rely on it, I'm being told. TBH, I'd remove that wp:-line too but this
is just me. tip guys' call.

FWIW, for all three:

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

[Mathias Krause]

On 14 February 2017 at 17:17, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Feb 12, 2017 at 10:12:07PM +0100, Mathias Krause wrote:
>> Those member serve no purpose -- not even fill padding for alignment or
>> such. So just get rid of them.
>
> Well, almost. You need the wp_works_ok removal patch too, otherwise you
> have the 3 bytes hole below.

Heh, indeed! But only since commit 79a8b9aa388b ("x86/CPU/AMD: Bring
back Compute Unit ID") ;)

> But the wp_works_ok goes away too so I guess that's fine.
>
> $ pahole -C cpuinfo_x86 vmlinux
> struct cpuinfo_x86 {
>         __u8                       x86;                  /*     0     1 */
>         __u8                       x86_vendor;           /*     1     1 */
>         __u8                       x86_model;            /*     2     1 */
>         __u8                       x86_mask;             /*     3     1 */
>         char                       wp_works_ok;          /*     4     1 */
>         __u8                       x86_virt_bits;        /*     5     1 */
>         __u8                       x86_phys_bits;        /*     6     1 */
>         __u8                       x86_coreid_bits;      /*     7     1 */
>         __u8                       cu_id;                /*     8     1 */
>
>         /* XXX 3 bytes hole, try to pack */

The cu_id member is "new". Without it there would be no hole. But,
yeah, without wp_works_ok everything's fine again.

Cheers,
Mathias

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Mathias Krause]

On 14 February 2017 at 17:20, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Feb 12, 2017 at 10:12:09PM +0100, Mathias Krause wrote:
>> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
>> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
>> correctly. This makes a process reading this file always see "wp : yes"
>> here -- otherwise there would be no process to begin with ;)
>>
>> As this status line in /proc/cpuinfo serves no purpose for quite some
>> time now, get rid of it.
>
> Right, sure, except /proc/cpuinfo's format is kind of an ABI and scripts
> rely on it, I'm being told.

That's the reason I haven't folded this change into patch 2. I had
similar doubts but it's not documented in Documentation/ and kinda
useless to test anyway -- what would a "wp : no" tell one?

> TBH, I'd remove that wp:-line too but this
> is just me. tip guys' call.
>
> FWIW, for all three:
>
> Acked-by: Borislav Petkov <bp@suse.de>

Thanks,
Mathias

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Borislav Petkov]

On Tue, Feb 14, 2017 at 05:47:08PM +0100, Mathias Krause wrote:
> That's the reason I haven't folded this change into patch 2. I had
> similar doubts but it's not documented in Documentation/ and kinda
> useless to test anyway -- what would a "wp : no" tell one?

Not that - the missing wp-line in there might puzzle some idiotic
userspace script. And then it is our fault all over again that we broke
the world.

But I'm just playing the devil's advocate here. Realistically, it is
very likely that no one would care.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

[Geert Uytterhoeven]

Hi Boris,

On Tue, Feb 14, 2017 at 5:17 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Feb 12, 2017 at 10:12:07PM +0100, Mathias Krause wrote:
>> Those member serve no purpose -- not even fill padding for alignment or
>> such. So just get rid of them.
>
> Well, almost. You need the wp_works_ok removal patch too, otherwise you
> have the 3 bytes hole below.

That's because you removed a char in commit 93a829e8e2c292f1
("x86, cpu: Convert FDIV bug detection), without compensating with padding ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[H. Peter Anvin]

On 02/12/17 13:12, Mathias Krause wrote:
> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
> correctly. This makes a process reading this file always see "wp : yes"
> here -- otherwise there would be no process to begin with ;)
> 
> As this status line in /proc/cpuinfo serves no purpose for quite some
> time now, get rid of it.
> 
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: H. Peter Anvin <hpa@linux.intel.com>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
>  arch/x86/kernel/cpu/proc.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> index 6df621ae62a7..c6c5217a7980 100644
> --- a/arch/x86/kernel/cpu/proc.c
> +++ b/arch/x86/kernel/cpu/proc.c
> @@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>  		   "coma_bug\t: %s\n"
>  		   "fpu\t\t: %s\n"
>  		   "fpu_exception\t: %s\n"
> -		   "cpuid level\t: %d\n"
> -		   "wp\t\t: yes\n",
> +		   "cpuid level\t: %d\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",
> @@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>  	seq_printf(m,
>  		   "fpu\t\t: yes\n"
>  		   "fpu_exception\t: yes\n"
> -		   "cpuid level\t: %d\n"
> -		   "wp\t\t: yes\n",
> +		   "cpuid level\t: %d\n",
>  		   c->cpuid_level);
>  }
>  #endif
> 

Potential userspace breakage, which is why the line was left in the
first place despite its value now being hard-coded.  Removing it will
save a whopping 9 bytes of kernel rodata; this is a very small price to
pay for guaranteeing continued compatibility.

Nacked-by: H. Peter Anvin <hpa@zytor.com>

Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

[Borislav Petkov]

On Tue, Feb 14, 2017 at 06:56:22PM +0100, Geert Uytterhoeven wrote:
> That's because

No, what Mathias said.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Borislav Petkov]

On Tue, Feb 14, 2017 at 10:13:22AM -0800, H. Peter Anvin wrote:
> Potential userspace breakage, which is why the line was left in the

We could keep the string in /proc/cpuinfo for compatibility but kill the
cpuinfo_x86 members.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

[H. Peter Anvin]

On 02/14/17 09:56, Geert Uytterhoeven wrote:
>>
>> Well, almost. You need the wp_works_ok removal patch too, otherwise you
>> have the 3 bytes hole below.
> 
> That's because you removed a char in commit 93a829e8e2c292f1
> ("x86, cpu: Convert FDIV bug detection), without compensating with padding ;-)
> 

Padding isn't a problem (other than efficiency) for a structure which is
strictly internal to the kernel as opposed to an ABI structure.

	-hpa

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Mathias Krause]

On 14 February 2017 at 19:13, H. Peter Anvin <hpa@zytor.com> wrote:
> On 02/12/17 13:12, Mathias Krause wrote:
>> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
>> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
>> correctly. This makes a process reading this file always see "wp : yes"
>> here -- otherwise there would be no process to begin with ;)
>>
>> As this status line in /proc/cpuinfo serves no purpose for quite some
>> time now, get rid of it.
>>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: H. Peter Anvin <hpa@linux.intel.com>
>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>> ---
>>  arch/x86/kernel/cpu/proc.c |    6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>> index 6df621ae62a7..c6c5217a7980 100644
>> --- a/arch/x86/kernel/cpu/proc.c
>> +++ b/arch/x86/kernel/cpu/proc.c
>> @@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>                  "coma_bug\t: %s\n"
>>                  "fpu\t\t: %s\n"
>>                  "fpu_exception\t: %s\n"
>> -                "cpuid level\t: %d\n"
>> -                "wp\t\t: yes\n",
>> +                "cpuid level\t: %d\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",
>> @@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>       seq_printf(m,
>>                  "fpu\t\t: yes\n"
>>                  "fpu_exception\t: yes\n"
>> -                "cpuid level\t: %d\n"
>> -                "wp\t\t: yes\n",
>> +                "cpuid level\t: %d\n",
>>                  c->cpuid_level);
>>  }
>>  #endif
>>
>
> Potential userspace breakage, which is why the line was left in the
> first place despite its value now being hard-coded.  Removing it will
> save a whopping 9 bytes of kernel rodata; this is a very small price to
> pay for guaranteeing continued compatibility.

Indeed. That's why I've separated the removal into an extra patch --
to make it easier not to take it.

>
> Nacked-by: H. Peter Anvin <hpa@zytor.com>

Do you want me to send the series again without this patch and patch
#6 (Geert took it already) or are you okay with sorting them out
yourself?

Cheers,
Mathias

Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo

[Mathias Krause]

On 14 February 2017 at 22:42, Mathias Krause <minipli@googlemail.com> wrote:
> On 14 February 2017 at 19:13, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 02/12/17 13:12, Mathias Krause wrote:
>>> As of commit a5c2a893dbd4 ("x86, 386 removal: Remove
>>> CONFIG_X86_WP_WORKS_OK") the kernel won't boot if CR0.WP isn't working
>>> correctly. This makes a process reading this file always see "wp : yes"
>>> here -- otherwise there would be no process to begin with ;)
>>>
>>> As this status line in /proc/cpuinfo serves no purpose for quite some
>>> time now, get rid of it.
>>>
>>> Cc: Borislav Petkov <bp@alien8.de>
>>> Cc: H. Peter Anvin <hpa@linux.intel.com>
>>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>>> ---
>>>  arch/x86/kernel/cpu/proc.c |    6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>>> index 6df621ae62a7..c6c5217a7980 100644
>>> --- a/arch/x86/kernel/cpu/proc.c
>>> +++ b/arch/x86/kernel/cpu/proc.c
>>> @@ -30,8 +30,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>>                  "coma_bug\t: %s\n"
>>>                  "fpu\t\t: %s\n"
>>>                  "fpu_exception\t: %s\n"
>>> -                "cpuid level\t: %d\n"
>>> -                "wp\t\t: yes\n",
>>> +                "cpuid level\t: %d\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",
>>> @@ -45,8 +44,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
>>>       seq_printf(m,
>>>                  "fpu\t\t: yes\n"
>>>                  "fpu_exception\t: yes\n"
>>> -                "cpuid level\t: %d\n"
>>> -                "wp\t\t: yes\n",
>>> +                "cpuid level\t: %d\n",
>>>                  c->cpuid_level);
>>>  }
>>>  #endif
>>>
>>
>> Potential userspace breakage, which is why the line was left in the
>> first place despite its value now being hard-coded.  Removing it will
>> save a whopping 9 bytes of kernel rodata; this is a very small price to
>> pay for guaranteeing continued compatibility.
>
> Indeed. That's why I've separated the removal into an extra patch --
> to make it easier not to take it.
>
>>
>> Nacked-by: H. Peter Anvin <hpa@zytor.com>
>
> Do you want me to send the series again without this patch and patch
> #6 (Geert took it already) or are you okay with sorting them out
> yourself?
>

Ping...
Peter, what's your preference here?

Mathias

[tip:x86/cpu] x86/cpu: Drop unneded members of struct cpuinfo_x86

[tip-bot for Mathias Krause]

Commit-ID:  04402116846f36adea9503d7cd5104a7ed27a1a6
Gitweb:     http://git.kernel.org/tip/04402116846f36adea9503d7cd5104a7ed27a1a6
Author:     Mathias Krause <minipli@googlemail.com>
AuthorDate: Sun, 12 Feb 2017 22:12:07 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 11 Mar 2017 14:30:23 +0100

x86/cpu: Drop unneded members of struct cpuinfo_x86

Those member serve no purpose -- not even fill padding for alignment or
such. So just get rid of them.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Mikael Starvik <starvik@axis.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "David S. Miller" <davem@davemloft.net>
Link: http://lkml.kernel.org/r/1486933932-585-2-git-send-email-minipli@googlemail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/include/asm/processor.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index f385eca..893f80e 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -80,7 +80,7 @@ extern u16 __read_mostly tlb_lld_1g[NR_INFO];
 
 /*
  *  CPU type and hardware bug flags. Kept separately for each CPU.
- *  Members of this structure are referenced in head.S, so think twice
+ *  Members of this structure are referenced in head_32.S, so think twice
  *  before touching them. [mj]
  */
 
@@ -91,11 +91,6 @@ struct cpuinfo_x86 {
 	__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;
-	char			pad1;
 #else
 	/* Number of 4K pages in DTLB/ITLB combined(in pages): */
 	int			x86_tlbsize;

[tip:x86/cpu] x86/cpu: Drop wp_works_ok member of struct cpuinfo_x86

[tip-bot for Mathias Krause]

Commit-ID:  6415813bae75feba10b8ca3ed6634a72c2a4d313
Gitweb:     http://git.kernel.org/tip/6415813bae75feba10b8ca3ed6634a72c2a4d313
Author:     Mathias Krause <minipli@googlemail.com>
AuthorDate: Sun, 12 Feb 2017 22:12:08 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 11 Mar 2017 14:30:24 +0100

x86/cpu: Drop wp_works_ok member of struct cpuinfo_x86

Remove the wp_works_ok member of struct cpuinfo_x86. It's an
optimization back from Linux v0.99 times where we had no fixup support
yet and did the CR0.WP test via special code in the page fault handler.
The < 0 test was an optimization to not do the special casing for each
NULL ptr access violation but just for the first one doing the WP test.
Today it serves no real purpose as the test no longer needs special code
in the page fault handler and the only call side -- mem_init() -- calls
it just once, anyway. However, Xen pre-initializes it to 1, to skip the
test.

Doing the test again for Xen should be no issue at all, as even the
commit introducing skipping the test (commit d560bc61575e ("x86, xen:
Suppress WP test on Xen")) mentioned it being ban aid only. And, in
fact, testing the patch on Xen showed nothing breaks.

The pre-fixup times are long gone and with the removal of the fallback
handling code in commit a5c2a893dbd4 ("x86, 386 removal: Remove
CONFIG_X86_WP_WORKS_OK") the kernel requires a working CR0.WP anyway.
So just get rid of the "optimization" and do the test unconditionally.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
Cc: Mikael Starvik <starvik@axis.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "David S. Miller" <davem@davemloft.net>
Link: http://lkml.kernel.org/r/1486933932-585-3-git-send-email-minipli@googlemail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/include/asm/processor.h |  4 +---
 arch/x86/kernel/cpu/proc.c       |  5 ++---
 arch/x86/kernel/setup.c          | 11 ++++-------
 arch/x86/mm/init_32.c            |  9 +++++----
 arch/x86/xen/enlighten.c         |  1 -
 5 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 893f80e..4aa93b5 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -89,9 +89,7 @@ struct cpuinfo_x86 {
 	__u8			x86_vendor;	/* CPU vendor */
 	__u8			x86_model;
 	__u8			x86_mask;
-#ifdef CONFIG_X86_32
-	char			wp_works_ok;	/* It doesn't on 386's */
-#else
+#ifdef CONFIG_X86_64
 	/* Number of 4K pages in DTLB/ITLB combined(in pages): */
 	int			x86_tlbsize;
 #endif
diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index 18ca99f..6df621a 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -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)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 4bf0c89..7cd7bbe 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -173,14 +173,11 @@ 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,
-};
+/* cpu data as detected by the assembly code in head_32.S */
+struct cpuinfo_x86 new_cpu_data;
+
 /* common cpu data for all cpus */
-struct cpuinfo_x86 boot_cpu_data __read_mostly = {
-	.wp_works_ok = -1,
-};
+struct cpuinfo_x86 boot_cpu_data __read_mostly;
 EXPORT_SYMBOL(boot_cpu_data);
 
 unsigned int def_to_bigsmp;
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 2b4b53e..4dddfaf 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -716,15 +716,17 @@ void __init paging_init(void)
  */
 static void __init test_wp_bit(void)
 {
+	int wp_works_ok;
+
 	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);
-	boot_cpu_data.wp_works_ok = do_test_wp_bit();
+	wp_works_ok = do_test_wp_bit();
 	clear_fixmap(FIX_WP_TEST);
 
-	if (!boot_cpu_data.wp_works_ok) {
+	if (!wp_works_ok) {
 		printk(KERN_CONT "No.\n");
 		panic("Linux doesn't support CPUs with broken WP.");
 	} else {
@@ -811,8 +813,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
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index ec1d5c4..bc3dab5 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -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