On Tue, Mar 31, 2020 at 08:05:40AM +0200, Heinrich Schuchardt wrote:
> Enable UEFI variables at runtime.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
> ---
>  lib/efi_loader/efi_runtime.c  |  6 +++++-
>  lib/efi_loader/efi_variable.c | 23 +++++++++++++++++------
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 664a0422e2..acd644202d 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -120,8 +120,12 @@ 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_VARIABLE |
>                               EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
> -                             EFI_RT_SUPPORTED_CONVERT_POINTER;
> +                             EFI_RT_SUPPORTED_CONVERT_POINTER |
> +                             EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO;
> 
>       /*
>        * This value must be synced with efi_runtime_detach_list
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 7c39542968..cf8b44c535 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -326,15 +326,13 @@ efi_status_t __efi_runtime EFIAPI 
> efi_query_variable_info(
>                       u64 *remaining_variable_storage_size,
>                       u64 *maximum_variable_size)
>  {
> -     /*
>       *maximum_variable_storage_size = EFI_VAR_BUF_SIZE -
>                                        sizeof(struct efi_var_file);
>       *remaining_variable_storage_size = efi_var_mem_free();
>       *maximum_variable_size = EFI_VAR_BUF_SIZE -
>                                sizeof(struct efi_var_file) -
>                                sizeof(struct efi_var_entry);
> -     */
> -     return EFI_UNSUPPORTED;
> +     return EFI_SUCCESS;
>  }
> 
>  /**
> @@ -351,7 +349,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);
>  }
> 
>  /**
> @@ -367,7 +366,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);
>  }
> 
>  /**
> @@ -385,7 +385,18 @@ efi_set_variable_runtime(u16 *variable_name, const 
> efi_guid_t *vendor,
>                        u32 attributes, efi_uintn_t data_size,
>                        const void *data)
>  {
> -     return EFI_UNSUPPORTED;
> +     const u32 required_attributes = EFI_VARIABLE_NON_VOLATILE |
> +                                     EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +                                     EFI_VARIABLE_RUNTIME_ACCESS;
> +

Why not support APPEND_WRITE?


> +     if (attributes &&
> +         (attributes & required_attributes) != required_attributes)
> +             return EFI_INVALID_PARAMETER;

UEFI specification says,
===8<====
Variables that have runtime access but that are not nonvolatile are read-
only data variables once ExitBootServices() is performed.
[snip]
EFI_WRITE_PROTECTED The variable in question is read-only.
===>8===

So in this case, we should return WRITE_PROTECTED instead of
INVALID_PARAMETER.

> +     if ((attributes & ~(u32)EFI_VARIABLE_MASK))
> +             return EFI_INVALID_PARAMETER;
> +
> +     return efi_set_variable_rt_int(variable_name, vendor, attributes,
> +                                    data_size, data);

I didn't follow detailed logic of this function, but
does it really return UNSUPPORTED if !FAT_WRITE?

-Takahiro Akashi

>  }
> 
>  /**
> --
> 2.25.1
> 

Reply via email to