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) >>> >>> 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 : >>>         0:    e8 00 00 00 00           callq  5 >>> >>>             1: R_X86_64_PLT32    cpu_bringup_and_idle-0x4 >>>         5:    e9 f6 0f 00 00           jmpq   1000 >>> >>>         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 >> 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 > Reviewed-by: Juergen Gross 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 >>   #include >> +.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 >> +#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 >> -#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") >> >