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