[U-Boot] [PATCH v3 00/11] efi_loader: error handling cmd/bootefi.c

This patch series is focused on providing error checking during the initialization of the EFI subsystem.
If any of the registrations routines fails bootefi is not executed. As currently the registration attempt cannot be unroled any further bootefi call will fail.
Passing a devicetree to bootefi selftest is enabled and a unit test is supplied.
This patch series implements some of the ideas of Simon's patch series efi: Enable basic sandbox support for EFI loader https://lists.denx.de/pipermail/u-boot/2018-February/320845.html and replaces his patches efi: Update efi_smbios_register() to return error code efi: Move the init check inside efi_init_obj_list() efi: Add error checking for efi_init_obj_list()
The differences are:
Simon's patch series only checks the failure of some initialization routines while this patch series checks all initialization routines called in the initialization stage of the EFI subsystem.
Simon's patch series detects an error but does not save the error state. So the next call of the bootefi command will duplicate handles. With the current patch series bootefi will return an error with any further call after a failed initialization.
Simon's patch series does not contain passing a devicetree to bootefi selftest.
v3 rebase a patch mention fdt address in online help for bootefi selftest v2 put existing and new patches into a patch series
Heinrich Schuchardt (11): efi_loader: efi_smbios_register should have a return value efi_loader: return efi_status_t from efi_gop_register efi_loader: return efi_status_t from efi_net_register efi_loader: consistently return efi_status_t efi_watchdog_register efi_loader: simplify calling efi_init_obj_list efi_loader: exit status for efi_reset_system_init efi_loader: efi_get_time_init should return status code efi_loader: do_bootefi_exec should always return an EFI status code efi_loader: check initialization of EFI subsystem is successful efi_loader: support device tree for bootefi selftest efi_selftest: check installation of the device tree
arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 4 +- arch/arm/mach-bcm283x/reset.c | 4 +- cmd/bootefi.c | 186 +++++++++++++++++++------------ include/efi_loader.h | 23 ++-- lib/efi_loader/efi_boottime.c | 2 + lib/efi_loader/efi_gop.c | 34 ++++-- lib/efi_loader/efi_net.c | 24 ++-- lib/efi_loader/efi_runtime.c | 18 ++- lib/efi_loader/efi_smbios.c | 23 ++-- lib/efi_loader/efi_watchdog.c | 4 +- lib/efi_selftest/Makefile | 1 + lib/efi_selftest/efi_selftest_fdt.c | 188 ++++++++++++++++++++++++++++++++ test/py/tests/test_efi_selftest.py | 14 +++ 13 files changed, 405 insertions(+), 120 deletions(-) create mode 100644 lib/efi_selftest/efi_selftest_fdt.c

Errors may occur inside efi_smbios_register().
- Return a status code. - Remove unused variables. - Use constants where applicable.
Suggested-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 remove a change in unrelated code resent with this patch series --- include/efi_loader.h | 2 +- lib/efi_loader/efi_smbios.c | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 07730c3f39..72c83fd503 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -185,7 +185,7 @@ int efi_net_register(void); /* Called by bootefi to make the watchdog available */ int efi_watchdog_register(void); /* Called by bootefi to make SMBIOS tables available */ -void efi_smbios_register(void); +efi_status_t efi_smbios_register(void);
struct efi_simple_file_system_protocol * efi_fs_from_path(struct efi_device_path *fp); diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index ac412e7362..62e9697902 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -13,20 +13,27 @@
static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
-void efi_smbios_register(void) +/* + * Install the SMBIOS table as a configuration table. + * + * @return status code + */ +efi_status_t efi_smbios_register(void) { /* Map within the low 32 bits, to allow for 32bit SMBIOS tables */ - uint64_t dmi = 0xffffffff; - /* Reserve 4kb for SMBIOS */ - uint64_t pages = 1; - int memtype = EFI_RUNTIME_SERVICES_DATA; + u64 dmi = U32_MAX; + efi_status_t ret;
- if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS) - return; + /* Reserve 4kiB page for SMBIOS */ + ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, + EFI_RUNTIME_SERVICES_DATA, 1, &dmi); + if (ret != EFI_SUCCESS) + return ret;
/* Generate SMBIOS tables */ write_smbios_table(dmi);
/* And expose them to our EFI payload */ - efi_install_configuration_table(&smbios_guid, (void*)(uintptr_t)dmi); + return efi_install_configuration_table(&smbios_guid, + (void *)(uintptr_t)dmi); }

All initialization routines should return a status code instead of a boolean.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- include/efi_loader.h | 2 +- lib/efi_loader/efi_gop.c | 34 ++++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 72c83fd503..779b8bde2e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -179,7 +179,7 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, const char *if_typename, int diskid, const char *pdevname); /* Called by bootefi to make GOP (graphical) interface available */ -int efi_gop_register(void); +efi_status_t efi_gop_register(void); /* Called by bootefi to make the network interface available */ int efi_net_register(void); /* Called by bootefi to make the watchdog available */ diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 3caddd5f84..91b0b6a064 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -125,8 +125,13 @@ efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, return EFI_EXIT(EFI_SUCCESS); }
-/* This gets called from do_bootefi_exec(). */ -int efi_gop_register(void) +/* + * Install graphical output protocol. + * + * If no supported video device exists this is not considered as an + * error. + */ +efi_status_t efi_gop_register(void) { struct efi_gop_obj *gopobj; u32 bpix, col, row; @@ -136,12 +141,15 @@ int efi_gop_register(void)
#ifdef CONFIG_DM_VIDEO struct udevice *vdev; + struct video_priv *priv;
/* We only support a single video output device for now */ - if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) - return -1; + if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) { + printf("WARNING: No video device\n"); + return EFI_SUCCESS; + }
- struct video_priv *priv = dev_get_uclass_priv(vdev); + priv = dev_get_uclass_priv(vdev); bpix = priv->bpix; col = video_get_xsize(vdev); row = video_get_ysize(vdev); @@ -170,13 +178,14 @@ int efi_gop_register(void) break; default: /* So far, we only work in 16 or 32 bit mode */ - return -1; + printf("WARNING: Unsupported video mode\n"); + return EFI_SUCCESS; }
gopobj = calloc(1, sizeof(*gopobj)); if (!gopobj) { printf("ERROR: Out of memory\n"); - return 1; + return EFI_OUT_OF_RESOURCES; }
/* Hook up to the device list */ @@ -186,8 +195,8 @@ int efi_gop_register(void) ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid, &gopobj->ops); if (ret != EFI_SUCCESS) { - printf("ERROR: Out of memory\n"); - return 1; + printf("ERROR: Failure adding gop protocol\n"); + return ret; } gopobj->ops.query_mode = gop_query_mode; gopobj->ops.set_mode = gop_set_mode; @@ -199,10 +208,11 @@ int efi_gop_register(void) gopobj->mode.info_size = sizeof(gopobj->info);
#ifdef CONFIG_DM_VIDEO - if (bpix == VIDEO_BPP32) { + if (bpix == VIDEO_BPP32) #else - if (bpix == LCD_COLOR32) { + if (bpix == LCD_COLOR32) #endif + { /* With 32bit color space we can directly expose the fb */ gopobj->mode.fb_base = fb_base; gopobj->mode.fb_size = fb_size; @@ -217,5 +227,5 @@ int efi_gop_register(void) gopobj->bpix = bpix; gopobj->fb = fb;
- return 0; + return EFI_SUCCESS; }

On 03/03/2018 03:28 PM, Heinrich Schuchardt wrote:
All initialization routines should return a status code instead of a boolean.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3 no change v2 new patch
include/efi_loader.h | 2 +- lib/efi_loader/efi_gop.c | 34 ++++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 72c83fd503..779b8bde2e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -179,7 +179,7 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, const char *if_typename, int diskid, const char *pdevname); /* Called by bootefi to make GOP (graphical) interface available */ -int efi_gop_register(void); +efi_status_t efi_gop_register(void); /* Called by bootefi to make the network interface available */ int efi_net_register(void); /* Called by bootefi to make the watchdog available */ diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 3caddd5f84..91b0b6a064 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -125,8 +125,13 @@ efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, return EFI_EXIT(EFI_SUCCESS); }
-/* This gets called from do_bootefi_exec(). */ -int efi_gop_register(void) +/*
- Install graphical output protocol.
- If no supported video device exists this is not considered as an
- error.
- */
+efi_status_t efi_gop_register(void) { struct efi_gop_obj *gopobj; u32 bpix, col, row; @@ -136,12 +141,15 @@ int efi_gop_register(void)
#ifdef CONFIG_DM_VIDEO struct udevice *vdev;
struct video_priv *priv;
/* We only support a single video output device for now */
- if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev)
return -1;
- if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) {
printf("WARNING: No video device\n");
I don't think we should emit a warning, just because someone enabled support for DM_VIDEO but doesn't have any device backing it. Imagine for example a cross-board U-Boot binary that adapts its device tree based on the board it finds. On some, it may use video output, on others it might not. Then we would emit a warning here for no good reason.
return EFI_SUCCESS;
- }
- struct video_priv *priv = dev_get_uclass_priv(vdev);
- priv = dev_get_uclass_priv(vdev); bpix = priv->bpix; col = video_get_xsize(vdev); row = video_get_ysize(vdev);
@@ -170,13 +178,14 @@ int efi_gop_register(void) break; default: /* So far, we only work in 16 or 32 bit mode */
return -1;
printf("WARNING: Unsupported video mode\n");
Same here. Maybe convert into debug() prints?
Alex
return EFI_SUCCESS;
}
gopobj = calloc(1, sizeof(*gopobj)); if (!gopobj) { printf("ERROR: Out of memory\n");
return 1;
return EFI_OUT_OF_RESOURCES;
}
/* Hook up to the device list */
@@ -186,8 +195,8 @@ int efi_gop_register(void) ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid, &gopobj->ops); if (ret != EFI_SUCCESS) {
printf("ERROR: Out of memory\n");
return 1;
printf("ERROR: Failure adding gop protocol\n");
} gopobj->ops.query_mode = gop_query_mode; gopobj->ops.set_mode = gop_set_mode;return ret;
@@ -199,10 +208,11 @@ int efi_gop_register(void) gopobj->mode.info_size = sizeof(gopobj->info);
#ifdef CONFIG_DM_VIDEO
- if (bpix == VIDEO_BPP32) {
- if (bpix == VIDEO_BPP32) #else
- if (bpix == LCD_COLOR32) {
- if (bpix == LCD_COLOR32) #endif
- { /* With 32bit color space we can directly expose the fb */ gopobj->mode.fb_base = fb_base; gopobj->mode.fb_size = fb_size;
@@ -217,5 +227,5 @@ int efi_gop_register(void) gopobj->bpix = bpix; gopobj->fb = fb;
- return 0;
- return EFI_SUCCESS; }

On 03/12/2018 01:07 PM, Alexander Graf wrote:
On 03/03/2018 03:28 PM, Heinrich Schuchardt wrote:
All initialization routines should return a status code instead of a boolean.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3 no change v2 new patch
include/efi_loader.h | 2 +- lib/efi_loader/efi_gop.c | 34 ++++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 72c83fd503..779b8bde2e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -179,7 +179,7 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, const char *if_typename, int diskid, const char *pdevname); /* Called by bootefi to make GOP (graphical) interface available */ -int efi_gop_register(void); +efi_status_t efi_gop_register(void); /* Called by bootefi to make the network interface available */ int efi_net_register(void); /* Called by bootefi to make the watchdog available */ diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 3caddd5f84..91b0b6a064 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -125,8 +125,13 @@ efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, return EFI_EXIT(EFI_SUCCESS); } -/* This gets called from do_bootefi_exec(). */ -int efi_gop_register(void) +/*
- Install graphical output protocol.
- If no supported video device exists this is not considered as an
- error.
- */
+efi_status_t efi_gop_register(void) { struct efi_gop_obj *gopobj; u32 bpix, col, row; @@ -136,12 +141,15 @@ int efi_gop_register(void) #ifdef CONFIG_DM_VIDEO struct udevice *vdev;
- struct video_priv *priv; /* We only support a single video output device for now */
- if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev)
return -1;
- if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) {
printf("WARNING: No video device\n");
I don't think we should emit a warning, just because someone enabled support for DM_VIDEO but doesn't have any device backing it. Imagine for example a cross-board U-Boot binary that adapts its device tree based on the board it finds. On some, it may use video output, on others it might not. Then we would emit a warning here for no good reason.
return EFI_SUCCESS;
- }
- struct video_priv *priv = dev_get_uclass_priv(vdev);
- priv = dev_get_uclass_priv(vdev); bpix = priv->bpix; col = video_get_xsize(vdev); row = video_get_ysize(vdev);
@@ -170,13 +178,14 @@ int efi_gop_register(void) break; default: /* So far, we only work in 16 or 32 bit mode */
return -1;
printf("WARNING: Unsupported video mode\n");
Same here. Maybe convert into debug() prints?
I've just applied it with those prints converted to debug() calls now.
Alex

Consistently return status codes form efi_net_register().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- include/efi_loader.h | 2 +- lib/efi_loader/efi_net.c | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 779b8bde2e..dd6ffdc484 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -181,7 +181,7 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, /* Called by bootefi to make GOP (graphical) interface available */ efi_status_t efi_gop_register(void); /* Called by bootefi to make the network interface available */ -int efi_net_register(void); +efi_status_t efi_net_register(void); /* Called by bootefi to make the watchdog available */ int efi_watchdog_register(void); /* Called by bootefi to make SMBIOS tables available */ diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 8c5d5b492c..e7fed79450 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -280,20 +280,22 @@ static void EFIAPI efi_network_timer_notify(struct efi_event *event, }
/* This gets called from do_bootefi_exec(). */ -int efi_net_register(void) +efi_status_t efi_net_register(void) { struct efi_net_obj *netobj; efi_status_t r;
if (!eth_get_dev()) { /* No eth device active, don't expose any */ - return 0; + return EFI_SUCCESS; }
/* We only expose the "active" eth device, so one is enough */ netobj = calloc(1, sizeof(*netobj)); - if (!netobj) - goto out_of_memory; + if (!netobj) { + printf("ERROR: Out of memory\n"); + return EFI_OUT_OF_RESOURCES; + }
/* Hook net up to the device list */ efi_add_handle(&netobj->parent); @@ -302,15 +304,15 @@ int efi_net_register(void) r = efi_add_protocol(netobj->parent.handle, &efi_net_guid, &netobj->net); if (r != EFI_SUCCESS) - goto out_of_memory; + goto failure_to_add_protocol; r = efi_add_protocol(netobj->parent.handle, &efi_guid_device_path, efi_dp_from_eth()); if (r != EFI_SUCCESS) - goto out_of_memory; + goto failure_to_add_protocol; r = efi_add_protocol(netobj->parent.handle, &efi_pxe_guid, &netobj->pxe); if (r != EFI_SUCCESS) - goto out_of_memory; + goto failure_to_add_protocol; netobj->net.revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION; netobj->net.start = efi_net_start; netobj->net.stop = efi_net_stop; @@ -366,8 +368,8 @@ int efi_net_register(void) return r; }
- return 0; -out_of_memory: - printf("ERROR: Out of memory\n"); - return 1; + return EFI_SUCCESS; +failure_to_add_protocol: + printf("ERROR: Failure to add protocol\n"); + return r; }

efi_watchdog_register() should always return a status code and not a boolean value.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- include/efi_loader.h | 2 +- lib/efi_loader/efi_watchdog.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index dd6ffdc484..08e6c6fb9c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -183,7 +183,7 @@ efi_status_t efi_gop_register(void); /* Called by bootefi to make the network interface available */ efi_status_t efi_net_register(void); /* Called by bootefi to make the watchdog available */ -int efi_watchdog_register(void); +efi_status_t efi_watchdog_register(void); /* Called by bootefi to make SMBIOS tables available */ efi_status_t efi_smbios_register(void);
diff --git a/lib/efi_loader/efi_watchdog.c b/lib/efi_loader/efi_watchdog.c index 35a45dedf8..b1c35a8e29 100644 --- a/lib/efi_loader/efi_watchdog.c +++ b/lib/efi_loader/efi_watchdog.c @@ -59,7 +59,7 @@ efi_status_t efi_set_watchdog(unsigned long timeout) * * This function is called by efi_init_obj_list() */ -int efi_watchdog_register(void) +efi_status_t efi_watchdog_register(void) { efi_status_t r;
@@ -85,5 +85,5 @@ int efi_watchdog_register(void) printf("ERROR: Failed to set watchdog timer\n"); return r; } - return 0; + return EFI_SUCCESS; }

efi_init_obj_list() should be executed only once.
Rather than having the caller check this variable and the callee set it, move all access to the variable inside the function. This reduces the logic needed to call efi_init_obj_list().
Suggested-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change, patch resent --- cmd/bootefi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index c17fa2ca23..1822e8a312 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -22,7 +22,7 @@
DECLARE_GLOBAL_DATA_PTR;
-static uint8_t efi_obj_list_initalized; +static u8 efi_obj_list_initialized;
static struct efi_device_path *bootefi_image_path; static struct efi_device_path *bootefi_device_path; @@ -30,7 +30,10 @@ static struct efi_device_path *bootefi_device_path; /* Initialize and populate EFI object list */ static void efi_init_obj_list(void) { - efi_obj_list_initalized = 1; + /* Initialize once only */ + if (efi_obj_list_initialized) + return; + efi_obj_list_initialized = 1;
/* Initialize EFI driver uclass */ efi_driver_init(); @@ -184,8 +187,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, }
/* Initialize and populate EFI object list */ - if (!efi_obj_list_initalized) - efi_init_obj_list(); + efi_init_obj_list();
efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, device_path, image_path); @@ -284,8 +286,7 @@ static int do_bootefi_bootmgr_exec(unsigned long fdt_addr) efi_status_t r;
/* Initialize and populate EFI object list */ - if (!efi_obj_list_initalized) - efi_init_obj_list(); + efi_init_obj_list();
/* * gd lives in a fixed register which may get clobbered while we execute @@ -350,8 +351,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) */ efi_save_gd(); /* Initialize and populate EFI object list */ - if (!efi_obj_list_initalized) - efi_init_obj_list(); + efi_init_obj_list(); /* Transfer environment variable efi_selftest as load options */ set_load_options(&loaded_image_info, "efi_selftest"); /* Execute the test */

efi_reset_system_init provides the architecture or board specific initialization of the EFI subsystem. Errors should be caught and signalled by a return code.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 4 ++-- arch/arm/mach-bcm283x/reset.c | 4 ++-- include/efi_loader.h | 11 ++++++++--- lib/efi_loader/efi_runtime.c | 15 ++++++++++++--- 4 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index 70a6070935..0e90302d3f 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -654,9 +654,9 @@ void __efi_runtime EFIAPI efi_reset_system( while (1) { } }
-void efi_reset_system_init(void) +efi_status_t efi_reset_system_init(void) { - efi_add_runtime_mmio(&rstcr, sizeof(*rstcr)); + return efi_add_runtime_mmio(&rstcr, sizeof(*rstcr)); }
#endif diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index b62cb8a51e..5b83fdf43d 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -82,9 +82,9 @@ void __efi_runtime EFIAPI efi_reset_system( while (1) { } }
-void efi_reset_system_init(void) +efi_status_t efi_reset_system_init(void) { - efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs)); + return efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs)); }
#endif diff --git a/include/efi_loader.h b/include/efi_loader.h index 08e6c6fb9c..199077d295 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -346,7 +346,7 @@ static inline int guidcmp(const efi_guid_t *g1, const efi_guid_t *g2)
/* Call this with mmio_ptr as the _pointer_ to a pointer to an MMIO region * to make it available at runtime */ -void efi_add_runtime_mmio(void *mmio_ptr, u64 len); +efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len);
/* Boards may provide the functions below to implement RTS functionality */
@@ -354,7 +354,9 @@ void __efi_runtime EFIAPI efi_reset_system( enum efi_reset_type reset_type, efi_status_t reset_status, unsigned long data_size, void *reset_data); -void efi_reset_system_init(void); + +/* Architecture specific initialization of the EFI subsystem */ +efi_status_t efi_reset_system_init(void);
efi_status_t __efi_runtime EFIAPI efi_get_time( struct efi_time *time, @@ -388,7 +390,10 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */ #define __efi_runtime_data #define __efi_runtime -static inline void efi_add_runtime_mmio(void *mmio_ptr, u64 len) { } +static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len) +{ + return EFI_SUCCESS; +}
/* No loader configured, stub out EFI_ENTRY */ static inline void efi_restore_gd(void) { } diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index ccb4fc6141..85f85711dd 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -134,8 +134,9 @@ void __weak __efi_runtime EFIAPI efi_reset_system( while (1) { } }
-void __weak efi_reset_system_init(void) +efi_status_t __weak efi_reset_system_init(void) { + return EFI_SUCCESS; }
efi_status_t __weak __efi_runtime EFIAPI efi_get_time( @@ -332,18 +333,26 @@ static efi_status_t EFIAPI efi_set_virtual_address_map( return EFI_EXIT(EFI_INVALID_PARAMETER); }
-void efi_add_runtime_mmio(void *mmio_ptr, u64 len) +efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len) { struct efi_runtime_mmio_list *newmmio; + efi_status_t ret;
u64 pages = (len + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; - efi_add_memory_map(*(uintptr_t *)mmio_ptr, pages, EFI_MMAP_IO, false); + ret = efi_add_memory_map(*(uintptr_t *)mmio_ptr, pages, EFI_MMAP_IO, + false); + if (ret != EFI_SUCCESS) + return ret;
newmmio = calloc(1, sizeof(*newmmio)); + if (!newmmio) + return EFI_OUT_OF_RESOURCES; newmmio->ptr = mmio_ptr; newmmio->paddr = *(uintptr_t *)mmio_ptr; newmmio->len = len; list_add_tail(&newmmio->link, &efi_runtime_mmio); + + return ret; }
/*

All EFI initialization functions should return a status code.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- include/efi_loader.h | 2 +- lib/efi_loader/efi_runtime.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 199077d295..85a47ca2f5 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -361,7 +361,7 @@ efi_status_t efi_reset_system_init(void); efi_status_t __efi_runtime EFIAPI efi_get_time( struct efi_time *time, struct efi_time_cap *capabilities); -void efi_get_time_init(void); +efi_status_t efi_get_time_init(void);
#ifdef CONFIG_CMD_BOOTEFI_SELFTEST /* diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 85f85711dd..c59d161c8f 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -147,8 +147,9 @@ efi_status_t __weak __efi_runtime EFIAPI efi_get_time( return EFI_DEVICE_ERROR; }
-void __weak efi_get_time_init(void) +efi_status_t __weak efi_get_time_init(void) { + return EFI_SUCCESS; }
struct efi_runtime_detach_list_struct {

The return type of do_bootefi_exec() is efi_status_t. So in case of an error we should always return an EFI status code.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change, patch resent --- cmd/bootefi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 1822e8a312..1f1c15a908 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -164,7 +164,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, struct efi_loaded_image loaded_image_info = {}; struct efi_object loaded_image_info_obj = {}; struct efi_device_path *memdp = NULL; - ulong ret; + efi_status_t ret;
EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, struct efi_system_table *st); @@ -229,7 +229,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, /* Load the EFI payload */ entry = efi_load_pe(efi, &loaded_image_info); if (!entry) { - ret = -ENOENT; + ret = EFI_LOAD_ERROR; goto exit; }

Up to now errors in the initialization of the EFI subsystems was not checked.
If any initialization fails, leave the bootefi command.
We do not retry initialization because this would require to undo all prior initalization steps.
Suggested-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 rebased (dependency on CONFIG_CMD_NET instead of CONFIG_NET) v2 no change, patch resent --- cmd/bootefi.c | 67 +++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 20 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 1f1c15a908..0eb9a1baab 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -22,40 +22,65 @@
DECLARE_GLOBAL_DATA_PTR;
-static u8 efi_obj_list_initialized; +#define OBJ_LIST_NOT_INITIALIZED 1 + +static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
static struct efi_device_path *bootefi_image_path; static struct efi_device_path *bootefi_device_path;
/* Initialize and populate EFI object list */ -static void efi_init_obj_list(void) +efi_status_t efi_init_obj_list(void) { + efi_status_t ret = EFI_SUCCESS; + /* Initialize once only */ - if (efi_obj_list_initialized) - return; - efi_obj_list_initialized = 1; + if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) + return efi_obj_list_initialized;
/* Initialize EFI driver uclass */ - efi_driver_init(); + ret = efi_driver_init(); + if (ret != EFI_SUCCESS) + goto out;
- efi_console_register(); + ret = efi_console_register(); + if (ret != EFI_SUCCESS) + goto out; #ifdef CONFIG_PARTITIONS - efi_disk_register(); + ret = efi_disk_register(); + if (ret != EFI_SUCCESS) + goto out; #endif #if defined(CONFIG_LCD) || defined(CONFIG_DM_VIDEO) - efi_gop_register(); + ret = efi_gop_register(); + if (ret != EFI_SUCCESS) + goto out; #endif #ifdef CONFIG_CMD_NET - efi_net_register(); + ret = efi_net_register(); + if (ret != EFI_SUCCESS) + goto out; #endif #ifdef CONFIG_GENERATE_SMBIOS_TABLE - efi_smbios_register(); + ret = efi_smbios_register(); + if (ret != EFI_SUCCESS) + goto out; #endif - efi_watchdog_register(); + ret = efi_watchdog_register(); + if (ret != EFI_SUCCESS) + goto out;
/* Initialize EFI runtime services */ - efi_reset_system_init(); - efi_get_time_init(); + ret = efi_reset_system_init(); + if (ret != EFI_SUCCESS) + goto out; + ret = efi_get_time_init(); + if (ret != EFI_SUCCESS) + goto out; + +out: + efi_obj_list_initialized = ret; + return ret; }
/* @@ -186,9 +211,6 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, assert(device_path && image_path); }
- /* Initialize and populate EFI object list */ - efi_init_obj_list(); - efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, device_path, image_path);
@@ -285,9 +307,6 @@ static int do_bootefi_bootmgr_exec(unsigned long fdt_addr) void *addr; efi_status_t r;
- /* Initialize and populate EFI object list */ - efi_init_obj_list(); - /* * gd lives in a fixed register which may get clobbered while we execute * the payload. So save it here and restore it on every callback entry @@ -316,6 +335,14 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) unsigned long addr, fdt_addr = 0; efi_status_t r;
+ /* Initialize EFI drivers */ + r = efi_init_obj_list(); + if (r != EFI_SUCCESS) { + printf("Error: Cannot set up EFI drivers, r = %lu\n", + r & ~EFI_ERROR_MASK); + return CMD_RET_FAILURE; + } + if (argc < 2) return CMD_RET_USAGE; #ifdef CONFIG_CMD_BOOTEFI_HELLO

The second argument of the bootefi command should always be usable to specify a device tree. This was missing for bootefi selftest and bootefi hello.
Proper error handling is added.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 Mention fdt address as argument in online help of bootefi selftest v2 new patch --- cmd/bootefi.c | 111 ++++++++++++++++++++++++------------------ include/efi_loader.h | 2 + lib/efi_loader/efi_boottime.c | 2 + 3 files changed, 68 insertions(+), 47 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 0eb9a1baab..6de7d8c743 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -178,11 +178,49 @@ static efi_status_t efi_run_in_el2(EFIAPI efi_status_t (*entry)( } #endif
+static efi_status_t efi_install_fdt(void *fdt) +{ + bootm_headers_t img = { 0 }; + ulong fdt_pages, fdt_size, fdt_start, fdt_end; + efi_status_t ret; + + if (fdt_check_header(fdt)) { + printf("ERROR: invalid device tree\n"); + return EFI_INVALID_PARAMETER; + } + + /* Prepare fdt for payload */ + fdt = copy_fdt(fdt); + if (!fdt) + return EFI_OUT_OF_RESOURCES; + + if (image_setup_libfdt(&img, fdt, 0, NULL)) { + printf("ERROR: failed to process device tree\n"); + return EFI_LOAD_ERROR; + } + + /* Link to it in the efi tables */ + ret = efi_install_configuration_table(&efi_guid_fdt, fdt); + if (ret != EFI_SUCCESS) + return EFI_OUT_OF_RESOURCES; + + /* And reserve the space in the memory map */ + fdt_start = ((ulong)fdt) & ~EFI_PAGE_MASK; + fdt_end = ((ulong)fdt) + fdt_totalsize(fdt); + fdt_size = (fdt_end - fdt_start) + EFI_PAGE_MASK; + fdt_pages = fdt_size >> EFI_PAGE_SHIFT; + /* Give a bootloader the chance to modify the device tree */ + fdt_pages += 2; + ret = efi_add_memory_map(fdt_start, fdt_pages, + EFI_BOOT_SERVICES_DATA, true); + return ret; +} + /* * Load an EFI payload into a newly allocated piece of memory, register all * EFI objects it would want to access and jump to it. */ -static efi_status_t do_bootefi_exec(void *efi, void *fdt, +static efi_status_t do_bootefi_exec(void *efi, struct efi_device_path *device_path, struct efi_device_path *image_path) { @@ -193,9 +231,6 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt,
EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, struct efi_system_table *st); - ulong fdt_pages, fdt_size, fdt_start, fdt_end; - const efi_guid_t fdt_guid = EFI_FDT_GUID; - bootm_headers_t img = { 0 };
/* * Special case for efi payload not loaded from disk, such as @@ -220,32 +255,6 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, */ efi_save_gd();
- if (fdt && !fdt_check_header(fdt)) { - /* Prepare fdt for payload */ - fdt = copy_fdt(fdt); - - if (image_setup_libfdt(&img, fdt, 0, NULL)) { - printf("ERROR: Failed to process device tree\n"); - return -EINVAL; - } - - /* Link to it in the efi tables */ - efi_install_configuration_table(&fdt_guid, fdt); - - /* And reserve the space in the memory map */ - fdt_start = ((ulong)fdt) & ~EFI_PAGE_MASK; - fdt_end = ((ulong)fdt) + fdt_totalsize(fdt); - fdt_size = (fdt_end - fdt_start) + EFI_PAGE_MASK; - fdt_pages = fdt_size >> EFI_PAGE_SHIFT; - /* Give a bootloader the chance to modify the device tree */ - fdt_pages += 2; - efi_add_memory_map(fdt_start, fdt_pages, - EFI_BOOT_SERVICES_DATA, true); - } else { - printf("WARNING: Invalid device tree, expect boot to fail\n"); - efi_install_configuration_table(&fdt_guid, NULL); - } - /* Transfer environment variable bootargs as load options */ set_load_options(&loaded_image_info, "bootargs"); /* Load the EFI payload */ @@ -301,7 +310,7 @@ exit: return ret; }
-static int do_bootefi_bootmgr_exec(unsigned long fdt_addr) +static int do_bootefi_bootmgr_exec(void) { struct efi_device_path *device_path, *file_path; void *addr; @@ -318,7 +327,7 @@ static int do_bootefi_bootmgr_exec(unsigned long fdt_addr) return 1;
printf("## Starting EFI application at %p ...\n", addr); - r = do_bootefi_exec(addr, (void *)fdt_addr, device_path, file_path); + r = do_bootefi_exec(addr, device_path, file_path); printf("## Application terminated, r = %lu\n", r & ~EFI_ERROR_MASK);
@@ -331,9 +340,10 @@ static int do_bootefi_bootmgr_exec(unsigned long fdt_addr) /* Interpreter command to boot an arbitrary EFI image from memory */ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - char *saddr, *sfdt; - unsigned long addr, fdt_addr = 0; + unsigned long addr; + char *saddr; efi_status_t r; + void *fdt_addr;
/* Initialize EFI drivers */ r = efi_init_obj_list(); @@ -345,6 +355,22 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (argc < 2) return CMD_RET_USAGE; + + if (argc > 2) { + fdt_addr = (void *)simple_strtoul(argv[2], NULL, 16); + if (!fdt_addr && *argv[2] != '0') + return CMD_RET_USAGE; + /* Install device tree */ + r = efi_install_fdt(fdt_addr); + if (r != EFI_SUCCESS) { + printf("ERROR: failed to install device tree\n"); + return CMD_RET_FAILURE; + } + } else { + /* Remove device tree. EFI_NOT_FOUND can be ignored here */ + efi_install_configuration_table(&efi_guid_fdt, NULL); + printf("WARNING: booting without device tree\n"); + } #ifdef CONFIG_CMD_BOOTEFI_HELLO if (!strcmp(argv[1], "hello")) { ulong size = __efi_helloworld_end - __efi_helloworld_begin; @@ -390,12 +416,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) { - unsigned long fdt_addr = 0; - - if (argc > 2) - fdt_addr = simple_strtoul(argv[2], NULL, 16); - - return do_bootefi_bootmgr_exec(fdt_addr); + return do_bootefi_bootmgr_exec(); } else { saddr = argv[1];
@@ -404,15 +425,11 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!addr && *saddr != '0') return CMD_RET_USAGE;
- if (argc > 2) { - sfdt = argv[2]; - fdt_addr = simple_strtoul(sfdt, NULL, 16); - } }
printf("## Starting EFI application at %08lx ...\n", addr); - r = do_bootefi_exec((void *)addr, (void *)fdt_addr, - bootefi_device_path, bootefi_image_path); + r = do_bootefi_exec((void *)addr, bootefi_device_path, + bootefi_image_path); printf("## Application terminated, r = %lu\n", r & ~EFI_ERROR_MASK);
@@ -433,7 +450,7 @@ static char bootefi_help_text[] = " - boot a sample Hello World application stored within U-Boot\n" #endif #ifdef CONFIG_CMD_BOOTEFI_SELFTEST - "bootefi selftest\n" + "bootefi selftest [fdt address]\n" " - boot an EFI selftest application stored within U-Boot\n" " Use environment variable efi_selftest to select a single test.\n" " Use 'setenv efi_selftest list' to enumerate all tests.\n" diff --git a/include/efi_loader.h b/include/efi_loader.h index 85a47ca2f5..3ccbb98aaf 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -93,6 +93,8 @@ extern const efi_guid_t efi_guid_console_control; extern const efi_guid_t efi_guid_device_path; /* GUID of the EFI_DRIVER_BINDING_PROTOCOL */ extern const efi_guid_t efi_guid_driver_binding_protocol; +/* GUID of the device tree table */ +extern const efi_guid_t efi_guid_fdt; extern const efi_guid_t efi_guid_loaded_image; extern const efi_guid_t efi_guid_device_path_to_text_protocol; extern const efi_guid_t efi_simple_file_system_protocol_guid; diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b80f59fd92..2f67030151 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -56,6 +56,8 @@ static volatile void *efi_gd, *app_gd;
static int entry_count; static int nesting_level; +/* GUID of the device tree table */ +const efi_guid_t efi_guid_fdt = EFI_FDT_GUID; /* GUID of the EFI_DRIVER_BINDING_PROTOCOL */ const efi_guid_t efi_guid_driver_binding_protocol = EFI_DRIVER_BINDING_PROTOCOL_GUID;

The unit test checks if a device tree is installed. It requires that the 'compatible' property of the root node exists. If available it prints the 'serial-number' property.
The serial-number property is derived from the environment variable 'serial#'. This can be used to check if the image_setup_libfdt() function is executed.
A Python test is supplied. It sets a value for serial# and checks that the selftest shows this as serial-number.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- lib/efi_selftest/Makefile | 1 + lib/efi_selftest/efi_selftest_fdt.c | 188 ++++++++++++++++++++++++++++++++++++ test/py/tests/test_efi_selftest.py | 14 +++ 3 files changed, 203 insertions(+) create mode 100644 lib/efi_selftest/efi_selftest_fdt.c
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index c4bdbdf6c0..88c44840d5 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -19,6 +19,7 @@ efi_selftest_console.o \ efi_selftest_devicepath.o \ efi_selftest_events.o \ efi_selftest_exitbootservices.o \ +efi_selftest_fdt.o \ efi_selftest_gop.o \ efi_selftest_manageprotocols.o \ efi_selftest_snp.o \ diff --git a/lib/efi_selftest/efi_selftest_fdt.c b/lib/efi_selftest/efi_selftest_fdt.c new file mode 100644 index 0000000000..24db0dcf7d --- /dev/null +++ b/lib/efi_selftest/efi_selftest_fdt.c @@ -0,0 +1,188 @@ +/* + * efi_selftest_pos + * + * Copyright (c) 2018 Heinrich Schuchardt xypron.glpk@gmx.de + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Test the EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL. + * + * The following services are tested: + * OutputString, TestString, SetAttribute. + */ + +#include <efi_selftest.h> +#include <libfdt.h> + +static struct efi_boot_services *boottime; +static const char *fdt; + +/* This should be sufficent for */ +#define BUFFERSIZE 0x100000 + +static efi_guid_t fdt_guid = EFI_FDT_GUID; + +/* + * Convert FDT value to host endianness. + * + * @val FDT value + * @return converted value + */ +static uint32_t f2h(fdt32_t val) +{ + char *buf = (char *)&val; + char i; + +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ + /* Swap the bytes */ + i = buf[0]; buf[0] = buf[3]; buf[3] = i; + i = buf[1]; buf[1] = buf[2]; buf[2] = i; +#endif + return *(uint32_t *)buf; +} + +/* + * Return the value of a property of the FDT root node. + * + * @name name of the property + * @return value of the property + */ +static char *get_property(const u16 *property) +{ + struct fdt_header *header = (struct fdt_header *)fdt; + const fdt32_t *pos; + const char *strings; + + if (!header) + return NULL; + + if (f2h(header->magic) != FDT_MAGIC) { + printf("Wrong magic\n"); + return NULL; + } + + pos = (fdt32_t *)(fdt + f2h(header->off_dt_struct)); + strings = fdt + f2h(header->off_dt_strings); + + for (;;) { + switch (f2h(pos[0])) { + case FDT_BEGIN_NODE: { + char *c = (char *)&pos[1]; + size_t i; + + for (i = 0; c[i]; ++i) + ; + pos = &pos[2 + (i >> 2)]; + break; + } + case FDT_PROP: { + struct fdt_property *prop = (struct fdt_property *)pos; + const char *label = &strings[f2h(prop->nameoff)]; + efi_status_t ret; + + /* Check if this is the property to be returned */ + if (!efi_st_strcmp_16_8(property, label)) { + char *str; + efi_uintn_t len = f2h(prop->len); + + if (!len) + return NULL; + /* + * The string might not be 0 terminated. + * It is safer to make a copy. + */ + ret = boottime->allocate_pool( + EFI_LOADER_DATA, len + 1, + (void **)&str); + if (ret != EFI_SUCCESS) { + efi_st_printf("AllocatePool failed\n"); + return NULL; + } + boottime->copy_mem(str, &pos[3], len); + str[len] = 0; + + return str; + } + + pos = &pos[3 + ((f2h(prop->len) + 3) >> 2)]; + break; + } + case FDT_NOP: + pos = &pos[1]; + break; + default: + return NULL; + } + } +} + +/* + * Setup unit test. + * + * @handle: handle of the loaded image + * @systable: system table + * @return: EFI_ST_SUCCESS for success + */ +static int setup(const efi_handle_t img_handle, + const struct efi_system_table *systable) +{ + efi_uintn_t i; + + boottime = systable->boottime; + + /* Find configuration tables */ + for (i = 0; i < systable->nr_tables; ++i) { + if (!efi_st_memcmp(&systable->tables[i].guid, &fdt_guid, + sizeof(efi_guid_t))) + fdt = systable->tables[i].table; + } + if (!fdt) { + efi_st_error("Missing device tree\n"); + return EFI_ST_FAILURE; + } + + return EFI_ST_SUCCESS; +} + +/* + * Execute unit test. + * + * @return: EFI_ST_SUCCESS for success + */ +static int execute(void) +{ + char *str; + efi_status_t ret; + + str = get_property(L"compatible"); + if (str) { + efi_st_printf("compatible: %s\n", str); + ret = boottime->free_pool(str); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + } else { + efi_st_printf("Missing property 'compatible'\n"); + return EFI_ST_FAILURE; + } + str = get_property(L"serial-number"); + if (str) { + efi_st_printf("serial-number: %s\n", str); + ret = boottime->free_pool(str); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(fdt) = { + .name = "device tree", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .on_request = true, +}; diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py index 66b799bed6..b1ef6bd581 100644 --- a/test/py/tests/test_efi_selftest.py +++ b/test/py/tests/test_efi_selftest.py @@ -24,6 +24,20 @@ def test_efi_selftest(u_boot_console): raise Exception('Reset failed during the EFI selftest') u_boot_console.restart_uboot();
+@pytest.mark.buildconfigspec('cmd_bootefi_selftest') +@pytest.mark.buildconfigspec('of_control') +def test_efi_selftest_device_tree(u_boot_console): + u_boot_console.run_command(cmd='setenv efi_selftest list') + output = u_boot_console.run_command('bootefi selftest') + assert ''device tree'' in output + u_boot_console.run_command(cmd='setenv efi_selftest device tree') + u_boot_console.run_command(cmd='setenv -f serial# Testing DT') + u_boot_console.run_command(cmd='bootefi selftest ${fdtcontroladdr}', wait_for_prompt=False) + m = u_boot_console.p.expect(['serial-number: Testing DT', 'U-Boot']) + if m != 0: + raise Exception('Reset failed in 'device tree' test') + u_boot_console.restart_uboot(); + @pytest.mark.buildconfigspec('cmd_bootefi_selftest') def test_efi_selftest_watchdog_reboot(u_boot_console): u_boot_console.run_command(cmd='setenv efi_selftest list')
participants (2)
-
Alexander Graf
-
Heinrich Schuchardt