All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement
@ 2018-11-26  3:14 Simon Glass
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 1/4] efi: Check for failure to create objects in selftest Simon Glass
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Simon Glass @ 2018-11-26  3:14 UTC (permalink / raw)
  To: u-boot

This collects the patches previously sent to break up the very large
functions in efi_loader into smaller pieces. Now that the other sandbox
stuff is applied, perhaps it is time to apply these patches.

This also adds a few new patches to fix more recent breakages.
Unfortunately we still cannot enable the efi loader tests since one of
the tests fails. Thus we should expect additional failures to appear
until that is resolved.

Changes in v16:
- Add new patch to check for failure to create objects in selftest
- Drop comments about efi_save_gd() being called in efi_init_obj_list()

Changes in v15:
- Add a comment about a leaked device path
- Add check for return values to bootefi_test_prepare()
- Drop call to efi_save_gd() in bootefi_test_prepare()
- Drop patch already applied
- Fix minor checkpatch nit with bracket

Changes in v14:
- Go back to the horrible long variable names
- Hopefully correct error paths in do_bootefi_exec()

Changes in v13:
- Drop 'efi_loader: Drop setup_ok' as we have an existing patch for that
- Drop patches previously applied
- Rebase to efi/efi-next

Changes in v12:
- Rename image to image_prot

Changes in v11:
- Drop patches previously applied

Changes in v9:
- Add comments to bootefi_test_prepare() about the memset()s

Changes in v7:
- Drop patch "efi: Init the 'rows' and 'cols' variables"
- Drop patches previous applied

Changes in v5:
- Drop call to efi_init_obj_list() which is now done in do_bootefi()
- Introduce load_options_path to specifyc U-Boot env var for load_options_path
- Rebase to master

Changes in v4:
- Rebase to master

Changes in v3:
- Add new patch to rename bootefi_test_finish() to bootefi_run_finish()
- Add new patch to split out test init/uninit into functions
- Add patch to create a function to set up for running EFI code

Simon Glass (4):
  efi: Check for failure to create objects in selftest
  efi: Split out test init/uninit into functions
  efi: Create a function to set up for running EFI code
  efi: Rename bootefi_test_finish() to bootefi_run_finish()

 cmd/bootefi.c | 118 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 89 insertions(+), 29 deletions(-)

-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v16 1/4] efi: Check for failure to create objects in selftest
  2018-11-26  3:14 [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
@ 2018-11-26  3:14 ` Simon Glass
  2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 2/4] efi: Split out test init/uninit into functions Simon Glass
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2018-11-26  3:14 UTC (permalink / raw)
  To: u-boot

At present a few error conditions are not checked. Before refactoring
this code, add some basic checks. Note that this code still leaks memory
in the event of error. This will be tackled after the refactor.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v16:
- Add new patch to check for failure to create objects in selftest

Changes in v15: None
Changes in v14: None
Changes in v13: None
Changes in v12: None
Changes in v11: None
Changes in v9: None
Changes in v7: None
Changes in v5: None
Changes in v4: None
Changes in v3: None

 cmd/bootefi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 3e37805ea13..5be10c9b832 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -532,7 +532,12 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
 						      (uintptr_t)&efi_selftest,
 						      (uintptr_t)&efi_selftest);
+		if (!bootefi_device_path)
+			return CMD_RET_FAILURE;
+
 		bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest");
+		if (!bootefi_image_path)
+			return CMD_RET_FAILURE;
 
 		r = efi_setup_loaded_image(bootefi_device_path,
 					   bootefi_image_path, &image_obj,
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v16 2/4] efi: Split out test init/uninit into functions
  2018-11-26  3:14 [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 1/4] efi: Check for failure to create objects in selftest Simon Glass
@ 2018-11-26  3:14 ` Simon Glass
  2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 3/4] efi: Create a function to set up for running EFI code Simon Glass
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2018-11-26  3:14 UTC (permalink / raw)
  To: u-boot

The functions in bootefi are very long because they mix high-level code
and control with the low-level implementation. To help with this, create
functions which handle preparing for running the test and cleaning up
afterwards.

Also shorten the awfully long variable names here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v16:
- Drop comments about efi_save_gd() being called in efi_init_obj_list()

Changes in v15:
- Add check for return values to bootefi_test_prepare()
- Drop call to efi_save_gd() in bootefi_test_prepare()
- Fix minor checkpatch nit with bracket

Changes in v14:
- Go back to the horrible long variable names

Changes in v13:
- Rebase to efi/efi-next

Changes in v12:
- Rename image to image_prot

Changes in v11: None
Changes in v9:
- Add comments to bootefi_test_prepare() about the memset()s

Changes in v7: None
Changes in v5:
- Drop call to efi_init_obj_list() which is now done in do_bootefi()

Changes in v4: None
Changes in v3:
- Add new patch to split out test init/uninit into functions

 cmd/bootefi.c | 85 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 64 insertions(+), 21 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 5be10c9b832..666d90e4b70 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -453,6 +453,66 @@ exit:
 	return ret;
 }
 
+#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
+/**
+ * bootefi_test_prepare() - prepare to run an EFI test
+ *
+ * This sets things up so we can call EFI functions. This involves preparing
+ * the 'gd' pointer and setting up the load ed image data structures.
+ *
+ * @image_objp: loaded_image_infop: Pointer to a struct which will hold the
+ *    loaded image object. This struct will be inited by this function before
+ *    use.
+ * @loaded_image_infop: Pointer to a struct which will hold the loaded image
+ *    info. This struct will be inited by this function before use.
+ * @path: File path to the test being run (often just the test name with a
+ *    backslash before it
+ * @test_func: Address of the test function that is being run
+ * @return 0 if OK, -ve on error
+ */
+static efi_status_t bootefi_test_prepare
+		(struct efi_loaded_image_obj **image_objp,
+		struct efi_loaded_image **loaded_image_infop,
+		const char *path,
+		ulong test_func)
+{
+	efi_status_t r;
+
+	/* Construct a dummy device path */
+	bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
+					      (uintptr_t)test_func,
+					      (uintptr_t)test_func);
+	if (!bootefi_device_path)
+		return EFI_OUT_OF_RESOURCES;
+	bootefi_image_path = efi_dp_from_file(NULL, 0, path);
+	if (!bootefi_image_path)
+		return EFI_OUT_OF_RESOURCES;
+	r = efi_setup_loaded_image(bootefi_device_path, bootefi_image_path,
+				   image_objp, loaded_image_infop);
+	if (r)
+		return r;
+
+	/* Transfer environment variable efi_selftest as load options */
+	set_load_options(*loaded_image_infop, "efi_selftest");
+
+	return 0;
+}
+
+/**
+ * bootefi_test_finish() - finish up after running an EFI test
+ *
+ * @image_obj: Pointer to a struct which holds the loaded image object
+ * @loaded_image_info: Pointer to a struct which holds the loaded image info
+ */
+static void bootefi_test_finish(struct efi_loaded_image_obj *image_obj,
+				struct efi_loaded_image *loaded_image_info)
+{
+	efi_restore_gd();
+	free(loaded_image_info->load_options);
+	efi_delete_handle(&image_obj->header);
+}
+#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
+
 static int do_bootefi_bootmgr_exec(void)
 {
 	struct efi_device_path *device_path, *file_path;
@@ -528,31 +588,14 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		struct efi_loaded_image_obj *image_obj;
 		struct efi_loaded_image *loaded_image_info;
 
-		/* Construct a dummy device path. */
-		bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
-						      (uintptr_t)&efi_selftest,
-						      (uintptr_t)&efi_selftest);
-		if (!bootefi_device_path)
-			return CMD_RET_FAILURE;
-
-		bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest");
-		if (!bootefi_image_path)
+		if (bootefi_test_prepare(&image_obj, &loaded_image_info,
+					 "\\selftest",
+					 (uintptr_t)&efi_selftest))
 			return CMD_RET_FAILURE;
 
-		r = efi_setup_loaded_image(bootefi_device_path,
-					   bootefi_image_path, &image_obj,
-					   &loaded_image_info);
-		if (r != EFI_SUCCESS)
-			return CMD_RET_FAILURE;
-
-		efi_save_gd();
-		/* Transfer environment variable efi_selftest as load options */
-		set_load_options(loaded_image_info, "efi_selftest");
 		/* Execute the test */
 		r = efi_selftest(&image_obj->header, &systab);
-		efi_restore_gd();
-		free(loaded_image_info->load_options);
-		efi_delete_handle(&image_obj->header);
+		bootefi_test_finish(image_obj, loaded_image_info);
 		return r != EFI_SUCCESS;
 	} else
 #endif
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v16 3/4] efi: Create a function to set up for running EFI code
  2018-11-26  3:14 [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 1/4] efi: Check for failure to create objects in selftest Simon Glass
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 2/4] efi: Split out test init/uninit into functions Simon Glass
@ 2018-11-26  3:14 ` Simon Glass
  2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 4/4] efi: Rename bootefi_test_finish() to bootefi_run_finish() Simon Glass
  2018-11-29 21:26 ` [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
  4 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2018-11-26  3:14 UTC (permalink / raw)
  To: u-boot

There is still duplicated code in efi_loader for tests and normal
operation.

Add a new bootefi_run_prepare() function which holds common code used to
set up U-Boot to run EFI code. Make use of this from the existing
bootefi_test_prepare() function, as well as do_bootefi_exec().

Also shorten a few variable names.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v16:
- Drop comments about efi_save_gd() being called in efi_init_obj_list()

Changes in v15:
- Fix minor checkpatch nit with bracket

Changes in v14:
- Go back to the horrible long variable names

Changes in v13: None
Changes in v12: None
Changes in v11: None
Changes in v9: None
Changes in v7: None
Changes in v5:
- Drop call to efi_init_obj_list() which is now done in do_bootefi()
- Introduce load_options_path to specifyc U-Boot env var for load_options_path

Changes in v4:
- Rebase to master

Changes in v3:
- Add patch to create a function to set up for running EFI code

 cmd/bootefi.c | 52 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 666d90e4b70..ca6693a245e 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -325,6 +325,25 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
 	return ret;
 }
 
+static efi_status_t bootefi_run_prepare(const char *load_options_path,
+		struct efi_device_path *device_path,
+		struct efi_device_path *image_path,
+		struct efi_loaded_image_obj **image_objp,
+		struct efi_loaded_image **loaded_image_infop)
+{
+	efi_status_t ret;
+
+	ret = efi_setup_loaded_image(device_path, image_path, image_objp,
+				     loaded_image_infop);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	/* Transfer environment variable as load options */
+	set_load_options(*loaded_image_infop, load_options_path);
+
+	return 0;
+}
+
 /**
  * do_bootefi_exec() - execute EFI binary
  *
@@ -374,13 +393,11 @@ static efi_status_t do_bootefi_exec(void *efi,
 		assert(device_path && image_path);
 	}
 
-	ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
-				     &loaded_image_info);
-	if (ret != EFI_SUCCESS)
-		goto exit;
+	ret = bootefi_run_prepare("bootargs", device_path, image_path,
+				  &image_obj, &loaded_image_info);
+	if (ret)
+		return ret;
 
-	/* Transfer environment variable bootargs as load options */
-	set_load_options(loaded_image_info, "bootargs");
 	/* Load the EFI payload */
 	entry = efi_load_pe(image_obj, efi, loaded_image_info);
 	if (!entry) {
@@ -468,16 +485,14 @@ exit:
  * @path: File path to the test being run (often just the test name with a
  *    backslash before it
  * @test_func: Address of the test function that is being run
+ * @load_options_path: U-Boot environment variable to use as load options
  * @return 0 if OK, -ve on error
  */
 static efi_status_t bootefi_test_prepare
 		(struct efi_loaded_image_obj **image_objp,
-		struct efi_loaded_image **loaded_image_infop,
-		const char *path,
-		ulong test_func)
+		struct efi_loaded_image **loaded_image_infop, const char *path,
+		ulong test_func, const char *load_options_path)
 {
-	efi_status_t r;
-
 	/* Construct a dummy device path */
 	bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
 					      (uintptr_t)test_func,
@@ -487,15 +502,10 @@ static efi_status_t bootefi_test_prepare
 	bootefi_image_path = efi_dp_from_file(NULL, 0, path);
 	if (!bootefi_image_path)
 		return EFI_OUT_OF_RESOURCES;
-	r = efi_setup_loaded_image(bootefi_device_path, bootefi_image_path,
-				   image_objp, loaded_image_infop);
-	if (r)
-		return r;
 
-	/* Transfer environment variable efi_selftest as load options */
-	set_load_options(*loaded_image_infop, "efi_selftest");
-
-	return 0;
+	return bootefi_run_prepare(load_options_path, bootefi_device_path,
+				   bootefi_image_path, image_objp,
+				   loaded_image_infop);
 }
 
 /**
@@ -589,8 +599,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		struct efi_loaded_image *loaded_image_info;
 
 		if (bootefi_test_prepare(&image_obj, &loaded_image_info,
-					 "\\selftest",
-					 (uintptr_t)&efi_selftest))
+					 "\\selftest", (uintptr_t)&efi_selftest,
+					 "efi_selftest"))
 			return CMD_RET_FAILURE;
 
 		/* Execute the test */
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v16 4/4] efi: Rename bootefi_test_finish() to bootefi_run_finish()
  2018-11-26  3:14 [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
                   ` (2 preceding siblings ...)
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 3/4] efi: Create a function to set up for running EFI code Simon Glass
@ 2018-11-26  3:14 ` Simon Glass
  2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
  2018-11-29 21:26 ` [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
  4 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2018-11-26  3:14 UTC (permalink / raw)
  To: u-boot

This function can be used from do_bootefi_exec() so that we use mostly the
same code for a normal EFI application and an EFI test.

Rename the function and use it in both places.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v16: None
Changes in v15:
- Add a comment about a leaked device path
- Drop patch already applied

Changes in v14:
- Go back to the horrible long variable names
- Hopefully correct error paths in do_bootefi_exec()

Changes in v13:
- Drop 'efi_loader: Drop setup_ok' as we have an existing patch for that
- Drop patches previously applied

Changes in v12: None
Changes in v11:
- Drop patches previously applied

Changes in v9: None
Changes in v7:
- Drop patch "efi: Init the 'rows' and 'cols' variables"
- Drop patches previous applied

Changes in v5:
- Rebase to master

Changes in v4:
- Rebase to master

Changes in v3:
- Add new patch to rename bootefi_test_finish() to bootefi_run_finish()

 cmd/bootefi.c | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index ca6693a245e..eadfd934b73 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -344,6 +344,20 @@ static efi_status_t bootefi_run_prepare(const char *load_options_path,
 	return 0;
 }
 
+/**
+ * bootefi_run_finish() - finish up after running an EFI test
+ *
+ * @loaded_image_info: Pointer to a struct which holds the loaded image info
+ * @image_objj: Pointer to a struct which holds the loaded image object
+ */
+static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj,
+			       struct efi_loaded_image *loaded_image_info)
+{
+	efi_restore_gd();
+	free(loaded_image_info->load_options);
+	efi_delete_handle(&image_obj->header);
+}
+
 /**
  * do_bootefi_exec() - execute EFI binary
  *
@@ -384,11 +398,11 @@ static efi_status_t do_bootefi_exec(void *efi,
 		 */
 		ret = efi_create_handle(&mem_handle);
 		if (ret != EFI_SUCCESS)
-			goto exit;
+			return ret; /* TODO: leaks device_path */
 		ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
 				       device_path);
 		if (ret != EFI_SUCCESS)
-			goto exit;
+			goto err_add_protocol;
 	} else {
 		assert(device_path && image_path);
 	}
@@ -396,13 +410,13 @@ static efi_status_t do_bootefi_exec(void *efi,
 	ret = bootefi_run_prepare("bootargs", device_path, image_path,
 				  &image_obj, &loaded_image_info);
 	if (ret)
-		return ret;
+		goto err_prepare;
 
 	/* Load the EFI payload */
 	entry = efi_load_pe(image_obj, efi, loaded_image_info);
 	if (!entry) {
 		ret = EFI_LOAD_ERROR;
-		goto exit;
+		goto err_prepare;
 	}
 
 	if (memdp) {
@@ -422,7 +436,7 @@ static efi_status_t do_bootefi_exec(void *efi,
 
 	if (setjmp(&image_obj->exit_jmp)) {
 		ret = image_obj->exit_status;
-		goto exit;
+		goto err_prepare;
 	}
 
 #ifdef CONFIG_ARM64
@@ -460,10 +474,11 @@ static efi_status_t do_bootefi_exec(void *efi,
 
 	ret = efi_do_enter(&image_obj->header, &systab, entry);
 
-exit:
+err_prepare:
 	/* image has returned, loaded-image obj goes *poof*: */
-	if (image_obj)
-		efi_delete_handle(&image_obj->header);
+	bootefi_run_finish(image_obj, loaded_image_info);
+
+err_add_protocol:
 	if (mem_handle)
 		efi_delete_handle(mem_handle);
 
@@ -508,19 +523,6 @@ static efi_status_t bootefi_test_prepare
 				   loaded_image_infop);
 }
 
-/**
- * bootefi_test_finish() - finish up after running an EFI test
- *
- * @image_obj: Pointer to a struct which holds the loaded image object
- * @loaded_image_info: Pointer to a struct which holds the loaded image info
- */
-static void bootefi_test_finish(struct efi_loaded_image_obj *image_obj,
-				struct efi_loaded_image *loaded_image_info)
-{
-	efi_restore_gd();
-	free(loaded_image_info->load_options);
-	efi_delete_handle(&image_obj->header);
-}
 #endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
 
 static int do_bootefi_bootmgr_exec(void)
@@ -605,7 +607,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 		/* Execute the test */
 		r = efi_selftest(&image_obj->header, &systab);
-		bootefi_test_finish(image_obj, loaded_image_info);
+		bootefi_run_finish(image_obj, loaded_image_info);
 		return r != EFI_SUCCESS;
 	} else
 #endif
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement
  2018-11-26  3:14 [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
                   ` (3 preceding siblings ...)
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 4/4] efi: Rename bootefi_test_finish() to bootefi_run_finish() Simon Glass
@ 2018-11-29 21:26 ` Simon Glass
  2018-11-30 21:27   ` Alexander Graf
  4 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2018-11-29 21:26 UTC (permalink / raw)
  To: u-boot

Hi Alex,

On Sun, 25 Nov 2018 at 20:14, Simon Glass <sjg@chromium.org> wrote:
>
> This collects the patches previously sent to break up the very large
> functions in efi_loader into smaller pieces. Now that the other sandbox
> stuff is applied, perhaps it is time to apply these patches.
>
> This also adds a few new patches to fix more recent breakages.
> Unfortunately we still cannot enable the efi loader tests since one of
> the tests fails. Thus we should expect additional failures to appear
> until that is resolved.
>
> Changes in v16:
> - Add new patch to check for failure to create objects in selftest
> - Drop comments about efi_save_gd() being called in efi_init_obj_list()
>
> Changes in v15:
> - Add a comment about a leaked device path
> - Add check for return values to bootefi_test_prepare()
> - Drop call to efi_save_gd() in bootefi_test_prepare()
> - Drop patch already applied
> - Fix minor checkpatch nit with bracket
>
> Changes in v14:
> - Go back to the horrible long variable names
> - Hopefully correct error paths in do_bootefi_exec()
>
> Changes in v13:
> - Drop 'efi_loader: Drop setup_ok' as we have an existing patch for that
> - Drop patches previously applied
> - Rebase to efi/efi-next
>
> Changes in v12:
> - Rename image to image_prot
>
> Changes in v11:
> - Drop patches previously applied
>
> Changes in v9:
> - Add comments to bootefi_test_prepare() about the memset()s
>
> Changes in v7:
> - Drop patch "efi: Init the 'rows' and 'cols' variables"
> - Drop patches previous applied
>
> Changes in v5:
> - Drop call to efi_init_obj_list() which is now done in do_bootefi()
> - Introduce load_options_path to specifyc U-Boot env var for load_options_path
> - Rebase to master
>
> Changes in v4:
> - Rebase to master
>
> Changes in v3:
> - Add new patch to rename bootefi_test_finish() to bootefi_run_finish()
> - Add new patch to split out test init/uninit into functions
> - Add patch to create a function to set up for running EFI code
>
> Simon Glass (4):
>   efi: Check for failure to create objects in selftest
>   efi: Split out test init/uninit into functions
>   efi: Create a function to set up for running EFI code
>   efi: Rename bootefi_test_finish() to bootefi_run_finish()
>
>  cmd/bootefi.c | 118 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 89 insertions(+), 29 deletions(-)

Any thoughts on this latest version?

Regards,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement
  2018-11-29 21:26 ` [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
@ 2018-11-30 21:27   ` Alexander Graf
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2018-11-30 21:27 UTC (permalink / raw)
  To: u-boot



On 29.11.18 22:26, Simon Glass wrote:
> Hi Alex,
> 
> On Sun, 25 Nov 2018 at 20:14, Simon Glass <sjg@chromium.org> wrote:
>>
>> This collects the patches previously sent to break up the very large
>> functions in efi_loader into smaller pieces. Now that the other sandbox
>> stuff is applied, perhaps it is time to apply these patches.
>>
>> This also adds a few new patches to fix more recent breakages.
>> Unfortunately we still cannot enable the efi loader tests since one of
>> the tests fails. Thus we should expect additional failures to appear
>> until that is resolved.
>>
>> Changes in v16:
>> - Add new patch to check for failure to create objects in selftest
>> - Drop comments about efi_save_gd() being called in efi_init_obj_list()
>>
>> Changes in v15:
>> - Add a comment about a leaked device path
>> - Add check for return values to bootefi_test_prepare()
>> - Drop call to efi_save_gd() in bootefi_test_prepare()
>> - Drop patch already applied
>> - Fix minor checkpatch nit with bracket
>>
>> Changes in v14:
>> - Go back to the horrible long variable names
>> - Hopefully correct error paths in do_bootefi_exec()
>>
>> Changes in v13:
>> - Drop 'efi_loader: Drop setup_ok' as we have an existing patch for that
>> - Drop patches previously applied
>> - Rebase to efi/efi-next
>>
>> Changes in v12:
>> - Rename image to image_prot
>>
>> Changes in v11:
>> - Drop patches previously applied
>>
>> Changes in v9:
>> - Add comments to bootefi_test_prepare() about the memset()s
>>
>> Changes in v7:
>> - Drop patch "efi: Init the 'rows' and 'cols' variables"
>> - Drop patches previous applied
>>
>> Changes in v5:
>> - Drop call to efi_init_obj_list() which is now done in do_bootefi()
>> - Introduce load_options_path to specifyc U-Boot env var for load_options_path
>> - Rebase to master
>>
>> Changes in v4:
>> - Rebase to master
>>
>> Changes in v3:
>> - Add new patch to rename bootefi_test_finish() to bootefi_run_finish()
>> - Add new patch to split out test init/uninit into functions
>> - Add patch to create a function to set up for running EFI code
>>
>> Simon Glass (4):
>>   efi: Check for failure to create objects in selftest
>>   efi: Split out test init/uninit into functions
>>   efi: Create a function to set up for running EFI code
>>   efi: Rename bootefi_test_finish() to bootefi_run_finish()
>>
>>  cmd/bootefi.c | 118 +++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 89 insertions(+), 29 deletions(-)
> 
> Any thoughts on this latest version?

You're still breaking bisectability rules (functional changes in code
shuffling). But I'm getting tired of all the back and forth, so let's
just cross our fingers that we never have to revert any of these patches.

Applied them all.


Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v16, 4/4] efi: Rename bootefi_test_finish() to bootefi_run_finish()
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 4/4] efi: Rename bootefi_test_finish() to bootefi_run_finish() Simon Glass
@ 2018-11-30 23:21   ` Alexander Graf
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2018-11-30 23:21 UTC (permalink / raw)
  To: u-boot

> This function can be used from do_bootefi_exec() so that we use mostly the
> same code for a normal EFI application and an EFI test.
> 
> Rename the function and use it in both places.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v16, 2/4] efi: Split out test init/uninit into functions
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 2/4] efi: Split out test init/uninit into functions Simon Glass
@ 2018-11-30 23:21   ` Alexander Graf
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2018-11-30 23:21 UTC (permalink / raw)
  To: u-boot

> The functions in bootefi are very long because they mix high-level code
> and control with the low-level implementation. To help with this, create
> functions which handle preparing for running the test and cleaning up
> afterwards.
> 
> Also shorten the awfully long variable names here.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v16, 1/4] efi: Check for failure to create objects in selftest
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 1/4] efi: Check for failure to create objects in selftest Simon Glass
@ 2018-11-30 23:21   ` Alexander Graf
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2018-11-30 23:21 UTC (permalink / raw)
  To: u-boot

> At present a few error conditions are not checked. Before refactoring
> this code, add some basic checks. Note that this code still leaks memory
> in the event of error. This will be tackled after the refactor.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v16, 3/4] efi: Create a function to set up for running EFI code
  2018-11-26  3:14 ` [U-Boot] [PATCH v16 3/4] efi: Create a function to set up for running EFI code Simon Glass
@ 2018-11-30 23:21   ` Alexander Graf
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2018-11-30 23:21 UTC (permalink / raw)
  To: u-boot

> There is still duplicated code in efi_loader for tests and normal
> operation.
> 
> Add a new bootefi_run_prepare() function which holds common code used to
> set up U-Boot to run EFI code. Make use of this from the existing
> bootefi_test_prepare() function, as well as do_bootefi_exec().
> 
> Also shorten a few variable names.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-30 23:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26  3:14 [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
2018-11-26  3:14 ` [U-Boot] [PATCH v16 1/4] efi: Check for failure to create objects in selftest Simon Glass
2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
2018-11-26  3:14 ` [U-Boot] [PATCH v16 2/4] efi: Split out test init/uninit into functions Simon Glass
2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
2018-11-26  3:14 ` [U-Boot] [PATCH v16 3/4] efi: Create a function to set up for running EFI code Simon Glass
2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
2018-11-26  3:14 ` [U-Boot] [PATCH v16 4/4] efi: Rename bootefi_test_finish() to bootefi_run_finish() Simon Glass
2018-11-30 23:21   ` [U-Boot] [U-Boot, v16, " Alexander Graf
2018-11-29 21:26 ` [U-Boot] [PATCH v16 0/4] efi_loader: Code refactoring and improvement Simon Glass
2018-11-30 21:27   ` Alexander Graf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.