Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1579281 > unrolled thread

[PATCH 0/6] struct cpuinfo_x86 related cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] struct cpuinfo_x86 related cleanups Mathias Krause <minipli@googlemail.com> - 2017-02-12 22:20 +0100
    [PATCH 5/6] cris: remove unused wp_works_ok macro Mathias Krause <minipli@googlemail.com> - 2017-02-12 22:20 +0100
      Re: [PATCH 5/6] cris: remove unused wp_works_ok macro Jesper Nilsson <jesper.nilsson@axis.com> - 2017-02-13 10:20 +0100
    [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Mathias Krause <minipli@googlemail.com> - 2017-02-12 22:20 +0100
      Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Borislav Petkov <bp@alien8.de> - 2017-02-14 17:20 +0100
        Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Mathias Krause <minipli@googlemail.com> - 2017-02-14 17:50 +0100
        Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-14 19:00 +0100
          Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Borislav Petkov <bp@alien8.de> - 2017-02-14 19:30 +0100
          Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 "H. Peter Anvin" <hpa@zytor.com> - 2017-02-14 19:50 +0100
    [PATCH 4/6] sparc: remove unused wp_works_ok macro Mathias Krause <minipli@googlemail.com> - 2017-02-12 22:20 +0100
      Re: [PATCH 4/6] sparc: remove unused wp_works_ok macro David Miller <davem@davemloft.net> - 2017-02-13 03:50 +0100
    [PATCH 2/6] x86/cpu: drop wp_works_ok member of struct cpuinfo_x86 Mathias Krause <minipli@googlemail.com> - 2017-02-12 22:20 +0100
    [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Mathias Krause <minipli@googlemail.com> - 2017-02-12 22:20 +0100
      Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Borislav Petkov <bp@alien8.de> - 2017-02-14 17:30 +0100
        Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Mathias Krause <minipli@googlemail.com> - 2017-02-14 17:50 +0100
          Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Borislav Petkov <bp@alien8.de> - 2017-02-14 18:20 +0100
      Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo "H. Peter Anvin" <hpa@zytor.com> - 2017-02-14 19:20 +0100
        Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Borislav Petkov <bp@alien8.de> - 2017-02-14 19:40 +0100
        Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Mathias Krause <minipli@googlemail.com> - 2017-02-14 22:50 +0100
          Re: [PATCH 3/6] x86/cpu: proc - remove "wp" status line in cpuinfo Mathias Krause <minipli@googlemail.com> - 2017-02-28 09:40 +0100

#1579281 — [PATCH 0/6] struct cpuinfo_x86 related cleanups

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1579283 — [PATCH 1/6] x86: 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.

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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>

[toc] | [prev] | [next] | [standalone]

#1579286 — [PATCH 2/6] 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.

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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>

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web