[1/6] x86: drop unneded members of struct cpuinfo_x86
diff mbox series

Message ID 1486933932-585-2-git-send-email-minipli@googlemail.com
State New, archived
Headers show
Series
  • struct cpuinfo_x86 related cleanups
Related show

Commit Message

Mathias Krause Feb. 12, 2017, 9:12 p.m. UTC
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(-)

Comments

Borislav Petkov Feb. 14, 2017, 4:17 p.m. UTC | #1
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 */
};
Mathias Krause Feb. 14, 2017, 4:40 p.m. UTC | #2
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
Geert Uytterhoeven Feb. 14, 2017, 5:56 p.m. UTC | #3
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
Borislav Petkov Feb. 14, 2017, 6:21 p.m. UTC | #4
On Tue, Feb 14, 2017 at 06:56:22PM +0100, Geert Uytterhoeven wrote:
> That's because

No, what Mathias said.
H. Peter Anvin Feb. 14, 2017, 6:46 p.m. UTC | #5
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

Patch
diff mbox series

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;