Re: [RFC 11/14] efi_loader: variable: export variables table for runtime access
Akashi-san, On Wed, Mar 18, 2020 at 10:53:45AM +0900, AKASHI Takahiro wrote: > On Tue, Mar 17, 2020 at 08:37:47AM +0100, Heinrich Schuchardt wrote: > > On 3/17/20 3:12 AM, AKASHI Takahiro wrote: > > > There are platforms on which OS's won't be able to access UEFI variables > > > at runtime (i.e. after ExitBootServices). > > > With this patch, all the UEFI variables are exported as a configuration > > > table so as to enable retrieving variables' values from the table, and > > > later modifying them via capsule-on-disk if necessary. > > > > I do not understand why we should expose our internal memory for holding > > UEFI variables to the operating system. This might end up in users > > trying to access the variables directly. > > I think that you somehow misunderstand my code as it never exposes > any "internal memory," although I don't know what it exactly means in > this context. > This configuration table is nothing but a list of data that represent > all the UEFI variables in implementation-agnostic format. > > > I do not understand why we should not keep the pointer in our private > > memory. > > Anyway, this patch naively implements Peter's proposal while I also > submitted another patch[1] that allows HL-OS to use GetVariable > interface directly via *caching*. How are the two approaches going to affect existig tools (i.e efivar --list) to read the variables? > > Since how we should enable accessing UEFI variables at runtime is > one of key issues, I'd rather discuss in boot-arch ML as I suggested > in the cover letter. > I have already re-activated the discussion there[2]. > Please make your comments there for wider audience. > > [1] https://lists.denx.de/pipermail/u-boot/2019-June/371769.html > [2] > https://lists.linaro.org/pipermail/boot-architecture/2020-March/001149.html > Will do Regards /Ilias
Re: [RFC 11/14] efi_loader: variable: export variables table for runtime access
On Tue, 17 Mar 2020 at 07:42, AKASHI Takahiro
wrote:
> There are platforms on which OS's won't be able to access UEFI variables
> at runtime (i.e. after ExitBootServices).
> With this patch, all the UEFI variables are exported as a configuration
> table so as to enable retrieving variables' values from the table, and
> later modifying them via capsule-on-disk if necessary.
>
> The idea is based on Peter's proposal[1] in boot-arch ML.
>
> [1]
> https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
>
> Signed-off-by: AKASHI Takahiro
> ---
> include/efi_loader.h | 7 +++
> lib/efi_loader/Kconfig| 10
> lib/efi_loader/efi_boottime.c | 10
> lib/efi_loader/efi_capsule.c | 4 ++
> lib/efi_loader/efi_variable.c | 109 ++
> 5 files changed, 140 insertions(+)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 79bdf9586d24..93ed5502821c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
> extern const efi_guid_t efi_guid_capsule_report;
> /* GUID of firmware management protocol */
> extern const efi_guid_t efi_guid_firmware_management_protocol;
> +/* GUID of runtime variable access */
> +extern const efi_guid_t efi_guid_variable_storage;
>
> /* GUID of RNG protocol */
> extern const efi_guid_t efi_guid_rng_protocol;
> @@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
> u64 *remaining_variable_storage_size,
> u64 *maximum_variable_size);
>
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +efi_status_t efi_install_variables_table(void);
> +#endif
> +
> /*
> * See section 3.1.3 in the v2.7 UEFI spec for more details on
> * the layout of EFI_LOAD_OPTION. In short it is:
> @@ -683,6 +689,7 @@ struct efi_load_option {
> void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
> unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8
> **data);
> efi_status_t efi_bootmgr_load(efi_handle_t *handle);
> +efi_status_t efi_install_variables_table(void);
>
> /* Capsule update */
> efi_status_t EFIAPI efi_update_capsule(
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 616e2acbe102..edb8d19059ea 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -41,6 +41,16 @@ config EFI_SET_TIME
> Provide the SetTime() runtime service at boottime. This service
> can be used by an EFI application to adjust the real time clock.
>
> +config EFI_VARIABLE_EXPORT
> + bool "Export variables table as configuration table"
> + depends on !EFI_VARIABLE_RUNTIME_ACCESS
> + depends on EFI_CAPSULE_UPDATE_VARIABLE
> + default y
> + help
> + Select this option if you want to export UEFI variables as
> + configuration table so that OS can still get UEFI variable's
> + value.
> +
> config EFI_DEVICE_PATH_TO_TEXT
> bool "Device path to text protocol"
> default y
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index c2a789b4f910..406644e4da67 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t
> image_handle,
> if (ret != EFI_SUCCESS)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> + /* Install variables database as a configuration table */
> + ret = efi_install_variables_table();
> + if (ret != EFI_SUCCESS)
> + return EFI_EXIT(ret);
> +#endif
> + /*
> +* TODO: remove a table after image is terminated.
> +*/
> +
> image_obj->exit_data_size = exit_data_size;
> image_obj->exit_data = exit_data;
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 1293270aea95..6b737aec1b28 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -18,7 +18,11 @@ static const efi_guid_t
> efi_guid_firmware_management_capsule_id =
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> const efi_guid_t efi_guid_firmware_management_protocol =
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
> +#else
> static const efi_guid_t efi_guid_variable_storage =
> EFI_VARIABLE_STORAGE_GUID;
> +#endif
>
Need to remove the static keyword here. The build fails when
CONFIG_EFI_VARIABLE_EXPORT is not defined, with the extern declaration now
being done in efi_loader.h
-sughosh
Re: [RFC 11/14] efi_loader: variable: export variables table for runtime access
On Tue, Mar 17, 2020 at 08:37:47AM +0100, Heinrich Schuchardt wrote:
> On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> > There are platforms on which OS's won't be able to access UEFI variables
> > at runtime (i.e. after ExitBootServices).
> > With this patch, all the UEFI variables are exported as a configuration
> > table so as to enable retrieving variables' values from the table, and
> > later modifying them via capsule-on-disk if necessary.
>
> I do not understand why we should expose our internal memory for holding
> UEFI variables to the operating system. This might end up in users
> trying to access the variables directly.
I think that you somehow misunderstand my code as it never exposes
any "internal memory," although I don't know what it exactly means in
this context.
This configuration table is nothing but a list of data that represent
all the UEFI variables in implementation-agnostic format.
> I do not understand why we should not keep the pointer in our private
> memory.
Anyway, this patch naively implements Peter's proposal while I also
submitted another patch[1] that allows HL-OS to use GetVariable
interface directly via *caching*.
Since how we should enable accessing UEFI variables at runtime is
one of key issues, I'd rather discuss in boot-arch ML as I suggested
in the cover letter.
I have already re-activated the discussion there[2].
Please make your comments there for wider audience.
[1] https://lists.denx.de/pipermail/u-boot/2019-June/371769.html
[2] https://lists.linaro.org/pipermail/boot-architecture/2020-March/001149.html
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> >
> > The idea is based on Peter's proposal[1] in boot-arch ML.
> >
> > [1]
> > https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > include/efi_loader.h | 7 +++
> > lib/efi_loader/Kconfig| 10
> > lib/efi_loader/efi_boottime.c | 10
> > lib/efi_loader/efi_capsule.c | 4 ++
> > lib/efi_loader/efi_variable.c | 109 ++
> > 5 files changed, 140 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 79bdf9586d24..93ed5502821c 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
> > extern const efi_guid_t efi_guid_capsule_report;
> > /* GUID of firmware management protocol */
> > extern const efi_guid_t efi_guid_firmware_management_protocol;
> > +/* GUID of runtime variable access */
> > +extern const efi_guid_t efi_guid_variable_storage;
> >
> > /* GUID of RNG protocol */
> > extern const efi_guid_t efi_guid_rng_protocol;
> > @@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
> > u64 *remaining_variable_storage_size,
> > u64 *maximum_variable_size);
> >
> > +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> > +efi_status_t efi_install_variables_table(void);
> > +#endif
> > +
> > /*
> >* See section 3.1.3 in the v2.7 UEFI spec for more details on
> >* the layout of EFI_LOAD_OPTION. In short it is:
> > @@ -683,6 +689,7 @@ struct efi_load_option {
> > void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
> > unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8
> > **data);
> > efi_status_t efi_bootmgr_load(efi_handle_t *handle);
> > +efi_status_t efi_install_variables_table(void);
> >
> > /* Capsule update */
> > efi_status_t EFIAPI efi_update_capsule(
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 616e2acbe102..edb8d19059ea 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -41,6 +41,16 @@ config EFI_SET_TIME
> > Provide the SetTime() runtime service at boottime. This service
> > can be used by an EFI application to adjust the real time clock.
> >
> > +config EFI_VARIABLE_EXPORT
> > + bool "Export variables table as configuration table"
> > + depends on !EFI_VARIABLE_RUNTIME_ACCESS
> > + depends on EFI_CAPSULE_UPDATE_VARIABLE
> > + default y
> > + help
> > + Select this option if you want to export UEFI variables as
> > + configuration table so that OS can still get UEFI variable's
> > + value.
> > +
> > config EFI_DEVICE_PATH_TO_TEXT
> > bool "Device path to text protocol"
> > default y
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index c2a789b4f910..406644e4da67 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t
> > image_handle,
> > if (ret != EFI_SUCCESS)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> > + /* Install variables database as a configuration table */
> > + ret =
Re: [RFC 11/14] efi_loader: variable: export variables table for runtime access
On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
There are platforms on which OS's won't be able to access UEFI variables
at runtime (i.e. after ExitBootServices).
With this patch, all the UEFI variables are exported as a configuration
table so as to enable retrieving variables' values from the table, and
later modifying them via capsule-on-disk if necessary.
I do not understand why we should expose our internal memory for holding
UEFI variables to the operating system. This might end up in users
trying to access the variables directly.
I do not understand why we should not keep the pointer in our private
memory.
Best regards
Heinrich
The idea is based on Peter's proposal[1] in boot-arch ML.
[1]
https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 7 +++
lib/efi_loader/Kconfig| 10
lib/efi_loader/efi_boottime.c | 10
lib/efi_loader/efi_capsule.c | 4 ++
lib/efi_loader/efi_variable.c | 109 ++
5 files changed, 140 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 79bdf9586d24..93ed5502821c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
extern const efi_guid_t efi_guid_capsule_report;
/* GUID of firmware management protocol */
extern const efi_guid_t efi_guid_firmware_management_protocol;
+/* GUID of runtime variable access */
+extern const efi_guid_t efi_guid_variable_storage;
/* GUID of RNG protocol */
extern const efi_guid_t efi_guid_rng_protocol;
@@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
u64 *remaining_variable_storage_size,
u64 *maximum_variable_size);
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+efi_status_t efi_install_variables_table(void);
+#endif
+
/*
* See section 3.1.3 in the v2.7 UEFI spec for more details on
* the layout of EFI_LOAD_OPTION. In short it is:
@@ -683,6 +689,7 @@ struct efi_load_option {
void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8
**data);
efi_status_t efi_bootmgr_load(efi_handle_t *handle);
+efi_status_t efi_install_variables_table(void);
/* Capsule update */
efi_status_t EFIAPI efi_update_capsule(
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 616e2acbe102..edb8d19059ea 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -41,6 +41,16 @@ config EFI_SET_TIME
Provide the SetTime() runtime service at boottime. This service
can be used by an EFI application to adjust the real time clock.
+config EFI_VARIABLE_EXPORT
+ bool "Export variables table as configuration table"
+ depends on !EFI_VARIABLE_RUNTIME_ACCESS
+ depends on EFI_CAPSULE_UPDATE_VARIABLE
+ default y
+ help
+ Select this option if you want to export UEFI variables as
+ configuration table so that OS can still get UEFI variable's
+ value.
+
config EFI_DEVICE_PATH_TO_TEXT
bool "Device path to text protocol"
default y
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index c2a789b4f910..406644e4da67 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t
image_handle,
if (ret != EFI_SUCCESS)
return EFI_EXIT(EFI_INVALID_PARAMETER);
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+ /* Install variables database as a configuration table */
+ ret = efi_install_variables_table();
+ if (ret != EFI_SUCCESS)
+ return EFI_EXIT(ret);
+#endif
+ /*
+* TODO: remove a table after image is terminated.
+*/
+
image_obj->exit_data_size = exit_data_size;
image_obj->exit_data = exit_data;
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 1293270aea95..6b737aec1b28 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -18,7 +18,11 @@ static const efi_guid_t
efi_guid_firmware_management_capsule_id =
EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
const efi_guid_t efi_guid_firmware_management_protocol =
EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
+#else
static const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
+#endif
/* for file system access */
static struct efi_file_handle *bootdev_root;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index c316bdfec0e4..270e5211f633 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -641,3 +641,112 @@ efi_status_t efi_init_variables(void)
{
[RFC 11/14] efi_loader: variable: export variables table for runtime access
There are platforms on which OS's won't be able to access UEFI variables
at runtime (i.e. after ExitBootServices).
With this patch, all the UEFI variables are exported as a configuration
table so as to enable retrieving variables' values from the table, and
later modifying them via capsule-on-disk if necessary.
The idea is based on Peter's proposal[1] in boot-arch ML.
[1]
https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 7 +++
lib/efi_loader/Kconfig| 10
lib/efi_loader/efi_boottime.c | 10
lib/efi_loader/efi_capsule.c | 4 ++
lib/efi_loader/efi_variable.c | 109 ++
5 files changed, 140 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 79bdf9586d24..93ed5502821c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
extern const efi_guid_t efi_guid_capsule_report;
/* GUID of firmware management protocol */
extern const efi_guid_t efi_guid_firmware_management_protocol;
+/* GUID of runtime variable access */
+extern const efi_guid_t efi_guid_variable_storage;
/* GUID of RNG protocol */
extern const efi_guid_t efi_guid_rng_protocol;
@@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
u64 *remaining_variable_storage_size,
u64 *maximum_variable_size);
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+efi_status_t efi_install_variables_table(void);
+#endif
+
/*
* See section 3.1.3 in the v2.7 UEFI spec for more details on
* the layout of EFI_LOAD_OPTION. In short it is:
@@ -683,6 +689,7 @@ struct efi_load_option {
void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
efi_status_t efi_bootmgr_load(efi_handle_t *handle);
+efi_status_t efi_install_variables_table(void);
/* Capsule update */
efi_status_t EFIAPI efi_update_capsule(
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 616e2acbe102..edb8d19059ea 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -41,6 +41,16 @@ config EFI_SET_TIME
Provide the SetTime() runtime service at boottime. This service
can be used by an EFI application to adjust the real time clock.
+config EFI_VARIABLE_EXPORT
+ bool "Export variables table as configuration table"
+ depends on !EFI_VARIABLE_RUNTIME_ACCESS
+ depends on EFI_CAPSULE_UPDATE_VARIABLE
+ default y
+ help
+ Select this option if you want to export UEFI variables as
+ configuration table so that OS can still get UEFI variable's
+ value.
+
config EFI_DEVICE_PATH_TO_TEXT
bool "Device path to text protocol"
default y
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index c2a789b4f910..406644e4da67 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t
image_handle,
if (ret != EFI_SUCCESS)
return EFI_EXIT(EFI_INVALID_PARAMETER);
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+ /* Install variables database as a configuration table */
+ ret = efi_install_variables_table();
+ if (ret != EFI_SUCCESS)
+ return EFI_EXIT(ret);
+#endif
+ /*
+* TODO: remove a table after image is terminated.
+*/
+
image_obj->exit_data_size = exit_data_size;
image_obj->exit_data = exit_data;
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 1293270aea95..6b737aec1b28 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -18,7 +18,11 @@ static const efi_guid_t
efi_guid_firmware_management_capsule_id =
EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
const efi_guid_t efi_guid_firmware_management_protocol =
EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
+#else
static const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
+#endif
/* for file system access */
static struct efi_file_handle *bootdev_root;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index c316bdfec0e4..270e5211f633 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -641,3 +641,112 @@ efi_status_t efi_init_variables(void)
{
return EFI_SUCCESS;
}
+
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+/**
+ * efi_install_variables_table() - install variables table
+ *
+ * This function adds a configuration table that contains all the cache
+ * data of UEFI variables for runtime access.
+ *
+ * Return: status code
+ */
+efi_status_t efi_install_variables_table(void)
+{
+ u16
