| Message ID | 20210701082049.68150-2-xypron.glpk@gmx.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: indicating capsule update results | expand |
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 >
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 */
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(-) -- 2.30.2