Re: [PATCH 4/8] efi_loader: Correct bounce-buffer setup

Simon Glass
2025-01-18T04:35:21Z

Hi Ilias,

On Fri, 17 Jan 2025 at 03:34, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:
>
> Hi Simon,
>
> Is this patch for -master of your tree?

Yes, but I haven't applied it yet. The updated bootflow_efi() test ([1]
which this is part of) is throwing up problems with how testing is done. We
cannot today reinit EFI so only get to run one test which does an EFI boot.
I'd like to improve that at some point.

The address/pointer tidy-up was rejected as well, I think by you, but
perhaps Heinrich?

>
> On Mon, 6 Jan 2025 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> >
> > This should set the bounce buffer to a pointer, not an address. Fix it.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > lib/efi_loader/efi_memory.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 2e197e94edd..9374f77d3b7 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -837,11 +837,11 @@ int efi_memory_init(void)
> > uint64_t efi_bounce_buffer_addr = 0xffffffff;
> >
> > if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
EFI_BOOT_SERVICES_DATA,
> > - (64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
> > + SZ_64M >> EFI_PAGE_SHIFT,
> > &efi_bounce_buffer_addr) != EFI_SUCCESS)
> > return -1;
> >
> > - efi_bounce_buffer = (void*)(uintptr_t)efi_bounce_buffer_addr;
> > + efi_bounce_buffer = map_sysmem(efi_bounce_buffer_addr, SZ_64M);
>
> map_sysmem does *exactly* the same thing so you need to adjust your
> commit message.
>
> Does this solve any sandbox crashes?

No, sandbox doesn't test this feature yet, but the new code is more
correct. Whenever you see an address, use map_sysmem() to turn it into a
pointer.

>
> /Ilias
> > #endif
> >
> > return 0;
> > --
> > 2.34.1
> >

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=439021
[2] https://patchwork.ozlabs.org/project/uboot/list/?series=436241