diff mbox series

[09/16] efi_loader: imply FAT, FAT_WRITE

Message ID 20200327052800.11022-10-xypron.glpk@gmx.de
State Accepted
Commit 93f6201af71d9a0a521c99212e6066778270a357
Headers show
Series efi_loader: non-volatile and runtime variables | expand

Commit Message

Heinrich Schuchardt March 27, 2020, 5:27 a.m. UTC
The UEFI spec requires support for the FAT file system.

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
 lib/efi_loader/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

--
2.25.1

Comments

AKASHI Takahiro March 31, 2020, 5:28 a.m. UTC | #1
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 at 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.

In addition, why do you treat FAT specifically here?
I remember that you insisted that other file system should be
allowed on U-Boot when I posted some patch.

-Takahiro Akashi


>  	imply USB_KEYBOARD_FN_KEYS
>  	imply VIDEO_ANSI
>  	help
> --
> 2.25.1
>
Heinrich Schuchardt March 31, 2020, 6:44 a.m. UTC | #2
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 at 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.

 >
 > In addition, why do you treat FAT specifically here?
 > I remember that you insisted that other file system should be
 > allowed on U-Boot when I posted some patch.

An EFI system partition is always FAT formatted. So if we want to safe
U-Boot variables to the EFI system partition we require FAT.

Best regards

Heinrich

 >
 > -Takahiro Akashi
 >
 >
 > >  	imply USB_KEYBOARD_FN_KEYS
 > >  	imply VIDEO_ANSI
 > >  	help
 > > --
 > > 2.25.1
AKASHI Takahiro March 31, 2020, 7:44 a.m. UTC | #3
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 at 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.

> >
> > In addition, why do you treat FAT specifically here?
> > I remember that you insisted that other file system should be
> > allowed on U-Boot when I posted some patch.
> 
> An EFI system partition is always FAT formatted. So if we want to safe
> U-Boot variables to the EFI system partition we require FAT.

As system partition is required to be in FAT, file system used on
other partitions must also be in FAT since, as I said before,
UEFI specification clearly defines its file system format based on FAT.
See section 13.3.

So,

> > I remember that you insisted that other file system should be
> > allowed on U-Boot when I posted some patch.

You reverted your statement above here.
That is my point.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> > -Takahiro Akashi
> >
> >
> > >  	imply USB_KEYBOARD_FN_KEYS
> > >  	imply VIDEO_ANSI
> > >  	help
> > > --
> > > 2.25.1
Mark Kettenis March 31, 2020, 8:20 a.m. UTC | #4
> Date: Tue, 31 Mar 2020 16:44:34 +0900
> From: AKASHI Takahiro <takahiro.akashi at 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 at 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.
Heinrich Schuchardt March 31, 2020, 1:08 p.m. UTC | #5
On 2020-03-31 09:44, AKASHI Takahiro wrote:
> 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 at 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.

Hello Akashi,

without FAT_WRITE we will not have persistence for variables.
SetVariable and GetVariable are still usable.

Best regards

Heinrich

>
>>>
>>> In addition, why do you treat FAT specifically here?
>>> I remember that you insisted that other file system should be
>>> allowed on U-Boot when I posted some patch.
>>
>> An EFI system partition is always FAT formatted. So if we want to safe
>> U-Boot variables to the EFI system partition we require FAT.
>
> As system partition is required to be in FAT, file system used on
> other partitions must also be in FAT since, as I said before,
> UEFI specification clearly defines its file system format based on FAT.
> See section 13.3.
>
> So,
>
>>> I remember that you insisted that other file system should be
>>> allowed on U-Boot when I posted some patch.
>
> You reverted your statement above here.
> That is my point.
>
> -Takahiro Akashi
>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> -Takahiro Akashi
>>>
>>>
>>>>  	imply USB_KEYBOARD_FN_KEYS
>>>>  	imply VIDEO_ANSI
>>>>  	help
>>>> --
>>>> 2.25.1
diff mbox series

Patch

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
 	imply USB_KEYBOARD_FN_KEYS
 	imply VIDEO_ANSI
 	help