| Message ID | 20190808103854.6192-3-jslaby@suse.cz (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v8,01/28] linkage: new macros for assembler symbols | expand |
On Thu, Aug 08, 2019 at 12:38:28PM +0200, Jiri Slaby wrote: > Some global data in the suspend code were marked as `ENTRY'. ENTRY was > intended for functions and shall be paired with ENDPROC. ENTRY also > aligns symbols which creates unnecessary holes here between data. Well, but are we sure that removing that alignment is fine in all cases? If so, the commit message should state why it is ok for those symbols to not be aligned now. And before it, phys_base looks like this: .globl phys_base ; .p2align 4, 0x90 ; phys_base: and it is correctly 8 bytes: ffffffff82011010 <phys_base>: ffffffff82011010: 00 00 add %al,(%rax) ffffffff82011012: 00 00 add %al,(%rax) ffffffff82011014: 00 00 add %al,(%rax) ffffffff82011016: 00 00 add %al,(%rax) However, with this patch, it becomes: .globl phys_base ; ; phys_base: ; .quad 0x0000000000000000 ; .type phys_base STT_OBJECT ; .size phys_base, .-phys_base which is better as now we'll have the proper symbol size: 86679: ffffffff8200f00a 8 OBJECT GLOBAL DEFAULT 11 phys_base but in the vmlinux image it is 14(!) bytes: ... ffffffff8200f002 <early_gdt_descr_base>: ffffffff8200f002: 00 a0 3b 82 ff ff add %ah,-0x7dc5(%rax) ffffffff8200f008: ff (bad) ffffffff8200f009: ff incl (%rax) ffffffff8200f00a <phys_base>: ffffffff8200f00a: 00 00 add %al,(%rax) ffffffff8200f00c: 00 00 add %al,(%rax) ffffffff8200f00e: 00 00 add %al,(%rax) ffffffff8200f010: 00 00 add %al,(%rax) <--- should end here but the toolchain "stretches" it for whatever reason. Perhaps padding? ffffffff8200f012: 00 00 add %al,(%rax) ffffffff8200f014: 00 00 add %al,(%rax) ffffffff8200f016: 00 00 add %al,(%rax) ffffffff8200f018 <early_pmd_flags>: ffffffff8200f018: e3 00 jrcxz ffffffff8200f01a <early_pmd_flags+0x2> ... And that looks strange... > Since we are dropping historical markings, make proper use of newly added > SYM_DATA in this code. > > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Acked-by: Pavel Machek <pavel@ucw.cz> > Cc: Len Brown <len.brown@intel.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: x86@kernel.org > Cc: linux-pm@vger.kernel.org > --- > arch/x86/kernel/acpi/wakeup_32.S | 2 +- > arch/x86/kernel/acpi/wakeup_64.S | 2 +- > arch/x86/kernel/head_32.S | 6 ++---- > arch/x86/kernel/head_64.S | 5 ++--- > 4 files changed, 6 insertions(+), 9 deletions(-) ... > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S > index f3d3e9646a99..6c1bf7ae55ff 100644 > --- a/arch/x86/kernel/head_64.S > +++ b/arch/x86/kernel/head_64.S > @@ -469,9 +469,8 @@ early_gdt_descr: > early_gdt_descr_base: > .quad INIT_PER_CPU_VAR(gdt_page) > > -ENTRY(phys_base) > - /* This must match the first entry in level2_kernel_pgt */ > - .quad 0x0000000000000000 > +/* This must match the first entry in level2_kernel_pgt */ > +SYM_DATA(phys_base, .quad 0x0000000000000000) You can write it SYM_DATA(phys_base, .quad 0x0) while at it. That string of 16 zeroes is unreadable and not needed. Thx.
diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S index e95e95960156..427249292aef 100644 --- a/arch/x86/kernel/acpi/wakeup_32.S +++ b/arch/x86/kernel/acpi/wakeup_32.S @@ -90,7 +90,7 @@ ret_point: .data ALIGN -ENTRY(saved_magic) .long 0 +SYM_DATA(saved_magic, .long 0) saved_eip: .long 0 # saved registers diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S index b0715c3ac18d..a142c5ee0d4f 100644 --- a/arch/x86/kernel/acpi/wakeup_64.S +++ b/arch/x86/kernel/acpi/wakeup_64.S @@ -134,4 +134,4 @@ saved_rbx: .quad 0 saved_rip: .quad 0 saved_rsp: .quad 0 -ENTRY(saved_magic) .quad 0 +SYM_DATA(saved_magic, .quad 0) diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S index 30f9cb2c0b55..d1e213da4782 100644 --- a/arch/x86/kernel/head_32.S +++ b/arch/x86/kernel/head_32.S @@ -507,10 +507,8 @@ GLOBAL(early_recursion_flag) __REFDATA .align 4 -ENTRY(initial_code) - .long i386_start_kernel -ENTRY(setup_once_ref) - .long setup_once +SYM_DATA(initial_code, .long i386_start_kernel) +SYM_DATA(setup_once_ref, .long setup_once) #ifdef CONFIG_PAGE_TABLE_ISOLATION #define PGD_ALIGN (2 * PAGE_SIZE) diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index f3d3e9646a99..6c1bf7ae55ff 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -469,9 +469,8 @@ early_gdt_descr: early_gdt_descr_base: .quad INIT_PER_CPU_VAR(gdt_page) -ENTRY(phys_base) - /* This must match the first entry in level2_kernel_pgt */ - .quad 0x0000000000000000 +/* This must match the first entry in level2_kernel_pgt */ +SYM_DATA(phys_base, .quad 0x0000000000000000) EXPORT_SYMBOL(phys_base) #include "../../x86/xen/xen-head.S"