Re: [PATCH v2 1/2] efi_loader: fix set_capsule_result()

NAK.

On Wed, Jun 30, 2021 at 05:31:15PM +0200, Heinrich Schuchardt wrote:

The log category must be LOG_CATEGORY LOGC_EFI.

efi_set_variable() should be called with EFI_CALL(). Use efi_set_variable_int() instead.

A log text "Updating ..." if SetVariable() fails does not make sense for a variable that is not required to be preexisting.

CapsuleLast should always be immediately updated.

As I said to your v1 in [1],

You are trying to fix several irrelevant issues here. Please split them into separate patches as you have always asked me before.

[1] https://lists.denx.de/pipermail/u-boot/2021-June/453148.html

-Takahiro Akashi

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

v2: don't update OsIndications in set_capsule_result() update CapsuleLast immediately

---

lib/efi_loader/efi_capsule.c | 39 +++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 2c37a0d97b..f87ef2a514 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -6,6 +6,8 @@

• 	Author: AKASHI Takahiro
  

*/

+#define LOG_CATEGORY LOGC_EFI

#include <common.h> #include <efi_loader.h> #include <efi_variable.h> @@ -95,13 +97,25 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule, else memset(&result.capsule_processed, 0, sizeof(time)); result.capsule_status = return_status;

• ret = efi_set_variable(variable_name16, &efi_guid_capsule_report,

• 	       EFI_VARIABLE_NON_VOLATILE |
  

• 	       EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

• 	       EFI_VARIABLE_RUNTIME_ACCESS,
  

• 	       sizeof(result), &result);
  

• if (ret)

• log_err("EFI: creating %ls failed\n", variable_name16);
  

• ret = efi_set_variable_int(variable_name16, &efi_guid_capsule_report,

• 		   EFI_VARIABLE_NON_VOLATILE |
  

• 		   EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

• 		   EFI_VARIABLE_RUNTIME_ACCESS,
  

• 		   sizeof(result), &result, false);
  

• if (ret != EFI_SUCCESS) {

• log_err("Setting %ls failed\n", variable_name16);
  

• return;
  

• }
• /* Variable CapsuleLast must not include terminating 0x0000 */
• ret = efi_set_variable_int(L"CapsuleLast", &efi_guid_capsule_report,

• 		   EFI_VARIABLE_READ_ONLY |
  

• 		   EFI_VARIABLE_NON_VOLATILE |
  

• 		   EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

• 		   EFI_VARIABLE_RUNTIME_ACCESS,
  

• 		   22, variable_name16, false);
  

• if (ret != EFI_SUCCESS)

• log_err("Setting %ls failed\n", L"CapsuleLast");
  

}

#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT @@ -988,7 +1002,6 @@ efi_status_t efi_launch_capsules(void) struct efi_capsule_header *capsule = NULL; u16 **files; unsigned int nfiles, index, i;

• u16 variable_name16[12]; efi_status_t ret;

  if (!check_run_capsules())

@@ -1045,16 +1058,6 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files);

• /* CapsuleLast */
• efi_create_indexed_name(variable_name16, sizeof(variable_name16),

• 		"Capsule", index - 1);
  

• efi_set_variable_int(L"CapsuleLast", &efi_guid_capsule_report,

• 	     EFI_VARIABLE_READ_ONLY |
  

• 	     EFI_VARIABLE_NON_VOLATILE |
  

• 	     EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

• 	     EFI_VARIABLE_RUNTIME_ACCESS,
  

• 	     22, variable_name16, false);
  

• return ret;

}

#endif /* CONFIG_EFI_CAPSULE_ON_DISK */

2.30.2