[PATCH v3 0/2] efi_loader: indicating capsule update results

When creating the Capsule#### variable we should immediately update CapsuleLast.
After each reboot we must clear flag EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED in variable OsIndications.
v3: clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED even after capsule update failure
Heinrich Schuchardt (2): efi_loader: fix set_capsule_result() efi_loader: clear OsIndications
lib/efi_loader/efi_capsule.c | 39 +++++++++++++++++++---------------- lib/efi_loader/efi_setup.c | 40 +++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 19 deletions(-)
-- 2.30.2

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.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3: no change --- 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

NAK again with the same reason that I gave on v1/v2. Please follow your own rules that you imposed on me.
-Takahiro Akashi
On Thu, Jul 01, 2021 at 10:20:48AM +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.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
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

After each reboot we must clear flag EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED in variable OsIndications.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3: always reset EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED --- lib/efi_loader/efi_setup.c | 40 +++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index 3c5cf9a435..cf6e768812 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -5,9 +5,12 @@ * Copyright (c) 2016-2018 Alexander Graf et al. */
+#define LOG_CATEGORY LOGC_EFI + #include <common.h> #include <efi_loader.h> #include <efi_variable.h> +#include <log.h>
#define OBJ_LIST_NOT_INITIALIZED 1
@@ -171,6 +174,37 @@ static efi_status_t efi_init_os_indications(void) &os_indications_supported, false); }
+ +/** + * efi_clear_os_indications() - clear OsIndications + * + * Clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED + */ +static efi_status_t efi_clear_os_indications(void) +{ + efi_uintn_t size; + u64 os_indications; + efi_status_t ret; + + size = sizeof(os_indications); + ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid, + NULL, &size, &os_indications, NULL); + if (ret != EFI_SUCCESS) + os_indications = 0; + else + os_indications &= + ~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED; + ret = efi_set_variable_int(L"OsIndications", &efi_global_variable_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + sizeof(os_indications), &os_indications, + false); + if (ret != EFI_SUCCESS) + log_err("Setting %ls failed\n", L"OsIndications"); + return ret; +} + /** * efi_init_obj_list() - Initialize and populate EFI object list * @@ -178,7 +212,7 @@ static efi_status_t efi_init_os_indications(void) */ efi_status_t efi_init_obj_list(void) { - efi_status_t ret = EFI_SUCCESS; + efi_status_t r, ret = EFI_SUCCESS;
/* Initialize once only */ if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) @@ -291,7 +325,11 @@ efi_status_t efi_init_obj_list(void) if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) && !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY)) ret = efi_launch_capsules(); + out: + r = efi_clear_os_indications(); + if (ret == EFI_SUCCESS) + ret = r; efi_obj_list_initialized = ret; return ret; } -- 2.30.2
participants (2)
-
AKASHI Takahiro
-
Heinrich Schuchardt