If code is loaded by EFI the loader will relocate the image
under 4GB. This cause offsets in x86 code generated by
sym_offs(SYMBOL) to be relocated too (basically they won't be
offsets from image base). In order to get real offset the
formulae "sym_offs(SYMBOL) - sym_offs(__image_base__)" is
used instead.
Also, in some case %esi register (that should point to
__image_base__ addresss) is not set so compute in all cases.
Code tested forcing failures in the code.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/head.S | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index f027ff45fd..296f76146a 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -188,8 +188,27 @@ early_error: /* Here to improve the disassembly. */
         xor     %edi,%edi                       # No VGA text buffer
         jmp     .Lprint_err
 .Lget_vtb:
-        mov     sym_esi(vga_text_buffer), %edi
+        mov     $sym_offs(vga_text_buffer), %edi
 .Lprint_err:
+        mov     $sym_offs(__image_base__), %ebx
+
+        /* compute base, relocation or not */
+        call    1f
+1:
+        pop     %esi
+        subl    $sym_offs(1b), %esi
+        addl    %ebx, %esi
+
+        /* adjust offset and load */
+        test    %edi, %edi
+        jz      1f
+        subl    %ebx, %edi
+        movl    (%edi,%esi,1), %edi
+1:
+
+        /* adjust message offset */
+        subl    %ebx, %ecx
+
         add     %ecx, %esi     # Add string offset to relocation base.
         # NOTE: No further use of sym_esi() till the end of the "function"!
 1:
-- 
2.45.2

On 07.08.2024 15:48, Alejandro Vallejo wrote:
> If code is loaded by EFI the loader will relocate the image
> under 4GB.

This is the MB2 EFI path you're talking about? Since there are two paths,
I think this needs clearly separating in all descriptions.

If it is the MB2 path, then "relocate" isn't quite correct, I think:
Relocations aren't applied in that case, as none are present in xen.gz.
I'd rather call this "put at an address below 4G". However, that isn't
any different from the non-EFI MB1/2 paths, is it? I feel like I'm
missing something here.

> This cause offsets in x86 code generated by
> sym_offs(SYMBOL) to be relocated too (basically they won't be
> offsets from image base). In order to get real offset the
> formulae "sym_offs(SYMBOL) - sym_offs(__image_base__)" is
> used instead.

The main calculations of %esi are, if I'm not mistaken,

        /* Store Xen image load base address in place accessible for 32-bit code. */
        lea     __image_base__(%rip),%esi

and

        /* Calculate the load base address. */
        call    1f
1:      pop     %esi
        sub     $sym_offs(1b), %esi

i.e. both deliberately %rip-relative to be position-independent. What's
wrong with this?

There are many more uses of sym_esi(). Why is it only this single one
which poses a problem?

> Also, in some case %esi register (that should point to
> __image_base__ addresss) is not set so compute in all cases.

Which "some case" is this?

> Code tested forcing failures in the code.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>

No Fixes: tag?

Jan