| Message ID | 20200327052800.11022-10-xypron.glpk@gmx.de |
|---|---|
| State | Accepted, archived |
| Commit | 93f6201af71d9a0a521c99212e6066778270a357 |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: non-volatile and runtime variables | expand |
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. 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 >
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. > > 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
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. > > > > 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
> 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.
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@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
On Tue, Mar 31, 2020 at 03:08:06PM +0200, Heinrich Schuchardt wrote: > 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@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. How about CONFIG_FAT? -Takahiro Akashi > 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 >
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 config EFI_RUNTIME_SET_VARIABLE depends on EFI_RUNTIME_GET_VARIABLE select FAT_WRITE ... I proposed similar idea before, but it was not accepted. It is time to do that. Thanks, -Takahiro Akashi
On Wed, Apr 01, 2020 at 08:57:33AM +0900, AKASHI Takahiro wrote: > On Tue, Mar 31, 2020 at 03:08:06PM +0200, Heinrich Schuchardt wrote: > > 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@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. > > How about CONFIG_FAT? (=> What if !CONFIG_FAT) More fundamentally, Why do you want to use a file as storage device for variables? why not raw partition (or just part of partition) on, say, NOR or eMMC? As you know, EDK2 saves variables directly on NOR (or block device? probably). -Takahiro Akashi > -Takahiro Akashi > > > 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 > >
On 4/1/20 3:14 AM, AKASHI Takahiro wrote: > On Wed, Apr 01, 2020 at 08:57:33AM +0900, AKASHI Takahiro wrote: >> On Tue, Mar 31, 2020 at 03:08:06PM +0200, Heinrich Schuchardt wrote: >>> 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@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. >> >> How about CONFIG_FAT? > > (=> What if !CONFIG_FAT) > > > More fundamentally, > Why do you want to use a file as storage device for variables? > why not raw partition (or just part of partition) on, say, > NOR or eMMC? > > As you know, EDK2 saves variables directly on NOR (or block device? > probably). Yes, we may add further stores later on. Ilias wants to use the RPMB area of eMMC devices. As an EFI system partition exists on any UEFI compatible device I think implementing this first is a valid approach. Best regards Heinrich > > -Takahiro Akashi > >> -Takahiro Akashi >> >>> 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 >>>
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 >
On Wed, Apr 01, 2020 at 08:31:10AM +0200, Heinrich Schuchardt wrote: > On 4/1/20 3:14 AM, AKASHI Takahiro wrote: > > On Wed, Apr 01, 2020 at 08:57:33AM +0900, AKASHI Takahiro wrote: > >> On Tue, Mar 31, 2020 at 03:08:06PM +0200, Heinrich Schuchardt wrote: > >>> 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@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. > >> > >> How about CONFIG_FAT? > > > > (=> What if !CONFIG_FAT) ? > > > > > > More fundamentally, > > Why do you want to use a file as storage device for variables? > > why not raw partition (or just part of partition) on, say, > > NOR or eMMC? > > > > As you know, EDK2 saves variables directly on NOR (or block device? > > probably). > > Yes, we may add further stores later on. Why not now? How will you generalize it? > Ilias wants to use the RPMB > area of eMMC devices. I think that he plans to use RPMB as secure storage for Standalone MM. > As an EFI system partition exists on any UEFI > compatible device I think implementing this first is a valid approach. I don't deny your approach here, but I expect that more generic framework for back storage be implemented and FAT driver be on top of that. -Takahiro Akashi > > Best regards > > Heinrich > > > > > -Takahiro Akashi > > > >> -Takahiro Akashi > >> > >>> 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 > >>> >
> Date: Wed, 1 Apr 2020 09:31:03 +0900 > From: AKASHI Takahiro <takahiro.akashi@linaro.org> > > 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. For OpenBSD/arm and OpenBSD/amr64 we adopted the UEFI interfaces in u-boot early on, even before most Linux distros did. As a result our requirements are very minimal: Our bootloader uses the folowing boot services: - HandleProtocol() - AllocatePages() - FreePages() - LocateHandle() - LocateHandleBuffer() - LocateProtocol() - CreateEvent() - SetTimer() - WaitForEvent() - CloseEvent() - GetMemoryMap() - SetWatchdogTimer() - Exit() runtime services: - ResetSystem() and protocols: - EFI_LOADED_IMAGE_PROTOCOL - EFI_DEVICE_PATH_PROTOCOL - EFI_BLOCK_IO_PROTOCOL - EFI_GRAPHICS_OUTPUT_PROTOCOL - EFI_RNG_PROTOCOL - EFI_SIMPLE_NETWORK_PROTOCOL - EFI_PXE_BASE_CODE_PROTOCOL Obviously the last four are only used if there is actual hardware to support these protocols. But I'd consider them non-optional if such hardware exists. Our kernel only uses a couple if runtime services: - SetVirtualAddressMap() - GetTime() (optional) - SetTime() (optional) Cheers, Mark
Hi Mark, On Wed, Apr 01, 2020 at 07:56:31PM +0200, Mark Kettenis wrote: > > Date: Wed, 1 Apr 2020 09:31:03 +0900 > > From: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > 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. > > For OpenBSD/arm and OpenBSD/amr64 we adopted the UEFI interfaces in > u-boot early on, even before most Linux distros did. As a result our > requirements are very minimal: Thank you for your quick feedback. As I'm not familiar with OpenBSD things, this information gives me much better view/understandings. Just from my curiosity, let me ask some questions. > Our bootloader uses the folowing boot services: You don't reply on any features from "EFI Boot Manager," including BootXXXX variables, do you? > - HandleProtocol() > - AllocatePages() > - FreePages() > - LocateHandle() > - LocateHandleBuffer() > - LocateProtocol() > - CreateEvent() > - SetTimer() > - WaitForEvent() > - CloseEvent() > - GetMemoryMap() > - SetWatchdogTimer() > - Exit() > > runtime services: + load_image/start_image, obviously? > - ResetSystem() > > and protocols: > > - EFI_LOADED_IMAGE_PROTOCOL > - EFI_DEVICE_PATH_PROTOCOL > - EFI_BLOCK_IO_PROTOCOL > - EFI_GRAPHICS_OUTPUT_PROTOCOL > - EFI_RNG_PROTOCOL > - EFI_SIMPLE_NETWORK_PROTOCOL > - EFI_PXE_BASE_CODE_PROTOCOL What about EFI_FILE_PROTOCOL and/or EFI_FILE_LOAD_PROTOCOL? Don't you have the assumption that there must be any EFI system partition (and file system) on your boards? -Takahiro Akashi > Obviously the last four are only used if there is actual hardware to > support these protocols. But I'd consider them non-optional if such > hardware exists. > > Our kernel only uses a couple if runtime services: > > - SetVirtualAddressMap() > - GetTime() (optional) > - SetTime() (optional) As you might know, I'm working on UEFI secure boot for U-Boot. Does OpenBSD support any kind of secure boot? If so, is it based on UEFI feature (in that case, UEFI variables are mandatory) or do you have your own solution? Thanks, -Takahiro Akashi > Cheers, > > Mark
On Wed, Apr 01, 2020 at 04:04:04PM +0900, AKASHI Takahiro wrote: > > [...] > > Yes, we may add further stores later on. > > Why not now? > How will you generalize it? > > > Ilias wants to use the RPMB > > area of eMMC devices. > > I think that he plans to use RPMB as secure storage for Standalone MM. Sorry for my really delayed replies, but I was over my head with EDK2 and RPMB indeed :) So my implementation is orthogonal to this patchset and I do have some code that write UEFI variables on an RPMB via OP-TEE. > > > As an EFI system partition exists on any UEFI > > compatible device I think implementing this first is a valid approach. > > I don't deny your approach here, but > I expect that more generic framework for back storage be > implemented and FAT driver be on top of that. > Akashi has a point here. On the other hand can't we just make the store a callback in the future? So for example is FAT_FS support is not included, we write the data into raw flash sectors or a file. That being said EDK2 uses Fvb and Fault Tolerant Writes to write the actual UEFI variables. So you end up needing 3 partitions for the actual storage. The question here is should we implement something similar to u-boot EFI code since this is only supposed to serve as a 'fallback'? If the board has OP-TEE then that will be responsible for writing the variables (and it will use whatever EDK2 uses). I'll have a closer look to the patchset and try to give some feedback. Regards /Ilias > -Takahiro Akashi > > > > > Best regards > > > > Heinrich > > > > > > > > -Takahiro Akashi > > > > > >> -Takahiro Akashi > > >> > > >>> 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 --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
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(+) -- 2.25.1