[PATCH 09/16] efi_loader: imply FAT, FAT_WRITE

Heinrich Schuchardt
2020-04-01T06:43:42Z

On 4/1/20 2:31 AM, AKASHI Takahiro wrote:
> On Tue, Mar 31, 2020 at 10:20:17AM +0200, Mark Kettenis wrote:
>>> Date: Tue, 31 Mar 2020 16:44:34 +0900
>>> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>>
>>> On Tue, Mar 31, 2020 at 08:44:02AM +0200, Heinrich Schuchardt wrote:
>>>> On March 31, 2020, 5:28 a.m. UTC Takahiro Akashi wrote:
>>>>> On Fri, Mar 27, 2020 at 06:27:53AM +0100, Heinrich Schuchardt wrote:
>>>>>> The UEFI spec requires support for the FAT file system.
>>>>>>
>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>>> ---
>>>>>> lib/efi_loader/Kconfig | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>>>>> index 9890144d41..e10ca05549 100644
>>>>>> --- a/lib/efi_loader/Kconfig
>>>>>> +++ b/lib/efi_loader/Kconfig
>>>>>> @@ -15,6 +15,8 @@ config EFI_LOADER
>>>>>> select HAVE_BLOCK_DEVICE
>>>>>> select REGEX
>>>>>> imply CFB_CONSOLE_ANSI
>>>>>> + imply FAT
>>>>>> + imply FAT_WRITE
>>>>>
>>>>> Obviously, this *imply* doesn't enforce enabling FAT.
>>>>> If it is absolutely necessary, another measure should be taken.
>>>>
>>>> If somebody wants to minimize the U-Boot size it might be necessary to
>>>> do without FAT_WRITE or FAT support.
>>>
>>> If so, Get/SetVariable won't be supported even in boot time
>>> with your patch applied. It is not practical for almost all users.
>>
>> I *strongly* disagree with that statement. Most users don't care
>> about U-Boot providing a full EFI implementation. They just want to
>> boot their OS. The basic EFI support in U-Boot is good enough for
>> that and for OpenBSD and some Linux distros on arm/arm64 this is the
>> only bootpath that works. If adding more code leads to board
>> maintainers disabling EFI support this isn't helpful.
>
> Okay, so can you please describe the minimum set of functionality
> for you? Without that, the discussion will not be fair.
>
> Anyhow, using "imply" to customize UEFI functionality would be
> poor. Instead, we should introduce explicit configuration, say,
>
> config EFI_CUSTOMIZE_RUNTIME_SERVICES
>
> if EFI_CUSTOMIZE_RUNTIME_SERVICES
>
> config EFI_RUNTIME_GET_VARIABLE
> select FAT

Why should exposing the GetVariable() service depend on FAT?

At least for GetVariable() I see no real benefit of not exposing it always.

>
> config EFI_RUNTIME_SET_VARIABLE
> depends on EFI_RUNTIME_GET_VARIABLE
> select FAT_WRITE

We cannot use writing to FAT at runtime. So this dependency seems incorrect.

>
> ...
>
> I proposed similar idea before, but it was not accepted.
> It is time to do that.

Unfortunately your patches for adjusting the storage of environments
never made it into master.

Best regards

Heinrich

>
> Thanks,
> -Takahiro Akashi
>