Hi Simon,
> [please can you avoid top-posting as it makes it had for people to read later] > Sure thing. > There is a pending series there which I haven't got to yet. > I can add that change to my bootmeth_efi dhcp fixes patch series if you want. > BTW, for this patch we need a test which covers EFI on sandbox, > perhaps running the helloworld program. The current bootstd testing > does not extend into the efi_loader code, so there is really no > coverage of the code in efi_exit_boot_services() from bootstd. I am not sure how sandbox works. I was able to test my setup which was crashing and was fixed after these changes. Regard, Shantur > > Regards, > Simon > > > > > > Kind regards, > > Shantur > > > > On Thu, Nov 16, 2023 at 1:35 AM Simon Glass <s...@chromium.org> wrote: > > > > > > EFI applications can be very large and thus used to cause boot failures > > > when malloc() space was exhausted. > > > > > > A recent changed fixed this by using the kernel_addr_r environment var > > > as the address of the buffer. However, it still frees the buffer when > > > the bootflow is discarded. > > > > > > Fix this by introducing a flag to indicate whether the buffer was > > > allocated, or not. > > > > > > Note that kernel_addr_r is not the last word here. It might be better > > > to use lmb to place images. But there is a lot of refactoring to do > > > before we can remove the environment variables. The distro scripts rely > > > on them so it is safe for bootstd to do so too. > > > > > > Fixes: 6a8c2f9781c bootstd: Avoid allocating memory for the EFI file > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > Reported by: Simon Glass <s...@chromium.org> > > > Reported by: Shantur Rathore <i...@shantur.com> > > > --- > > > > > > boot/bootflow.c | 3 ++- > > > boot/bootmeth_efi.c | 1 + > > > include/bootflow.h | 5 ++++- > > > 3 files changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/boot/bootflow.c b/boot/bootflow.c > > > index 6922e7e0c4e7..1ea2966ae9a5 100644 > > > --- a/boot/bootflow.c > > > +++ b/boot/bootflow.c > > > @@ -467,7 +467,8 @@ void bootflow_free(struct bootflow *bflow) > > > free(bflow->name); > > > free(bflow->subdir); > > > free(bflow->fname); > > > - free(bflow->buf); > > > + if (!(bflow->flags & BOOTFLOWF_STATIC_BUF)) > > > + free(bflow->buf); > > > free(bflow->os_name); > > > free(bflow->fdt_fname); > > > free(bflow->bootmeth_priv); > > > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c > > > index ae936c8daa18..9ba7734911e1 100644 > > > --- a/boot/bootmeth_efi.c > > > +++ b/boot/bootmeth_efi.c > > > @@ -160,6 +160,7 @@ static int efiload_read_file(struct bootflow *bflow, > > > ulong addr) > > > if (ret) > > > return log_msg_ret("read", ret); > > > bflow->buf = map_sysmem(addr, bflow->size); > > > + bflow->flags |= BOOTFLOWF_STATIC_BUF; > > > > > > set_efi_bootdev(desc, bflow); > > > > > > diff --git a/include/bootflow.h b/include/bootflow.h > > > index 44d3741eacae..fede8f22a2b8 100644 > > > --- a/include/bootflow.h > > > +++ b/include/bootflow.h > > > @@ -43,9 +43,12 @@ enum bootflow_state_t { > > > * and it is using the prior-stage FDT, which is the U-Boot control > > > FDT. > > > * This is only possible with the EFI bootmeth (distro-efi) and only > > > when > > > * CONFIG_OF_HAS_PRIOR_STAGE is enabled > > > + * @BOOTFLOWF_STATIC_BUF: Indicates that @bflow->buf is statically set, > > > rather > > > + * than being allocated by malloc(). > > > */ > > > enum bootflow_flags_t { > > > BOOTFLOWF_USE_PRIOR_FDT = 1 << 0, > > > + BOOTFLOWF_STATIC_BUF = 1 << 1, > > > }; > > > > > > /** > > > @@ -72,7 +75,7 @@ enum bootflow_flags_t { > > > * @fname: Filename of bootflow file (allocated) > > > * @logo: Logo to display for this bootflow (BMP format) > > > * @logo_size: Size of the logo in bytes > > > - * @buf: Bootflow file contents (allocated) > > > + * @buf: Bootflow file contents (allocated unless @flags & > > > BOOTFLOWF_STATIC_BUF) > > > * @size: Size of bootflow file in bytes > > > * @err: Error number received (0 if OK) > > > * @os_name: Name of the OS / distro being booted, or NULL if not known > > > -- > > > 2.43.0.rc0.421.g78406f8d94-goog > > >