Re: [PATCH] x86/wakeup: Fix code generation for bogus_saved_magic

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Sat, Nov 9, 2024 at 12:37 AM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> bogus_saved_magic() is in a .code64 section but invokved in 32bit mode.  This

Typo: invoked

> causes a real encoding difference.
>
> Before:
>   66 c7 04 25 14 80 0b 00 53 0e    movw   $0xe53,0xb8014(,%eiz,1)
>
> After:
>   66 c7 05 14 80 0b 00 53 0e       movw   $0xe53,0xb8014
>
> The differnce happens to be benign, but move the logic back into a .code32 for

Typo: difference

> sanity sake.  Annotate it with ELF metadata while doing so.
>
> Fixes: d8c8fef09054 ("Provide basic Xen PM infrastructure")
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> This issue dates back to the very introduction of S3 support in Xen, in 2007.
> ---
>  xen/arch/x86/boot/wakeup.S | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/boot/wakeup.S b/xen/arch/x86/boot/wakeup.S
> index 08447e193496..c929fe921823 100644
> --- a/xen/arch/x86/boot/wakeup.S
> +++ b/xen/arch/x86/boot/wakeup.S
> @@ -153,15 +153,16 @@ wakeup_32:
>          /* Now in compatibility mode. Long-jump to 64-bit mode */
>          ljmp    $BOOT_CS64, $bootsym_rel(wakeup_64,6)
>
> +FUNC_LOCAL(bogus_saved_magic, 0)
> +        movw    $0x0e00 + 'S', 0xb8014
> +        jmp     bogus_saved_magic
> +END(bogus_saved_magic)
> +
>          .code64
>  wakeup_64:
>          /* Jump to high mappings and the higher-level wakeup code. */
>          movabs  $s3_resume, %rbx
>          jmp     *%rbx
>
> -bogus_saved_magic:
> -        movw    $0x0e00 + 'S', 0xb8014
> -        jmp     bogus_saved_magic
> -
>  /* Stack for wakeup: rest of first trampoline page. */
>  ENTRY(wakeup_stack_start)

Hi,
   I agree with the code move, it's supposed to be 32 bit so it should
be in the 32 bit section.
Does the ELF annotation help with debug information? Maybe worth
adding to the comment.

OT: If I understood correctly, the code is writing a character on
screen in a tight loop. Maybe an hlt instruction could be helpful?

BTW

Reviewed-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>

Frediano