Re: [PATCH v2 09/11] x86/efi: avoid a relocation in efi_arch_post_exit_boot()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.04.2025 15:08, Roger Pau Monne wrote:
> Instead of using the absolute __start_xen address, calculate it as an
> offset from the current instruction pointer.  The relocation would be
> problematic if the loader has acknowledged the Xen image section
> attributes, and mapped .init.text with just read and execute permissions.

Fine in principle, but ...

> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -266,7 +266,9 @@ static void __init noreturn efi_arch_post_exit_boot(void)
>  
>                     /* Jump to higher mappings. */
>                     "mov    stack_start(%%rip), %%rsp\n\t"
> -                   "movabs $__start_xen, %[rip]\n\t"
> +                   "lea    __start_xen(%%rip), %[rip]\n\t"
> +                   "add    %[offset], %[rip]\n\t"
> +
>                     "push   %[cs]\n\t"
>                     "push   %[rip]\n\t"
>                     "lretq"
> @@ -274,7 +276,8 @@ static void __init noreturn efi_arch_post_exit_boot(void)
>                       [cr4] "+&r" (cr4)
>                     : [cr3] "r" (idle_pg_table),
>                       [cs] "i" (__HYPERVISOR_CS),
> -                     [ds] "r" (__HYPERVISOR_DS)
> +                     [ds] "r" (__HYPERVISOR_DS),
> +                     [offset] "r" (__XEN_VIRT_START - xen_phys_start)
>                     : "memory" );
>      unreachable();
>  }

... imo ought to come with a brief comment, to keep people from trying to
undo ("simplify") that again.

[offset]'s constraint could in principle be "rme", I think, as [rip] is
"&r" already. Just that the compiler (at least gcc) won't synthesize a
memory operand, and the value can't be expressed by an immediate. IOW -
probably all fine with just "r". Of course if/when we add further operands
here, we need to pay attention to the number of registers needed.

Jan