Messages in this thread | | | Subject | Re: [PATCH v2 01/24] x86/xen: Mark cpu_bringup_and_idle() as dead_end_function | From | Juergen Gross <> | Date | Mon, 30 Aug 2021 07:55:02 +0200 |
| |
On 23.08.21 10:40, Juergen Gross wrote: > On 20.08.21 21:31, Josh Poimboeuf wrote: >> On Fri, Aug 20, 2021 at 12:22:28PM -0700, Josh Poimboeuf wrote: >>> On Thu, Jun 24, 2021 at 11:41:00AM +0200, Peter Zijlstra wrote: >>>> The asm_cpu_bringup_and_idle() function is required to push the return >>>> value on the stack in order to make ORC happy, but the only reason >>>> objtool doesn't complain is because of a happy accident. >>>> >>>> The thing is that asm_cpu_bringup_and_idle() doesn't return, so >>>> validate_branch() never terminates and falls through to the next >>>> function, which in the normal case is the hypercall_page. And that, as >>>> it happens, is 4095 NOPs and a RET. >>>> >>>> Make asm_cpu_bringup_and_idle() terminate on it's own, by making the >>>> function it calls as a dead-end. This way we no longer rely on what >>>> code happens to come after. >>>> >>>> Fixes: c3881eb58d56 ("x86/xen: Make the secondary CPU idle tasks >>>> reliable") >>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> >>> >>> Looks right. Only problem is, with my assembler I get this: >>> >>> arch/x86/kernel/head_64.o: warning: objtool: .text+0x5: >>> unreachable instruction >>> >>> Because gas insists on jumping over the page of nops... >>> >>> 0000000000000000 <asm_cpu_bringup_and_idle>: >>> 0: e8 00 00 00 00 callq 5 >>> <asm_cpu_bringup_and_idle+0x5> >>> 1: R_X86_64_PLT32 cpu_bringup_and_idle-0x4 >>> 5: e9 f6 0f 00 00 jmpq 1000 >>> <xen_hypercall_set_trap_table> >>> a: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) >>> 11: 00 00 00 00 >>> 15: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) >>> 1c: 00 00 00 00 >>> 20: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) >>> 27: 00 00 00 00 >>> 2b: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) >>> 32: 00 00 00 00 >>> 36: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) >>> 3d: 00 00 00 00 >> >> Here's a fix: >> >> From: Josh Poimboeuf <jpoimboe@redhat.com> >> Subject: [PATCH] x86/xen: Move hypercall_page to top of the file >> >> Because hypercall_page is page-aligned, the assembler inexplicably adds >> an unreachable jump from after the end of the previous code to the >> beginning of hypercall_page. >> >> That confuses objtool, understandably. It also creates significant text >> fragmentation. As a result, much of the object file is wasted text >> (nops). >> >> Move hypercall_page to the beginning of the file to both prevent the >> text fragmentation and avoid the dead jump instruction. >> >> $ size /tmp/head_64.before.o /tmp/head_64.after.o >> text data bss dec hex filename >> 10924 307252 4096 322272 4eae0 >> /tmp/head_64.before.o >> 6823 307252 4096 318171 4dadb >> /tmp/head_64.after.o >> >> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com > > Reviewed-by: Juergen Gross <jgross@suse.com>
Umm, will this be carried through the tip tree, or shall I take it in the xen tree?
Juergen
> > > Juergen > >> --- >> arch/x86/xen/xen-head.S | 34 +++++++++++++++++----------------- >> 1 file changed, 17 insertions(+), 17 deletions(-) >> >> diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S >> index cb6538ae2fe0..488944d6d430 100644 >> --- a/arch/x86/xen/xen-head.S >> +++ b/arch/x86/xen/xen-head.S >> @@ -20,6 +20,23 @@ >> #include <xen/interface/xen-mca.h> >> #include <asm/xen/interface.h> >> +.pushsection .text >> + .balign PAGE_SIZE >> +SYM_CODE_START(hypercall_page) >> + .rept (PAGE_SIZE / 32) >> + UNWIND_HINT_FUNC >> + .skip 31, 0x90 >> + ret >> + .endr >> + >> +#define HYPERCALL(n) \ >> + .equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \ >> + .type xen_hypercall_##n, @function; .size xen_hypercall_##n, 32 >> +#include <asm/xen-hypercalls.h> >> +#undef HYPERCALL >> +SYM_CODE_END(hypercall_page) >> +.popsection >> + >> #ifdef CONFIG_XEN_PV >> __INIT >> SYM_CODE_START(startup_xen) >> @@ -64,23 +81,6 @@ SYM_CODE_END(asm_cpu_bringup_and_idle) >> #endif >> #endif >> -.pushsection .text >> - .balign PAGE_SIZE >> -SYM_CODE_START(hypercall_page) >> - .rept (PAGE_SIZE / 32) >> - UNWIND_HINT_FUNC >> - .skip 31, 0x90 >> - ret >> - .endr >> - >> -#define HYPERCALL(n) \ >> - .equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \ >> - .type xen_hypercall_##n, @function; .size xen_hypercall_##n, 32 >> -#include <asm/xen-hypercalls.h> >> -#undef HYPERCALL >> -SYM_CODE_END(hypercall_page) >> -.popsection >> - >> ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS, .asciz "linux") >> ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION, .asciz "2.6") >> ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION, .asciz "xen-3.0") >> >
[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |