On Wed, 11 Dec 2024 at 15:54, Simon Glass <[email protected]> wrote: > > Some functions are passing addresses instead of pointers to the > efi_add_memory_map() function. This confusion is understandable since > the function arguments indicate an address. > > Make a note of the 8 places where there are problems, which would break > usage in sandbox tests. > > Future work will resolve these problems.
NAK, this serves no purpose whatsoever. Just send a patch with whatever changes you think are needed Thanks /Ilias > > Signed-off-by: Simon Glass <[email protected]> > --- > A request was made in v4 to drop this patch, but in the same thread, a > query was sent about why we need these addresses, so at least until the > series is about to be applied, I've left it in, so we can have that > discussion in one place and build a shared understanding of why these > addresses are needed, instead of peppering the rest of the series with > similar questions. > > Link: > https://lore.kernel.org/u-boot/cac_iwjlex4z41cdpx4u07xecygjb7h+ynd8aqyl-wruh7yr...@mail.gmail.com/ > > (no changes since v2) > > Changes in v2: > - Add a new patch with comments where incorrect addresses are used > > lib/efi_loader/efi_acpi.c | 2 ++ > lib/efi_loader/efi_bootmgr.c | 5 +++++ > lib/efi_loader/efi_helper.c | 1 + > lib/efi_loader/efi_smbios.c | 6 +++++- > lib/efi_loader/efi_var_mem.c | 2 ++ > 5 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c > index 67bd7f8ca24..ffb360d461b 100644 > --- a/lib/efi_loader/efi_acpi.c > +++ b/lib/efi_loader/efi_acpi.c > @@ -28,12 +28,14 @@ efi_status_t efi_acpi_register(void) > /* Mark space used for tables */ > start = ALIGN_DOWN(gd->arch.table_start, EFI_PAGE_MASK); > end = ALIGN(gd->arch.table_end, EFI_PAGE_MASK); > + /* TODO(sjg): This should use (ulong)map_sysmem(start, ...) */ > ret = efi_add_memory_map(start, end - start, EFI_ACPI_RECLAIM_MEMORY); > if (ret != EFI_SUCCESS) > return ret; > if (gd->arch.table_start_high) { > start = ALIGN_DOWN(gd->arch.table_start_high, EFI_PAGE_MASK); > end = ALIGN(gd->arch.table_end_high, EFI_PAGE_MASK); > + /* TODO(sjg): This should use (ulong)map_sysmem(start, ...) */ > ret = efi_add_memory_map(start, end - start, > EFI_ACPI_RECLAIM_MEMORY); > if (ret != EFI_SUCCESS) > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > index 8c51a6ef2ed..bb635d77b53 100644 > --- a/lib/efi_loader/efi_bootmgr.c > +++ b/lib/efi_loader/efi_bootmgr.c > @@ -365,6 +365,8 @@ static efi_status_t prepare_loaded_image(u16 *label, > ulong addr, ulong size, > * TODO: expose the ramdisk to OS. > * Need to pass the ramdisk information by the architecture-specific > * methods such as 'pmem' device-tree node. > + * > + * TODO(sjg): This should use (ulong)map_sysmem(addr) > */ > ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE); > if (ret != EFI_SUCCESS) { > @@ -399,6 +401,7 @@ static efi_status_t efi_bootmgr_release_uridp(struct > uridp_context *ctx) > > /* cleanup for iso or img image */ > if (ctx->ramdisk_blk_dev) { > + /* TODO(sjg): This should use (ulong)map_sysmem(...) */ > ret = efi_add_memory_map(ctx->image_addr, ctx->image_size, > EFI_CONVENTIONAL_MEMORY); > if (ret != EFI_SUCCESS) > @@ -506,6 +509,8 @@ static efi_status_t try_load_from_uri_path(struct > efi_device_path_uri *uridp, > > source_buffer = NULL; > source_size = 0; > + > + /* TODO(sjg): This does not work on sandbox */ > } else if (efi_check_pe((void *)image_addr, image_size, NULL) == > EFI_SUCCESS) { > /* > * loaded_dp must exist until efi application returns, > diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c > index bf96f61d3d0..fd89ef6036f 100644 > --- a/lib/efi_loader/efi_helper.c > +++ b/lib/efi_loader/efi_helper.c > @@ -486,6 +486,7 @@ static efi_status_t copy_fdt(void **fdtp) > log_err("Failed to reserve space for FDT\n"); > goto done; > } > + /* TODO(sjg): This does not work on sandbox */ > new_fdt = (void *)(uintptr_t)new_fdt_addr; > memcpy(new_fdt, fdt, fdt_totalsize(fdt)); > fdt_set_totalsize(new_fdt, fdt_size); > diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c > index 8d2ef6deb51..02d4a5dd045 100644 > --- a/lib/efi_loader/efi_smbios.c > +++ b/lib/efi_loader/efi_smbios.c > @@ -40,7 +40,11 @@ efi_status_t efi_smbios_register(void) > return EFI_NOT_FOUND; > } > > - /* Mark space used for tables */ > + /* > + * Mark space used for tables/ > + * > + * TODO(sjg): This should use (ulong)map_sysmem(addr, ...) > + */ > ret = efi_add_memory_map(addr, TABLE_SIZE, EFI_RUNTIME_SERVICES_DATA); > if (ret) > return ret; > diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c > index 139e16aad7c..5c69a1e0f3e 100644 > --- a/lib/efi_loader/efi_var_mem.c > +++ b/lib/efi_loader/efi_var_mem.c > @@ -226,6 +226,8 @@ efi_status_t efi_var_mem_init(void) > &memory); > if (ret != EFI_SUCCESS) > return ret; > + > + /* TODO(sjg): This does not work on sandbox */ > efi_var_buf = (struct efi_var_file *)(uintptr_t)memory; > memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE); > efi_var_buf->magic = EFI_VAR_FILE_MAGIC; > -- > 2.34.1 >
