diff mbox series

[v1,2/4] efi_loader: Add OS notifications for SetVariableRT in RAM

Message ID 20240406140203.248211-3-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series Enable SetvariableRT | expand

Commit Message

Ilias Apalodimas April 6, 2024, 2:01 p.m. UTC 
Previous patches enable SetVariableRT using a volatile storage backend
using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's no
recommendation from the spec on how to notify the OS, add a volatile
EFI variable that contains the filename relative to the ESP. OS'es
can use that file and update it at runtime

$~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile
GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c
Name: "RTStorageVolatile"
Attributes:
	Boot Service Access
	Runtime Service Access
Value:
00000000  75 62 6f 6f 74 65 66 69  2e 76 61 72 00           |ubootefi.var.   |

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 include/efi_loader.h          |  4 ++++
 lib/efi_loader/efi_runtime.c  |  4 ----
 lib/efi_loader/efi_variable.c | 20 ++++++++++++++++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

Comments

Heinrich Schuchardt April 8, 2024, 6:18 a.m. UTC | #1 
On 4/6/24 16:01, Ilias Apalodimas wrote:
> Previous patches enable SetVariableRT using a volatile storage backend
> using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's no
> recommendation from the spec on how to notify the OS, add a volatile
> EFI variable that contains the filename relative to the ESP. OS'es
> can use that file and update it at runtime
>
> $~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile
> GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c
> Name: "RTStorageVolatile"
> Attributes:
> 	Boot Service Access
> 	Runtime Service Access
> Value:
> 00000000  75 62 6f 6f 74 65 66 69  2e 76 61 72 00           |ubootefi.var.   |
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   include/efi_loader.h          |  4 ++++
>   lib/efi_loader/efi_runtime.c  |  4 ----
>   lib/efi_loader/efi_variable.c | 20 ++++++++++++++++++++
>   3 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 7daca0afba2b..25551fe18948 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -159,6 +159,10 @@ static inline void efi_set_bootdev(const char *dev, const char *devnr,
>   #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \
>   	EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
>   		 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
> +#define U_BOOT_EFI_RT_VAR_FILE_GUID \
> +	EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \
> +		 0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c)
> +
>
>   /* Use internal device tree when starting UEFI application */
>   #define EFI_FDT_USE_INTERNAL NULL
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 8ebbea7e5c69..d898ba6c268f 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -127,10 +127,6 @@ efi_status_t efi_init_runtime_supported(void)
>   				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
>   				EFI_RT_SUPPORTED_CONVERT_POINTER;
>
> -	if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
> -		rt_table->runtime_services_supported |=
> -			EFI_RT_SUPPORTED_SET_VARIABLE;
> -

Why do you want to remove this flag?

Best regards

Heinrich

>   	/*
>   	 * This value must be synced with efi_runtime_detach_list
>   	 * as well as efi_runtime_services.
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index f79041e6bedd..f97c8c57f75c 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -554,6 +554,26 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
>    */
>   void efi_variables_boot_exit_notify(void)
>   {
> +	const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
> +	const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID;
> +	efi_status_t ret;
> +
> +	if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> +		struct efi_rt_properties_table *rt_prop =
> +			efi_get_configuration_table(&rt_prop_guid);
> +
> +		ret = efi_set_variable_int(u"RTStorageVolatile",
> +					   &efi_guid_efi_rt_var_file,
> +					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					   EFI_VARIABLE_RUNTIME_ACCESS |
> +					   EFI_VARIABLE_READ_ONLY,
> +					   sizeof(EFI_VAR_FILE_NAME),
> +					   EFI_VAR_FILE_NAME, false);
> +		if (ret != EFI_SUCCESS)
> +			rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;
> +		else
> +			log_err("Can't RTStorage. SetVariableRT won't be available\n");
> +	}
>   	/* Switch variable services functions to runtime version */
>   	efi_runtime_services.get_variable = efi_get_variable_runtime;
>   	efi_runtime_services.get_next_variable_name =
Ilias Apalodimas April 8, 2024, 6:41 a.m. UTC | #2 
Hi Heinrich,

> > +
> >
> >   /* Use internal device tree when starting UEFI application */
> >   #define EFI_FDT_USE_INTERNAL NULL
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index 8ebbea7e5c69..d898ba6c268f 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -127,10 +127,6 @@ efi_status_t efi_init_runtime_supported(void)
> >                               EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
> >                               EFI_RT_SUPPORTED_CONVERT_POINTER;
> >
> > -     if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
> > -             rt_table->runtime_services_supported |=
> > -                     EFI_RT_SUPPORTED_SET_VARIABLE;
> > -
>
[...]

> Why do you want to remove this flag?
>

I don't, I messed this up during my rebase before sending the patches.

The code in EBS() was supposed to re-enable it.
It's all fixed in patch #3, but this patch needs a change as well
rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;
should be
rt_prop->runtime_services_supported |= EFI_RT_SUPPORTED_SET_VARIABLE;
in efi_variables_boot_exit_notify()

Thanks
/Ilias

> Best regards
>
> Heinrich
>
> >       /*
> >        * This value must be synced with efi_runtime_detach_list
> >        * as well as efi_runtime_services.
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index f79041e6bedd..f97c8c57f75c 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -554,6 +554,26 @@ if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> >    */
> >   void efi_variables_boot_exit_notify(void)
> >   {
> > +     const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
> > +     const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID;
> > +     efi_status_t ret;
> > +
> > +     if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> > +             struct efi_rt_properties_table *rt_prop =
> > +                     efi_get_configuration_table(&rt_prop_guid);
> > +
> > +             ret = efi_set_variable_int(u"RTStorageVolatile",
> > +                                        &efi_guid_efi_rt_var_file,
> > +                                        EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +                                        EFI_VARIABLE_RUNTIME_ACCESS |
> > +                                        EFI_VARIABLE_READ_ONLY,
> > +                                        sizeof(EFI_VAR_FILE_NAME),
> > +                                        EFI_VAR_FILE_NAME, false);
> > +             if (ret != EFI_SUCCESS)
> > +                     rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;
> > +             else
> > +                     log_err("Can't RTStorage. SetVariableRT won't be available\n");
> > +     }
> >       /* Switch variable services functions to runtime version */
> >       efi_runtime_services.get_variable = efi_get_variable_runtime;
> >       efi_runtime_services.get_next_variable_name =
>
diff mbox series

Patch

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 7daca0afba2b..25551fe18948 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -159,6 +159,10 @@  static inline void efi_set_bootdev(const char *dev, const char *devnr,
 #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \
 	EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
 		 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
+#define U_BOOT_EFI_RT_VAR_FILE_GUID \
+	EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \
+		 0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c)
+
 
 /* Use internal device tree when starting UEFI application */
 #define EFI_FDT_USE_INTERNAL NULL
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 8ebbea7e5c69..d898ba6c268f 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -127,10 +127,6 @@  efi_status_t efi_init_runtime_supported(void)
 				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
 				EFI_RT_SUPPORTED_CONVERT_POINTER;
 
-	if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
-		rt_table->runtime_services_supported |=
-			EFI_RT_SUPPORTED_SET_VARIABLE;
-
 	/*
 	 * This value must be synced with efi_runtime_detach_list
 	 * as well as efi_runtime_services.
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index f79041e6bedd..f97c8c57f75c 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -554,6 +554,26 @@  if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
  */
 void efi_variables_boot_exit_notify(void)
 {
+	const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
+	const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID;
+	efi_status_t ret;
+
+	if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
+		struct efi_rt_properties_table *rt_prop =
+			efi_get_configuration_table(&rt_prop_guid);
+
+		ret = efi_set_variable_int(u"RTStorageVolatile",
+					   &efi_guid_efi_rt_var_file,
+					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+					   EFI_VARIABLE_RUNTIME_ACCESS |
+					   EFI_VARIABLE_READ_ONLY,
+					   sizeof(EFI_VAR_FILE_NAME),
+					   EFI_VAR_FILE_NAME, false);
+		if (ret != EFI_SUCCESS)
+			rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;
+		else
+			log_err("Can't RTStorage. SetVariableRT won't be available\n");
+	}
 	/* Switch variable services functions to runtime version */
 	efi_runtime_services.get_variable = efi_get_variable_runtime;
 	efi_runtime_services.get_next_variable_name =