On 4/1/20 3:41 AM, AKASHI Takahiro wrote:
> 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?

This is the list of attributes that have to be set as a *minimum* in a
valid call to SetVariable. APPEND_WRITE is not required but optional.

>
>
>> +    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.

Here I check the parameters of the call which may create a new variable.

We should later in the code check if the variable already exists and if
it is volatile return EFI_WRITE_PROTECTED.

Best regards

Heinrich

>
>> +    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