diff mbox series

[v2,16/17] efi_loader: enable UEFI variables at runtime

Message ID 20200707031200.65511-17-xypron.glpk@gmx.de
State New
Headers show
Series efi_loader: non-volatile and runtime variables | expand

Commit Message

Heinrich Schuchardt July 7, 2020, 3:11 a.m. UTC 
Enable UEFI variables at runtime.

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
 lib/efi_loader/efi_runtime.c  | 2 ++
 lib/efi_loader/efi_variable.c | 6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

--
2.27.0

Comments

Ilias Apalodimas July 7, 2020, 1:10 p.m. UTC | #1 
On Tue, Jul 07, 2020 at 05:11:59AM +0200, Heinrich Schuchardt wrote:
> Enable UEFI variables at runtime.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  lib/efi_loader/efi_runtime.c  | 2 ++
>  lib/efi_loader/efi_variable.c | 6 ++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 5b6506fbdc..91a4551448 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
>  	rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
>  	rt_table->length = sizeof(struct efi_rt_properties_table);
>  	rt_table->runtime_services_supported =
> +				EFI_RT_SUPPORTED_GET_VARIABLE |
> +				EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
>  				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
>  				EFI_RT_SUPPORTED_CONVERT_POINTER;
> 
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 13123e7e41..c472a054d0 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
>  efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
>  			 u32 *attributes, efi_uintn_t *data_size, void *data)
>  {
> -	return EFI_UNSUPPORTED;
> +	return efi_get_variable_int(variable_name, vendor, attributes,
> +				    data_size, data, NULL);
>  }
> 
>  /**
> @@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
>  efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
>  				   u16 *variable_name, efi_guid_t *vendor)
>  {
> -	return EFI_UNSUPPORTED;
> +	return efi_get_next_variable_name_int(variable_name_size, variable_name,
> +					      vendor);

Since we use the internal functions for the variables, don't we include the 
EFI_VARIABLE_READ_ONLY bit as well (which is different on the tee based
version). Wouldn't it be better to just remove the runtime variant completely?

Thanks
/Ilias
>  }
> 
>  /**
> --
> 2.27.0
>
Ilias Apalodimas July 7, 2020, 1:30 p.m. UTC | #2 
On Tue, 7 Jul 2020 at 16:10, <ilias.apalodimas at linaro.org> wrote:
>
> On Tue, Jul 07, 2020 at 05:11:59AM +0200, Heinrich Schuchardt wrote:
> > Enable UEFI variables at runtime.
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > ---
> >  lib/efi_loader/efi_runtime.c  | 2 ++
> >  lib/efi_loader/efi_variable.c | 6 ++++--
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index 5b6506fbdc..91a4551448 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
> >       rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
> >       rt_table->length = sizeof(struct efi_rt_properties_table);
> >       rt_table->runtime_services_supported =
> > +                             EFI_RT_SUPPORTED_GET_VARIABLE |
> > +                             EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
> >                               EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
> >                               EFI_RT_SUPPORTED_CONVERT_POINTER;
> >
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index 13123e7e41..c472a054d0 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
> >  efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
> >                        u32 *attributes, efi_uintn_t *data_size, void *data)
> >  {
> > -     return EFI_UNSUPPORTED;
> > +     return efi_get_variable_int(variable_name, vendor, attributes,
> > +                                 data_size, data, NULL);
> >  }
> >
> >  /**
> > @@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
> >  efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> >                                  u16 *variable_name, efi_guid_t *vendor)
> >  {
> > -     return EFI_UNSUPPORTED;
> > +     return efi_get_next_variable_name_int(variable_name_size, variable_name,
> > +                                           vendor);
>
> Since we use the internal functions for the variables, don't we include the
> EFI_VARIABLE_READ_ONLY bit as well (which is different on the tee based
> version). Wouldn't it be better to just remove the runtime variant completely?

Replying to myself on that. Removing is not possible since I just
noticed the EFI_ENTRY is still
present on the non-runtime variants. The RO flag needs to be handled though.

Thanks
/Ilias
>
> Thanks
> /Ilias
> >  }
> >
> >  /**
> > --
> > 2.27.0
> >
Heinrich Schuchardt July 7, 2020, 2:02 p.m. UTC | #3 
On 07.07.20 15:30, Ilias Apalodimas wrote:
> On Tue, 7 Jul 2020 at 16:10, <ilias.apalodimas at linaro.org> wrote:
>>
>> On Tue, Jul 07, 2020 at 05:11:59AM +0200, Heinrich Schuchardt wrote:
>>> Enable UEFI variables at runtime.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>  lib/efi_loader/efi_runtime.c  | 2 ++
>>>  lib/efi_loader/efi_variable.c | 6 ++++--
>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>> index 5b6506fbdc..91a4551448 100644
>>> --- a/lib/efi_loader/efi_runtime.c
>>> +++ b/lib/efi_loader/efi_runtime.c
>>> @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
>>>       rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
>>>       rt_table->length = sizeof(struct efi_rt_properties_table);
>>>       rt_table->runtime_services_supported =
>>> +                             EFI_RT_SUPPORTED_GET_VARIABLE |
>>> +                             EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
>>>                               EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
>>>                               EFI_RT_SUPPORTED_CONVERT_POINTER;
>>>
>>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>>> index 13123e7e41..c472a054d0 100644
>>> --- a/lib/efi_loader/efi_variable.c
>>> +++ b/lib/efi_loader/efi_variable.c
>>> @@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
>>>  efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
>>>                        u32 *attributes, efi_uintn_t *data_size, void *data)
>>>  {
>>> -     return EFI_UNSUPPORTED;
>>> +     return efi_get_variable_int(variable_name, vendor, attributes,
>>> +                                 data_size, data, NULL);
>>>  }
>>>
>>>  /**
>>> @@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
>>>  efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
>>>                                  u16 *variable_name, efi_guid_t *vendor)
>>>  {
>>> -     return EFI_UNSUPPORTED;
>>> +     return efi_get_next_variable_name_int(variable_name_size, variable_name,
>>> +                                           vendor);
>>
>> Since we use the internal functions for the variables, don't we include the
>> EFI_VARIABLE_READ_ONLY bit as well (which is different on the tee based
>> version). Wouldn't it be better to just remove the runtime variant completely?
>
> Replying to myself on that. Removing is not possible since I just
> noticed the EFI_ENTRY is still
> present on the non-runtime variants. The RO flag needs to be handled though.

Thanks for reviewing. Yes we have to mask the RO bit.

Best regards

Heinrich
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 5b6506fbdc..91a4551448 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -121,6 +121,8 @@  efi_status_t efi_init_runtime_supported(void)
 	rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
 	rt_table->length = sizeof(struct efi_rt_properties_table);
 	rt_table->runtime_services_supported =
+				EFI_RT_SUPPORTED_GET_VARIABLE |
+				EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
 				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
 				EFI_RT_SUPPORTED_CONVERT_POINTER;

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 13123e7e41..c472a054d0 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -672,7 +672,8 @@  static efi_status_t __efi_runtime EFIAPI
 efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
 			 u32 *attributes, efi_uintn_t *data_size, void *data)
 {
-	return EFI_UNSUPPORTED;
+	return efi_get_variable_int(variable_name, vendor, attributes,
+				    data_size, data, NULL);
 }

 /**
@@ -688,7 +689,8 @@  static efi_status_t __efi_runtime EFIAPI
 efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
 				   u16 *variable_name, efi_guid_t *vendor)
 {
-	return EFI_UNSUPPORTED;
+	return efi_get_next_variable_name_int(variable_name_size, variable_name,
+					      vendor);
 }

 /**