[...] > > > > +config EFI_RT_VOLATILE_STORE > > + bool "Allow variable runtime services in volatile storage (e.g RAM)" > > + depends on EFI_VARIABLE_FILE_STORE > > + help > > + When EFI variables are stored on file we don't allow SetVariableRT, > > + since the OS doesn't know how to write that file. At he same time > > + we copy runtime variables in DRAM and support GetVariableRT > > + > > + Enable this option to allow SetVariableRT on the RAM backend of > > + the EFI variable storate. The OS will be responsible for syncing > > Hello Ilias, > > I like the idea of adding this for experimenting. We should express that > this is just experimental and may be removed in future releases, if we > find a better solution.
U-Boot documentation is good enough? I've cc'ed Peter (which maintain efivar AFAIK). If we agree on how to teach efivar/efibootmgr to use that file, I can describe it in docs > > %s/storate/stroage/ > > > + the RAM contents to the file, otherwise any changes made during > > + runtime won't persist reboots. > > + Authenticated variables are not supported. > > Should we mention here that this behavior is not compliant with the UEFI > specification? > > Should we unset CONFIG_EFI_EBBR_2_1_CONFORMANCE if this flag is set? No, I don't think so. I'll ask around to make sure. However, EBBR already standardizes the file format and explicitly prevents storing authenticated variables. There's a separate test suite from arm called SIE (SEcurity interface extensions) which is complementary to SystemReady-IR and tests EFI authenticated variables. > > > + > > config EFI_MM_COMM_TEE > > bool "UEFI variables storage service via the trusted world" > > depends on OPTEE > > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c > > index 18da6892e796..b38ce239e2d2 100644 > > --- a/lib/efi_loader/efi_runtime.c > > +++ b/lib/efi_loader/efi_runtime.c > > @@ -126,6 +126,10 @@ efi_status_t efi_init_runtime_supported(void) > > EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP | > > EFI_RT_SUPPORTED_CONVERT_POINTER; > > > > + if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) > > + rt_table->runtime_services_supported |= > > + EFI_RT_SUPPORTED_SET_VARIABLE; > > + > > /* > > * This value must be synced with efi_runtime_detach_list > > * as well as efi_runtime_services. > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > > index 40f7a0fb10d5..3b770ff16971 100644 > > --- a/lib/efi_loader/efi_variable.c > > +++ b/lib/efi_loader/efi_variable.c > > @@ -218,17 +218,21 @@ efi_get_next_variable_name_int(efi_uintn_t > > *variable_name_size, > > return efi_get_next_variable_name_mem(variable_name_size, > > variable_name, vendor); > > } > > > > -efi_status_t efi_set_variable_int(const u16 *variable_name, > > - const efi_guid_t *vendor, > > - u32 attributes, efi_uintn_t data_size, > > - const void *data, bool ro_check) > > +/** > > + * efi_check_setvariable() - checks defined by the UEFI spec for > > setvariable > > + * > > + * @variable_name: name of the variable > > + * @vendor: vendor GUID > > + * @attributes: attributes of the variable > > + * @data_size: size of the buffer with the variable value > > + * @data: buffer with the variable value > > + * Return: status code > > + */ > > +static efi_status_t __efi_runtime EFIAPI > > +efi_check_setvariable(const u16 *variable_name, const efi_guid_t *vendor, > > + u32 attributes, efi_uintn_t data_size, > > + const void *data) > > { > > - struct efi_var_entry *var; > > - efi_uintn_t ret; > > - bool append, delete; > > - u64 time = 0; > > - enum efi_auth_var_type var_type; > > - > > if (!variable_name || !*variable_name || !vendor) > > return EFI_INVALID_PARAMETER; > > > > @@ -256,6 +260,25 @@ efi_status_t efi_set_variable_int(const u16 > > *variable_name, > > !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) > > return EFI_INVALID_PARAMETER; > > > > + return EFI_SUCCESS; > > +} > > + > > +efi_status_t efi_set_variable_int(const u16 *variable_name, > > + const efi_guid_t *vendor, > > + u32 attributes, efi_uintn_t data_size, > > + const void *data, bool ro_check) > > +{ > > + struct efi_var_entry *var; > > + efi_uintn_t ret; > > + bool append, delete; > > + u64 time = 0; > > + enum efi_auth_var_type var_type; > > + > > + ret = efi_check_setvariable(variable_name, vendor, attributes, > > data_size, > > + data); > > + if (ret != EFI_SUCCESS) > > + return ret; > > + > > /* check if a variable exists */ > > var = efi_var_mem_find(vendor, variable_name, NULL); > > append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); > > @@ -434,7 +457,72 @@ efi_set_variable_runtime(u16 *variable_name, const > > efi_guid_t *vendor, > > u32 attributes, efi_uintn_t data_size, > > const void *data) > > { > > +#if IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE) > > Please, use 'if' instead of #if. The Sure > > > + struct efi_var_entry *var; > > + efi_uintn_t ret; > > + bool append, delete; > > + u64 time = 0; > > + > > + /* Authenticated variables are not supported */ > > + if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) > > Adding EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS here would make it > clearer that all authenticated variables are unsupported. Ok, will do > > Non-volatile variables are read-only at runtime. Please, check that > EFI_VARIABLE_NON_VOLATILE is set. You mean volatile right? But yea this needs fixing > > if ((attributes & ( > EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | > EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)) || > (~attributes & EFI_VARIABLE_NON_VOLATILE) > return EFI_INVALID_PARAMETER; > > > + return EFI_INVALID_PARAMETER; > > + > > + ret = efi_check_setvariable(variable_name, vendor, attributes, > > data_size, > > + data); > > + if (ret != EFI_SUCCESS) > > + return ret; > > + > > + /* check if a variable exists */ > > + var = efi_var_mem_find(vendor, variable_name, NULL); > > + append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); > > + attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; > > Why is the u32 conversion necessary? The result should be the same > without conversion. > > The UEFI specification defines the different attribute constants as > 32bit. Why do we define them as 64bit in include/efi.h? I guess this is an existing misinterpretation of the spec. I copy pasted this check from efi_set_variable_int(). It seems we should fix both. I'll include the fix in v2 > > > + delete = !append && (!data_size || !attributes); > > + > > + if (var) { > > + if (var->attr & EFI_VARIABLE_READ_ONLY) > > + return EFI_WRITE_PROTECTED; > > + > > + /* attributes won't be changed */ > > + if (!delete && (((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY) != > > + (attributes & ~(u32)EFI_VARIABLE_READ_ONLY)))) > > + return EFI_INVALID_PARAMETER; > > ditto > > > + time = var->time; > > + } else { > > + if (delete || append) > > EDK II allows to create a variable with EFI_VARIABLE_APPEND_WRITE. See > previous discussions about changing this at boot time. Yes, the reason I kept it like this is that we didn't merge that yet and I wanted to keep the same behavior. I'll update this one [...] Thanks /Ilias