| Message ID | 20211204155657.2913911-15-sjg@chromium.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi: Improvements to U-Boot running on top of UEFI | expand |
On 12/4/21 07:56, Simon Glass wrote: > At present the 'efi' command only works in the EFI payload. Update it to > work in the app too, so the memory map can be examined. cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a future patch we should try to move to using common code for commands efi and efidebug. Best regards Heinrich > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > cmd/Makefile | 2 +- > cmd/efi.c | 48 ++++++++++++++++++++++++++++++++--------------- > include/efi.h | 15 +++++++++++++++ > lib/efi/efi_app.c | 33 ++++++++++++++++++++++++++++++++ > 4 files changed, 82 insertions(+), 16 deletions(-) > > diff --git a/cmd/Makefile b/cmd/Makefile > index 891819ae0f6..df50625bde7 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -58,7 +58,7 @@ obj-$(CONFIG_CMD_EXTENSION) += extension_board.o > obj-$(CONFIG_CMD_ECHO) += echo.o > obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o > obj-$(CONFIG_CMD_EEPROM) += eeprom.o > -obj-$(CONFIG_EFI_STUB) += efi.o > +obj-$(CONFIG_EFI) += efi.o > obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o > obj-$(CONFIG_CMD_ELF) += elf.o > obj-$(CONFIG_HUSH_PARSER) += exit.o > diff --git a/cmd/efi.c b/cmd/efi.c > index d2400acbbba..c0384e0db28 100644 > --- a/cmd/efi.c > +++ b/cmd/efi.c > @@ -13,6 +13,8 @@ > #include <sort.h> > #include <asm/global_data.h> > > +DECLARE_GLOBAL_DATA_PTR; > + > static const char *const type_name[] = { > "reserved", > "loader_code", > @@ -217,37 +219,53 @@ static void efi_print_mem_table(struct efi_mem_desc *desc, int desc_size, > static int do_efi_mem(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > { > - struct efi_mem_desc *desc; > - struct efi_entry_memmap *map; > + struct efi_mem_desc *orig, *desc; > + uint version, key; > + int desc_size; > int size, ret; > bool skip_bs; > > skip_bs = !argc || *argv[0] != 'a'; > - ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size); > - switch (ret) { > - case -ENOENT: > - printf("No EFI table available\n"); > - goto done; > - case -EPROTONOSUPPORT: > - printf("Incorrect EFI table version\n"); > - goto done; > + if (IS_ENABLED(CONFIG_EFI_APP)) { > + ret = efi_get_mmap(&orig, &size, &key, &desc_size, &version); > + if (ret) { > + printf("Cannot read memory map (err=%d)\n", ret); > + return CMD_RET_FAILURE; > + } > + } else { > + struct efi_entry_memmap *map; > + > + ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size); > + switch (ret) { > + case -ENOENT: > + printf("No EFI table available\n"); > + goto done; > + case -EPROTONOSUPPORT: > + printf("Incorrect EFI table version\n"); > + goto done; > + } > + orig = map->desc; > + desc_size = map->desc_size; > + version = map->version; > } > - printf("EFI table at %lx, memory map %p, size %x, version %x, descr. size %#x\n", > - gd->arch.table, map, size, map->version, map->desc_size); > - if (map->version != EFI_MEM_DESC_VERSION) { > + printf("EFI table at %lx, memory map %p, size %x, key %x, version %x, descr. size %#x\n", > + gd->arch.table, orig, size, key, version, desc_size); > + if (version != EFI_MEM_DESC_VERSION) { > printf("Incorrect memory map version\n"); > ret = -EPROTONOSUPPORT; > goto done; > } > > - desc = efi_build_mem_table(map->desc, size, map->desc_size, skip_bs); > + desc = efi_build_mem_table(orig, size, desc_size, skip_bs); > if (!desc) { > ret = -ENOMEM; > goto done; > } > > - efi_print_mem_table(desc, map->desc_size, skip_bs); > + efi_print_mem_table(desc, desc_size, skip_bs); > free(desc); > + if (IS_ENABLED(CONFIG_EFI_APP)) > + free(orig); > done: > if (ret) > printf("Error: %d\n", ret); > diff --git a/include/efi.h b/include/efi.h > index 1dc806a1267..8a43430d3df 100644 > --- a/include/efi.h > +++ b/include/efi.h > @@ -610,4 +610,19 @@ int efi_store_memory_map(struct efi_priv *priv); > */ > int efi_call_exit_boot_services(void); > > +/** > + * efi_get_mmap() - Get the memory map from EFI > + * > + * This is used in the app. The caller must free *@descp when done > + * > + * @descp: Returns allocated pointer to EFI memory map table > + * @sizep: Returns size of table in bytes > + * @keyp: Returns memory-map key > + * @desc_sizep: Returns size of each @desc_base record > + * @versionp: Returns version number of memory map > + * @return 0 on success, -ve on error > + */ > +int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp, > + int *desc_sizep, uint *versionp); > + > #endif /* _LINUX_EFI_H */ > diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c > index 36e3f1de427..55fa1ac57d5 100644 > --- a/lib/efi/efi_app.c > +++ b/lib/efi/efi_app.c > @@ -32,6 +32,39 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) > return -ENOSYS; > } > > +int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp, > + int *desc_sizep, uint *versionp) > +{ > + struct efi_priv *priv = efi_get_priv(); > + struct efi_boot_services *boot = priv->sys_table->boottime; > + efi_uintn_t size, desc_size, key; > + struct efi_mem_desc *desc; > + efi_status_t ret; > + u32 version; > + > + /* Get the memory map so we can switch off EFI */ > + size = 0; > + ret = boot->get_memory_map(&size, NULL, &key, &desc_size, &version); > + if (ret != EFI_BUFFER_TOO_SMALL) > + return log_msg_ret("get", -ENOMEM); > + > + desc = malloc(size); > + if (!desc) > + return log_msg_ret("mem", -ENOMEM); > + > + ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version); > + if (ret) > + return log_msg_ret("get", -EINVAL); > + > + *descp = desc; > + *sizep = size; > + *desc_sizep = desc_size; > + *versionp = version; > + *keyp = key; > + > + return 0; > +} > + > /** > * efi_print_str() - Print a UFT-16 string to the U-Boot console > * >
Hi Heinrich, On Thu, 9 Dec 2021 at 13:27, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 12/4/21 07:56, Simon Glass wrote: > > At present the 'efi' command only works in the EFI payload. Update it to > > work in the app too, so the memory map can be examined. > > cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a > future patch we should try to move to using common code for commands efi > and efidebug. Yes and that depends on EFI_LOADER. Perhaps one day we can have a common helper for this, e.g. in lib/efi ? Regards, Simon
On Fri, Dec 17, 2021 at 09:37:21AM -0700, Simon Glass wrote: > Hi Heinrich, > > On Thu, 9 Dec 2021 at 13:27, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 12/4/21 07:56, Simon Glass wrote: > > > At present the 'efi' command only works in the EFI payload. Update it to > > > work in the app too, so the memory map can be examined. > > > > cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a > > future patch we should try to move to using common code for commands efi > > and efidebug. > > Yes and that depends on EFI_LOADER. Perhaps one day we can have a > common helper for this, e.g. in lib/efi ? It will inevitably means that we should stick to *UEFI* interfaces (not EFI_LOADER's internal interfaces like efi_xxx_yyy()) to implement those commands, including most of efidebug subcommands. -Takahiro Akashi > Regards, > Simon
Hi Takahiro, On Sun, 19 Dec 2021 at 19:38, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > On Fri, Dec 17, 2021 at 09:37:21AM -0700, Simon Glass wrote: > > Hi Heinrich, > > > > On Thu, 9 Dec 2021 at 13:27, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > On 12/4/21 07:56, Simon Glass wrote: > > > > At present the 'efi' command only works in the EFI payload. Update it to > > > > work in the app too, so the memory map can be examined. > > > > > > cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a > > > future patch we should try to move to using common code for commands efi > > > and efidebug. > > > > Yes and that depends on EFI_LOADER. Perhaps one day we can have a > > common helper for this, e.g. in lib/efi ? > > It will inevitably means that we should stick to *UEFI* interfaces > (not EFI_LOADER's internal interfaces like efi_xxx_yyy()) to implement > those commands, including most of efidebug subcommands. That seems right to me, although of course there may be some 'inside knowledge' for some subcommands. Regards, Simon
diff --git a/cmd/Makefile b/cmd/Makefile index 891819ae0f6..df50625bde7 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -58,7 +58,7 @@ obj-$(CONFIG_CMD_EXTENSION) += extension_board.o obj-$(CONFIG_CMD_ECHO) += echo.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o -obj-$(CONFIG_EFI_STUB) += efi.o +obj-$(CONFIG_EFI) += efi.o obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_HUSH_PARSER) += exit.o diff --git a/cmd/efi.c b/cmd/efi.c index d2400acbbba..c0384e0db28 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -13,6 +13,8 @@ #include <sort.h> #include <asm/global_data.h> +DECLARE_GLOBAL_DATA_PTR; + static const char *const type_name[] = { "reserved", "loader_code", @@ -217,37 +219,53 @@ static void efi_print_mem_table(struct efi_mem_desc *desc, int desc_size, static int do_efi_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - struct efi_mem_desc *desc; - struct efi_entry_memmap *map; + struct efi_mem_desc *orig, *desc; + uint version, key; + int desc_size; int size, ret; bool skip_bs; skip_bs = !argc || *argv[0] != 'a'; - ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size); - switch (ret) { - case -ENOENT: - printf("No EFI table available\n"); - goto done; - case -EPROTONOSUPPORT: - printf("Incorrect EFI table version\n"); - goto done; + if (IS_ENABLED(CONFIG_EFI_APP)) { + ret = efi_get_mmap(&orig, &size, &key, &desc_size, &version); + if (ret) { + printf("Cannot read memory map (err=%d)\n", ret); + return CMD_RET_FAILURE; + } + } else { + struct efi_entry_memmap *map; + + ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size); + switch (ret) { + case -ENOENT: + printf("No EFI table available\n"); + goto done; + case -EPROTONOSUPPORT: + printf("Incorrect EFI table version\n"); + goto done; + } + orig = map->desc; + desc_size = map->desc_size; + version = map->version; } - printf("EFI table at %lx, memory map %p, size %x, version %x, descr. size %#x\n", - gd->arch.table, map, size, map->version, map->desc_size); - if (map->version != EFI_MEM_DESC_VERSION) { + printf("EFI table at %lx, memory map %p, size %x, key %x, version %x, descr. size %#x\n", + gd->arch.table, orig, size, key, version, desc_size); + if (version != EFI_MEM_DESC_VERSION) { printf("Incorrect memory map version\n"); ret = -EPROTONOSUPPORT; goto done; } - desc = efi_build_mem_table(map->desc, size, map->desc_size, skip_bs); + desc = efi_build_mem_table(orig, size, desc_size, skip_bs); if (!desc) { ret = -ENOMEM; goto done; } - efi_print_mem_table(desc, map->desc_size, skip_bs); + efi_print_mem_table(desc, desc_size, skip_bs); free(desc); + if (IS_ENABLED(CONFIG_EFI_APP)) + free(orig); done: if (ret) printf("Error: %d\n", ret); diff --git a/include/efi.h b/include/efi.h index 1dc806a1267..8a43430d3df 100644 --- a/include/efi.h +++ b/include/efi.h @@ -610,4 +610,19 @@ int efi_store_memory_map(struct efi_priv *priv); */ int efi_call_exit_boot_services(void); +/** + * efi_get_mmap() - Get the memory map from EFI + * + * This is used in the app. The caller must free *@descp when done + * + * @descp: Returns allocated pointer to EFI memory map table + * @sizep: Returns size of table in bytes + * @keyp: Returns memory-map key + * @desc_sizep: Returns size of each @desc_base record + * @versionp: Returns version number of memory map + * @return 0 on success, -ve on error + */ +int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp, + int *desc_sizep, uint *versionp); + #endif /* _LINUX_EFI_H */ diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index 36e3f1de427..55fa1ac57d5 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -32,6 +32,39 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) return -ENOSYS; } +int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp, + int *desc_sizep, uint *versionp) +{ + struct efi_priv *priv = efi_get_priv(); + struct efi_boot_services *boot = priv->sys_table->boottime; + efi_uintn_t size, desc_size, key; + struct efi_mem_desc *desc; + efi_status_t ret; + u32 version; + + /* Get the memory map so we can switch off EFI */ + size = 0; + ret = boot->get_memory_map(&size, NULL, &key, &desc_size, &version); + if (ret != EFI_BUFFER_TOO_SMALL) + return log_msg_ret("get", -ENOMEM); + + desc = malloc(size); + if (!desc) + return log_msg_ret("mem", -ENOMEM); + + ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version); + if (ret) + return log_msg_ret("get", -EINVAL); + + *descp = desc; + *sizep = size; + *desc_sizep = desc_size; + *versionp = version; + *keyp = key; + + return 0; +} + /** * efi_print_str() - Print a UFT-16 string to the U-Boot console *
At present the 'efi' command only works in the EFI payload. Update it to work in the app too, so the memory map can be examined. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) cmd/Makefile | 2 +- cmd/efi.c | 48 ++++++++++++++++++++++++++++++++--------------- include/efi.h | 15 +++++++++++++++ lib/efi/efi_app.c | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 16 deletions(-)