[PATCH] efi_loader: optional persistence of variables

[Heinrich Schuchardt]

On 3/23/22 17:22, Tom Saeger wrote:
> On Tue, Mar 22, 2022 at 06:03:28PM -0500, Tom Saeger wrote:
>> On Tue, Mar 22, 2022 at 11:33:20PM +0100, Heinrich Schuchardt wrote:
>>> On 3/22/22 23:16, Tom Saeger wrote:
>>>> On Tue, Mar 22, 2022 at 10:41:40PM +0100, Heinrich Schuchardt wrote:
>>>>> On 3/22/22 22:21, Tom Saeger wrote:
>>>>>> Since be66b89da306 ("efi_loader: configuration of variables store")
>>>>>> the choice of EFI_VARIABLE_FILE_STORE or EFI_MM_COMM_TEE
>>>>>> is mutually-exclusive, however efi_var_to_file also allows
>>>>>> for "neither".  Set Kconfig choice optional.
>>>>>>
>>>>>> Signed-off-by: Tom Saeger <tom.saeger at oracle.com>
>>>>>> ---
>>>>>>     lib/efi_loader/Kconfig | 1 +
>>>>>>     1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>>>>> index e5e35fe51f65..9add2a286ff4 100644
>>>>>> --- a/lib/efi_loader/Kconfig
>>>>>> +++ b/lib/efi_loader/Kconfig
>>>>>> @@ -44,6 +44,7 @@ config EFI_SETUP_EARLY
>>>>>>
>>>>>>     choice
>>>>>>     	prompt "Store for non-volatile UEFI variables"
>>>>>> +	optional
>>>>>
>>>>> Storing non-volatile variables is required by the UEFI specification.
>>>>>
>>>>> How should a user understand that a boot option he just created vanishes
>>>>> upon reboot?
>>>>>
>>>>> Please, explain your use case.
>>>>
>>>> bootefi ${kernel_addr}
>>>>
>>>> in this case linux kernel for an Armv8 platform.
>>>> This platform does not want or need any variables to persist.
>>>>
>>>> bootefi eventually calls efi_var_from_file(), where *NOT* defining
>>>> CONFIG_EFI_VARIABLE_FILE_STORE would allow this use case to work,
>>>> otherwise it fails.  This was possible before
>>>> be66b89da306 ("efi_loader: configuration of variables store").
>>>
>>> Please, describe exactly what does not work. Is it building or is it
>>> running the bootefi command?
>>
>> after be66b89da306 CONFIG_EFI_VARIABLE_FILE_STORE is now selected due to
>> Kconfig choice.  This is the difference.
>>
>> It fails while running, but the difference is the build.
>> I'd rather configure for the old behavior - that being return
>> EFI_SUCCESS from efi_var_from_file() when CONFIG_EFI_VARIABLE_FILE_STORE
>> isn't defined.
>>
>> The call-stack is something like:
>>
>> do_bootefi->efi_init_obj_list->efi_init_variables->efi_var_from_file
>
> I think perhaps I stumbled upon an unsupported mode, which the prior
> Kconfig unintentionally allowed.
>
> If so - this patch doesn't make sense.
>
> It was however convenient for our application.
> (NO ESP, NO persistence)
>
>>
>>>
>>> Does your system have an ESP?
>
> I'll look into this - perhaps it should, despite strong desire to
> disallow changes to it.
>
>
>>> Where does bootefi fail?
>
> When CONFIG_EFI_VARIABLE_FILE_STORE is defined, runtime complains (as it
> should)
>
> "Failed to persist EFI variables"

This message is written as an error log message in efi_var_to_file().
The only caller is efi_set_variable_int() which ignores the return value
of efi_var_to_file().

This does not lead to a boot failure.

Which failure do you try to solve?

Best regards

Heinrich