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
