* [PATCH 1/1] efi_loader: fix set_capsule_result()
@ 2021-06-29 7:40 Heinrich Schuchardt
2021-06-29 8:59 ` Ilias Apalodimas
2021-06-29 12:26 ` AKASHI Takahiro
0 siblings, 2 replies; 3+ messages in thread
From: Heinrich Schuchardt @ 2021-06-29 7:40 UTC (permalink / raw)
To: u-boot
Cc: Alexander Graf, AKASHI Takahiro, Ilias Apalodimas, Sughosh Ganu,
Heinrich Schuchardt
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.
Flag EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be reset in
variable OsIndications.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_loader/efi_capsule.c | 38 +++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 60309d4a07..f7c0c5725f 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>
@@ -84,6 +86,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
struct efi_capsule_result_variable_header result;
struct efi_time time;
efi_status_t ret;
+ efi_uintn_t size;
+ u64 os_indications;
efi_create_indexed_name(variable_name16, sizeof(variable_name16),
"Capsule", index);
@@ -95,13 +99,33 @@ 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);
+
+ if (return_status != EFI_SUCCESS)
+ return;
+
+ 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");
}
#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] efi_loader: fix set_capsule_result()
2021-06-29 7:40 [PATCH 1/1] efi_loader: fix set_capsule_result() Heinrich Schuchardt
@ 2021-06-29 8:59 ` Ilias Apalodimas
2021-06-29 12:26 ` AKASHI Takahiro
1 sibling, 0 replies; 3+ messages in thread
From: Ilias Apalodimas @ 2021-06-29 8:59 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot, Alexander Graf, AKASHI Takahiro, Sughosh Ganu
On Tue, Jun 29, 2021 at 09:40:21AM +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.
>
> Flag EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be reset in
> variable OsIndications.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_loader/efi_capsule.c | 38 +++++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 60309d4a07..f7c0c5725f 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>
> @@ -84,6 +86,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
> struct efi_capsule_result_variable_header result;
> struct efi_time time;
> efi_status_t ret;
> + efi_uintn_t size;
> + u64 os_indications;
>
> efi_create_indexed_name(variable_name16, sizeof(variable_name16),
> "Capsule", index);
> @@ -95,13 +99,33 @@ 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);
> +
> + if (return_status != EFI_SUCCESS)
> + return;
> +
The spec for capsule updates delivered on mass storage devices says:
"This bit will be cleared from OsIndications by system firmware in all
cases during processing following reboot."
So shouldn't we clear the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
bit even if the update failed?
> + 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");
> }
>
> #ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] efi_loader: fix set_capsule_result()
2021-06-29 7:40 [PATCH 1/1] efi_loader: fix set_capsule_result() Heinrich Schuchardt
2021-06-29 8:59 ` Ilias Apalodimas
@ 2021-06-29 12:26 ` AKASHI Takahiro
1 sibling, 0 replies; 3+ messages in thread
From: AKASHI Takahiro @ 2021-06-29 12:26 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: u-boot, Alexander Graf, Ilias Apalodimas, Sughosh Ganu
On Tue, Jun 29, 2021 at 09:40:21AM +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.
>
> Flag EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED must be reset in
> variable OsIndications.
You are trying to fix several irrelevant issues here.
Please split them into separate patches as you have always
asked me before.
-Takahiro Akashi
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_loader/efi_capsule.c | 38 +++++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 60309d4a07..f7c0c5725f 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>
> @@ -84,6 +86,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
> struct efi_capsule_result_variable_header result;
> struct efi_time time;
> efi_status_t ret;
> + efi_uintn_t size;
> + u64 os_indications;
>
> efi_create_indexed_name(variable_name16, sizeof(variable_name16),
> "Capsule", index);
> @@ -95,13 +99,33 @@ 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);
> +
> + if (return_status != EFI_SUCCESS)
> + return;
> +
> + 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");
> }
>
> #ifdef CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-29 12:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-29 7:40 [PATCH 1/1] efi_loader: fix set_capsule_result() Heinrich Schuchardt
2021-06-29 8:59 ` Ilias Apalodimas
2021-06-29 12:26 ` AKASHI Takahiro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox