Re: [PATCH 2/5] x86: Fix early output messages in case of EFI

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Thu, Aug 8, 2024 at 1:58 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 08.08.2024 14:50, Frediano Ziglio wrote:
> > On Thu, Aug 8, 2024 at 10:29 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>
> >> (re-adding xen-devel@)
>
> Did you notice this in my earlier reply? You dropped the list again.
>

Yes, later, sorry for that.

> >> On 08.08.2024 10:33, Frediano Ziglio wrote:
> >>> On Thu, Aug 8, 2024 at 8:49 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>>>> 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
> >>>>
> >>>
> >>> Which is correct
> >>>
> >>>> 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?
> >>>>
> >>>
> >>> This can be wrong if sym_offs(1b) was relocated and not patched by
> >>> efi_arch_relocate_image.
> >>
> >> Of course, if in the course of GrUB's loading of xen.efi base relocations
> >> are applied (unlike when loading an ELF binary, where afaik base relocs
> >> would be ignored, even if there were any), then this calculation is of
> >> course going to be wrong. Can't we correct it though, to properly resemble
> >> PIC code:
> >>
> >>         /* Calculate the load base address. */
> >>         call    1f
> >> 1:      pop     %esi
> >>         sub     1b - start, %esi
> >>
> >> or (because start is in a different section):
> >>
> >>         /* Calculate the load base address. */
> >>         call    1f
> >> 1:      pop     %esi
> >>         sub     $sym_offs(1b), %esi
> >>         add     $sym_offs(start), %esi
> >>
> >> (or something along these lines)?
> >>
> >
> > Yes, that works. But is a bit painfull, I mean, the %esi will point to
> > the correct address, but still you will use something like
> > syms_esi(foo) expecting to work but it won't as there will be applied
> > a relocation offset.
>
> I find your reply contradictory in itself. You first say this works, to
> then say it can't work. The underlying idea has to be to establish %esi
> such that it works uniformly.
>

The computation of %esi is correct after the additional "add" command,
in the sense it will point to the current base (under 4GB) however
then you will use syms_esi(foo) thinking "if %esi is correct then also
syms_esi is correct" and it isn't.
So either you need to add another offset to make syms_esi(foo) correct
having %esi not pointing to the base or assuming that syms_esi(foo)
would need fixing.
Potentially the first option would be better, you just need to
remember to correct %esi after rolling back relocations.

> > On 32bit PIC code you could use something like
> > foo@GOTOFF(%esi), assuing %esi is pointing to the global offset table.
> > I was trying to use that but linker is complaining a bit as generating
> > a 64bit relocation. The x64 architecture supports such relocation as
> > 32bit but I didn't find a way to tell assembler to use the 32bit
> > version instead of the 64bit one. Also I didn't find a way to set
> > _GLOBAL_OFFSET_TABLE_ where I want it to be, it looks like that if the
> > linker is not generating it is not picking up the forcedly set symbol.
>
> Even if the toolchain permitted this: We don't have and don't want to
> have any GOT. Note how the linker script actually has an assertion for
> .got to be empty (plus a few more ones for other sections).
>

I know, @GOTOFF does not generate .got entries, that's the reason I
tried to use it and tell linker my idea of _GLOBAL_OFFSET_TABLE_. It's
just that we need real offsets without relocations and I was looking
at a way to get it. That would simply solve the relocation issue
without having to offset %esi to some weird value.

> Jan

Frediano