| Message ID | 20240418125456.50944-1-ilias.apalodimas@linaro.org |
|---|---|
| Headers | show |
| Series | [v3,1/4] efi_loader: conditionally enable SetvariableRT | expand |
On 4/18/24 14:54, Ilias Apalodimas wrote: > Hi! > This is v3 of SetVariable at runtime [0] > > Nothing changed drastically from v2. > A few more test cases have been added, comments/suggestions have been > addressed and a bug where deleting a variable by setting 'attributes' to > 0 has been fixed. > > Changes since v2: > - Add more selftests checking for variable deletion as well as the > VarToFile header format > - Use unaligned sized variables on tests > - Add a new function to get the variable entry length instead of > repurposing efi_var_mem_compare() > - Converted VarToFile to RO > - Added a few comments requested by Heinrich > - Fixed a bug where SetVariable with attributes set to 0 did not delete > the variable but returned EFI_INVALID_PARAMETER instead > - Run FWTS 'uefitests' and attach the log in patch #1 > - Added r-b tags from Heinrich > > Changes since v1: > - Instead of Creating VarToFile at SetVariable, create it on GetVariable. > This allows us to get rid of the preallocated RT buffer, since the > address is user provided > - convert Set/GetVariableRT -> Set/GetVariable at runtime > - return EFI_INVALID_PARAM is NV is not set at runtime > - Heinrich sent me the efi_var_collect_mem() variant > > Changes since the RFC: > - Return EFI_INVALID_PARAM if attributes are not volatile > - Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables > - Add 2 EFI variables and allow userspace to write the file > - Add selftests > > I also have a patch enable QueryVariableInfo [1], but I don't want to > introduce new patches now. This also needs a few more testcases of its > own so I'll send it once we finalize this one. > > [0] https://lore.kernel.org/u-boot/20240417101928.119115-1-ilias.apalodimas@linaro.org/T/ > [1] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commit/ce35270aaeb93686d7e013f3b028808e8af88cc0 > > Regards > /Ilias > > Ilias Apalodimas (4): > efi_loader: conditionally enable SetvariableRT > efi_loader: Add OS notifications for SetVariable at runtime > efi_loader: add an EFI variable with the file contents > efi_selftest: add tests for setvariableRT I am missing a defconfig change which is needed to run the unit test in Gitlab CI. I would prefer testing this on the sandbox. Best regards Heinrich > > include/efi_loader.h | 4 + > include/efi_variable.h | 16 +- > lib/charset.c | 2 +- > lib/efi_loader/Kconfig | 16 ++ > lib/efi_loader/efi_runtime.c | 42 ++++ > lib/efi_loader/efi_var_common.c | 6 +- > lib/efi_loader/efi_var_mem.c | 151 ++++++++----- > lib/efi_loader/efi_variable.c | 122 ++++++++-- > lib/efi_loader/efi_variable_tee.c | 5 - > .../efi_selftest_variables_runtime.c | 211 +++++++++++++++++- > 10 files changed, 495 insertions(+), 80 deletions(-) > > -- > 2.40.1 >
On Sat, 20 Apr 2024 at 10:20, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 4/18/24 14:54, Ilias Apalodimas wrote: > > Hi! > > This is v3 of SetVariable at runtime [0] > > > > Nothing changed drastically from v2. > > A few more test cases have been added, comments/suggestions have been > > addressed and a bug where deleting a variable by setting 'attributes' to > > 0 has been fixed. > > > > Changes since v2: > > - Add more selftests checking for variable deletion as well as the > > VarToFile header format > > - Use unaligned sized variables on tests > > - Add a new function to get the variable entry length instead of > > repurposing efi_var_mem_compare() > > - Converted VarToFile to RO > > - Added a few comments requested by Heinrich > > - Fixed a bug where SetVariable with attributes set to 0 did not delete > > the variable but returned EFI_INVALID_PARAMETER instead > > - Run FWTS 'uefitests' and attach the log in patch #1 > > - Added r-b tags from Heinrich > > > > Changes since v1: > > - Instead of Creating VarToFile at SetVariable, create it on GetVariable. > > This allows us to get rid of the preallocated RT buffer, since the > > address is user provided > > - convert Set/GetVariableRT -> Set/GetVariable at runtime > > - return EFI_INVALID_PARAM is NV is not set at runtime > > - Heinrich sent me the efi_var_collect_mem() variant > > > > Changes since the RFC: > > - Return EFI_INVALID_PARAM if attributes are not volatile > > - Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables > > - Add 2 EFI variables and allow userspace to write the file > > - Add selftests > > > > I also have a patch enable QueryVariableInfo [1], but I don't want to > > introduce new patches now. This also needs a few more testcases of its > > own so I'll send it once we finalize this one. > > > > [0] https://lore.kernel.org/u-boot/20240417101928.119115-1-ilias.apalodimas@linaro.org/T/ > > [1] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commit/ce35270aaeb93686d7e013f3b028808e8af88cc0 > > > > Regards > > /Ilias > > > > Ilias Apalodimas (4): > > efi_loader: conditionally enable SetvariableRT > > efi_loader: Add OS notifications for SetVariable at runtime > > efi_loader: add an EFI variable with the file contents > > efi_selftest: add tests for setvariableRT > > I am missing a defconfig change which is needed to run the unit test in > Gitlab CI. I would prefer testing this on the sandbox. > > Best regards Ok I'll send a followup since you already sent a PR with these Thanks! /Ilias > > Heinrich > > > > > include/efi_loader.h | 4 + > > include/efi_variable.h | 16 +- > > lib/charset.c | 2 +- > > lib/efi_loader/Kconfig | 16 ++ > > lib/efi_loader/efi_runtime.c | 42 ++++ > > lib/efi_loader/efi_var_common.c | 6 +- > > lib/efi_loader/efi_var_mem.c | 151 ++++++++----- > > lib/efi_loader/efi_variable.c | 122 ++++++++-- > > lib/efi_loader/efi_variable_tee.c | 5 - > > .../efi_selftest_variables_runtime.c | 211 +++++++++++++++++- > > 10 files changed, 495 insertions(+), 80 deletions(-) > > > > -- > > 2.40.1 > > >