[PATCH v2 3/3] efi: Show the location of the bounce buffer

[Simon Glass]

Hi Heinrich,

On Sun, 11 Aug 2024 at 15:50, Simon Glass <sjg at chromium.org> wrote:
>
> +Marek Vasut too
>
> Hi Heinrich,
>
> On Fri, 9 Aug 2024 at 13:02, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> >
> >
> > Am 9. August 2024 20:36:35 MESZ schrieb Simon Glass <sjg at chromium.org>:
> > >Hi Heinrich,
> > >
> > >On Fri, 9 Aug 2024 at 11:32, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >>
> > >> On 01.08.24 19:36, Simon Glass wrote:
> > >> > The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is not
> > >> > clear whether it is still needed, but 24 boards (lx2160ardb_tfa_stmm,
> > >> > lx2162aqds_tfa_SECURE_BOOT and the like) use it.
> > >> >
> > >> > This feature uses EFI page allocation to create a 64MB buffer 'in space'
> > >> > without any knowledge of where boards intend to load their images. This
> > >> > may result in image corruption or other problems.
> > >> >
> > >> > For example, if the feature is enabled on qemu_arm64 it puts the EFI
> > >> > bounce buffer at 1045MB, with the kernel at 1028MB and the ramdisk at
> > >> > 1088MB. The kernel is probably smaller than 27MB but the buffer does
> > >> > overlap the ramdisk.
> > >> >
> > >> > The solution is probably to use BOUNCE_BUFFER instead, with the EFI
> > >> > version being dropped. For now, show the address of the EFI bounce
> > >> > buffer so people have a better chance to detect the problem.
> > >> >
> > >> > Note: I avoided converting this #ifdef to use IS_ENABLED() since I hope
> > >> > that the feature may be removed.
> > >>
> > >> EFI_BLOCK_IO_PROTOCOL.ReadBlocks() and
> > >> EFI_BLOCK_IO_PROTOCOL.WriteBlocks() are expected to block until all
> > >> bytes are transferred.
> > >>
> > >> The complete file transfer is chunked according to the size of the EFI
> > >> bounce buffer.
> > >>
> > >> Taking care of alignment issues would best handled by the block uclass.
> > >>
> > >> When CONFIG_BOUNCE_BUFFER=y, bounce_buffer_start_extalign() uses
> > >> memalign() which limits the bounce buffer to available malloc() memory
> > >> (typically < 2 MiB).
> > >>
> > >> I cannot see how blk_read() with CONFIG_BOUNCE_BUFFER=y will work if
> > >> blkcnt * desc->blksz is greater than the available malloc memory.
> > >
> > >Yes, it uses malloc(). The maximum read length on MMC, for example,
> > >seems to default to 32MB, although it is only 128KB on a few
> > >platforms.
> > >
> > >I am thinking we should be allocating space in the bloblist to hold
> > >all these sorts of things...it can be as large as needed and we can
> > >allocate space for it during relocation.
> > >
> > >>
> > >> Shouldn't the block uclass support chunking?
> > >
> > >It only supports readling whole blocks, if that is what you mean. Are
> > >you suggesting changing the blk API, or something else?
> >
> > It supports reading multiple blocks. If not all blocks fit into the memory that can be assigned via memalgn, the block layer coud separate the blocks into chunks. This would replace the EFI bounce buffer.
>
> Ah OK, yes it could do that.

I am still not sure what changes are needed with this patch. I cannot
use malloc() for the bounce buffer, so for now (until we improve
things) I would like to print this message as a warning to fsl users.

So please can you provide a clear response to the above?

Regards,
Simon