Hi Heinrich,

On Wed, 30 Apr 2025 at 09:17, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
>
> On 30.04.25 17:01, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 30 Apr 2025 at 08:04, Heinrich Schuchardt
> > <heinrich.schucha...@canonical.com> wrote:
> >>
> >> On 30.04.25 15:54, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Wed, 30 Apr 2025 at 04:55, Heinrich Schuchardt
> >>> <heinrich.schucha...@canonical.com> wrote:
> >>>>
> >>>> The EFI sub-system needs the load address and not the entry point
> >>>> to boot the binary passed from the bootm command. The entry point
> >>>> is derived from the PE-COFF header of the binary.
> >>>>
> >>>> Fixes: ecc7fdaa9ef1 ("bootm: Add a bootm command for type IH_OS_EFI")
> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> >>>> ---
> >>>>    boot/bootm_os.c | 4 ++--
> >>>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/boot/bootm_os.c b/boot/bootm_os.c
> >>>> index dc9d3e61fca..a3c7cb5332e 100644
> >>>> --- a/boot/bootm_os.c
> >>>> +++ b/boot/bootm_os.c
> >>>> @@ -498,11 +498,11 @@ static int do_bootm_efi(int flag, struct 
> >>>> bootm_info *bmi)
> >>>>           /* We expect to return */
> >>>>           images->os.type = IH_TYPE_STANDALONE;
> >>>>
> >>>> -       image_buf = map_sysmem(images->ep, images->os.image_len);
> >>>> +       image_buf = map_sysmem(images->os.image_start, 
> >>>> images->os.image_len);
> >>>>
> >>>>           /* Run EFI image */
> >>>>           printf("## Transferring control to EFI (at address %08lx) 
> >>>> ...\n",
> >>>> -              images->ep);
> >>>> +              images->os.image_start);
> >>>>           bootstage_mark(BOOTSTAGE_ID_RUN_OS);
> >>>>
> >>>>           ret = efi_binary_run(image_buf, images->os.image_len,
> >>>
> >>> In this case ep should be set to image_start as it is with many
> >>> methods. How come it isn't in this case?
> >>
> >> As described above the entry point is encoded in the PE-COFF header.
> >>
> >> What makes you think that a user would set it in an its file containing
> >> an EFI binary?
> >>
> >> If it where set, it would point to an address after the load address and
> >> not to the start of the PE-COFF header.
> >>
> >> The proper design would completely remove load and entry addresses in
> >> FIT images and use LMB for memory allocation.
> >
> > You are missing the point entirely.
> >
> > However the image calculates the entry point is up to that image. But
> > the correct entry point should be put into ->ep
> >
> > We can't have some OS--booting functions using ->ep and some using ->os.load
> >
> > So please dig in a little more and fix whatever is going wrong here
> > and ensure that ->ep is set correctly. If you need help, please ask.
>
> The only information that the EFI sub-system cares about is the start of
> the image. That information is in os.start_image.
>
> Field ep is for the entry point and not for the start of the image and
> EFI should never take it from the FIT meta-information.
>
> This is why whatever value I put into the field entry of my handcrafted
> its file is completely irrelevant.

As discussed, we should split efi_binary_run() so that we can do the
loading. For now:

Reviewed-by: Simon Glass <s...@chromium.org>

Reply via email to