[PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Ilias Apalodimas]

When we try to load EFI variables from a file in the ESP partition and the
file is missing We print a scary error looking like
=> printenv -e
** Unable to read file ubootefi.var **
Failed to load EFI variables

This is not an error though since the file wasn't there to begin with.
So silence the warning by aborting the load if the file is not there,
instead of failing the load.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_var_file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index 62e071bd8341..7d7141473634 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
 		return EFI_OUT_OF_RESOURCES;
 	}
 
+	ret = efi_set_blk_dev_to_system_partition();
+	if (ret != EFI_SUCCESS)
+		goto error;
+	if (!fs_exists(EFI_VAR_FILE_NAME))
+		goto error;
+
 	ret = efi_set_blk_dev_to_system_partition();
 	if (ret != EFI_SUCCESS)
 		goto error;
-- 
2.38.1

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Etienne Carriere]

On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> When we try to load EFI variables from a file in the ESP partition and the
> file is missing We print a scary error looking like
> => printenv -e
> ** Unable to read file ubootefi.var **
> Failed to load EFI variables
>
> This is not an error though since the file wasn't there to begin with.
> So silence the warning by aborting the load if the file is not there,
> instead of failing the load.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  lib/efi_loader/efi_var_file.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index 62e071bd8341..7d7141473634 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
>                 return EFI_OUT_OF_RESOURCES;
>         }
>
> +       ret = efi_set_blk_dev_to_system_partition();
> +       if (ret != EFI_SUCCESS)
> +               goto error;
> +       if (!fs_exists(EFI_VAR_FILE_NAME))
> +               goto error;
> +
>         ret = efi_set_blk_dev_to_system_partition();
>         if (ret != EFI_SUCCESS)
>                 goto error;

This later call to efi_set_blk_dev_to_system_partition() can be
removed since already done above.

Etienne

> --
> 2.38.1
>

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Ilias Apalodimas]

Hi Etienne,

On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > When we try to load EFI variables from a file in the ESP partition and the
> > file is missing We print a scary error looking like
> > => printenv -e
> > ** Unable to read file ubootefi.var **
> > Failed to load EFI variables
> >
> > This is not an error though since the file wasn't there to begin with.
> > So silence the warning by aborting the load if the file is not there,
> > instead of failing the load.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  lib/efi_loader/efi_var_file.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > index 62e071bd8341..7d7141473634 100644
> > --- a/lib/efi_loader/efi_var_file.c
> > +++ b/lib/efi_loader/efi_var_file.c
> > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> >                 return EFI_OUT_OF_RESOURCES;
> >         }
> >
> > +       ret = efi_set_blk_dev_to_system_partition();
> > +       if (ret != EFI_SUCCESS)
> > +               goto error;
> > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > +               goto error;
> > +
> >         ret = efi_set_blk_dev_to_system_partition();
> >         if (ret != EFI_SUCCESS)
> >                 goto error;
>
> This later call to efi_set_blk_dev_to_system_partition() can be
> removed since already done above.
>

iirc fs_exists() unconditionally calls fs_close() so we need that double call

Cheers
/Ilias
> Etienne
>
> > --
> > 2.38.1
> >

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Heinrich Schuchardt]

On 1/19/23 12:45, Ilias Apalodimas wrote:
> Hi Etienne,
>
> On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
>>
>> On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
>> <ilias.apalodimas@linaro.org> wrote:
>>>
>>> When we try to load EFI variables from a file in the ESP partition and the
>>> file is missing We print a scary error looking like
>>> => printenv -e
>>> ** Unable to read file ubootefi.var **

This message is in line fs/fat/fat.c:1297:
printf("** Unable to read file %s **\n", filename);

The message is misplaced. If any message is written, it should be in
fat_read_at().

But I think fat_read_at() should also be quiet. It is the task of the
calling application to shout out if a file cannot be read.

I have not checked if a test in test/py/tests/test_fs/ relies on the
error message.

>>> Failed to load EFI variables

If there is a writable ESP, file ubootefi.var is created on first boot.
So this message should only once in the lifecycle of the board.

Afterwards this message indicates that something is really wrong.

If board vendors doesn't like it, they can create a dummy file.

Best regards

Heinrich

>>>
>>> This is not an error though since the file wasn't there to begin with.
>>> So silence the warning by aborting the load if the file is not there,
>>> instead of failing the load.
>>>
>>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>> ---
>>>   lib/efi_loader/efi_var_file.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
>>> index 62e071bd8341..7d7141473634 100644
>>> --- a/lib/efi_loader/efi_var_file.c
>>> +++ b/lib/efi_loader/efi_var_file.c
>>> @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
>>>                  return EFI_OUT_OF_RESOURCES;
>>>          }
>>>
>>> +       ret = efi_set_blk_dev_to_system_partition();
>>> +       if (ret != EFI_SUCCESS)
>>> +               goto error;
>>> +       if (!fs_exists(EFI_VAR_FILE_NAME))
>>> +               goto error;
>>> +
>>>          ret = efi_set_blk_dev_to_system_partition();
>>>          if (ret != EFI_SUCCESS)
>>>                  goto error;
>>
>> This later call to efi_set_blk_dev_to_system_partition() can be
>> removed since already done above.
>>
>
> iirc fs_exists() unconditionally calls fs_close() so we need that double call
>
> Cheers
> /Ilias
>> Etienne
>>
>>> --
>>> 2.38.1
>>>

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Ilias Apalodimas]

Hi Heinrich,
On Thu, Jan 19, 2023 at 01:42:20PM +0100, Heinrich Schuchardt wrote:
> On 1/19/23 12:45, Ilias Apalodimas wrote:
> > Hi Etienne,
> >
> > On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> > <etienne.carriere@linaro.org> wrote:
> > >
> > > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > When we try to load EFI variables from a file in the ESP partition and the
> > > > file is missing We print a scary error looking like
> > > > => printenv -e
> > > > ** Unable to read file ubootefi.var **
>
> This message is in line fs/fat/fat.c:1297:
> printf("** Unable to read file %s **\n", filename);
>
> The message is misplaced. If any message is written, it should be in
> fat_read_at().
>
> But I think fat_read_at() should also be quiet. It is the task of the
> calling application to shout out if a file cannot be read.
>
> I have not checked if a test in test/py/tests/test_fs/ relies on the
> error message.
>
> > > > Failed to load EFI variables
>
> If there is a writable ESP, file ubootefi.var is created on first boot.

Are you sure?  I think it's created the moment you create an EFI variable.
So on a board that uses preseeded files you'll always see the message.

Thanks
/Ilias
> So this message should only once in the lifecycle of the board.
>
> Afterwards this message indicates that something is really wrong.
>
> If board vendors doesn't like it, they can create a dummy file.
>
> Best regards
>
> Heinrich
>
> > > >
> > > > This is not an error though since the file wasn't there to begin with.
> > > > So silence the warning by aborting the load if the file is not there,
> > > > instead of failing the load.
> > > >
> > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > ---
> > > >   lib/efi_loader/efi_var_file.c | 6 ++++++
> > > >   1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > > index 62e071bd8341..7d7141473634 100644
> > > > --- a/lib/efi_loader/efi_var_file.c
> > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> > > >                  return EFI_OUT_OF_RESOURCES;
> > > >          }
> > > >
> > > > +       ret = efi_set_blk_dev_to_system_partition();
> > > > +       if (ret != EFI_SUCCESS)
> > > > +               goto error;
> > > > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > > > +               goto error;
> > > > +
> > > >          ret = efi_set_blk_dev_to_system_partition();
> > > >          if (ret != EFI_SUCCESS)
> > > >                  goto error;
> > >
> > > This later call to efi_set_blk_dev_to_system_partition() can be
> > > removed since already done above.
> > >
> >
> > iirc fs_exists() unconditionally calls fs_close() so we need that double call
> >
> > Cheers
> > /Ilias
> > > Etienne
> > >
> > > > --
> > > > 2.38.1
> > > >
>

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Etienne Carriere]

On Thu, 19 Jan 2023 at 13:53, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Heinrich,
> On Thu, Jan 19, 2023 at 01:42:20PM +0100, Heinrich Schuchardt wrote:
> > On 1/19/23 12:45, Ilias Apalodimas wrote:
> > > Hi Etienne,
> > >
> > > On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> > > <etienne.carriere@linaro.org> wrote:
> > > >
> > > > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > When we try to load EFI variables from a file in the ESP partition and the
> > > > > file is missing We print a scary error looking like
> > > > > => printenv -e
> > > > > ** Unable to read file ubootefi.var **
> >
> > This message is in line fs/fat/fat.c:1297:
> > printf("** Unable to read file %s **\n", filename);
> >
> > The message is misplaced. If any message is written, it should be in
> > fat_read_at().
> >
> > But I think fat_read_at() should also be quiet. It is the task of the
> > calling application to shout out if a file cannot be read.
> >
> > I have not checked if a test in test/py/tests/test_fs/ relies on the
> > error message.
> >
> > > > > Failed to load EFI variables
> >
> > If there is a writable ESP, file ubootefi.var is created on first boot.
>
> Are you sure?  I think it's created the moment you create an EFI variable.
> So on a board that uses preseeded files you'll always see the message.

It's created once EFI variables are loaded from preseed.
Later boots load the variables from the file.

>
> Thanks
> /Ilias
> > So this message should only once in the lifecycle of the board.
> >
> > Afterwards this message indicates that something is really wrong.
> >
> > If board vendors doesn't like it, they can create a dummy file.
> >
> > Best regards
> >
> > Heinrich
> >
> > > > >
> > > > > This is not an error though since the file wasn't there to begin with.
> > > > > So silence the warning by aborting the load if the file is not there,
> > > > > instead of failing the load.
> > > > >
> > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > ---
> > > > >   lib/efi_loader/efi_var_file.c | 6 ++++++
> > > > >   1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > > > index 62e071bd8341..7d7141473634 100644
> > > > > --- a/lib/efi_loader/efi_var_file.c
> > > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> > > > >                  return EFI_OUT_OF_RESOURCES;
> > > > >          }
> > > > >
> > > > > +       ret = efi_set_blk_dev_to_system_partition();
> > > > > +       if (ret != EFI_SUCCESS)
> > > > > +               goto error;
> > > > > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > > > > +               goto error;
> > > > > +
> > > > >          ret = efi_set_blk_dev_to_system_partition();
> > > > >          if (ret != EFI_SUCCESS)
> > > > >                  goto error;
> > > >
> > > > This later call to efi_set_blk_dev_to_system_partition() can be
> > > > removed since already done above.
> > > >
> > >
> > > iirc fs_exists() unconditionally calls fs_close() so we need that double call
> > >
> > > Cheers
> > > /Ilias
> > > > Etienne
> > > >
> > > > > --
> > > > > 2.38.1
> > > > >
> >

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Ilias Apalodimas]

Hi Etienne,

On Thu, 19 Jan 2023 at 15:15, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> On Thu, 19 Jan 2023 at 13:53, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Heinrich,
> > On Thu, Jan 19, 2023 at 01:42:20PM +0100, Heinrich Schuchardt wrote:
> > > On 1/19/23 12:45, Ilias Apalodimas wrote:
> > > > Hi Etienne,
> > > >
> > > > On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> > > > <etienne.carriere@linaro.org> wrote:
> > > > >
> > > > > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > >
> > > > > > When we try to load EFI variables from a file in the ESP partition and the
> > > > > > file is missing We print a scary error looking like
> > > > > > => printenv -e
> > > > > > ** Unable to read file ubootefi.var **
> > >
> > > This message is in line fs/fat/fat.c:1297:
> > > printf("** Unable to read file %s **\n", filename);
> > >
> > > The message is misplaced. If any message is written, it should be in
> > > fat_read_at().
> > >
> > > But I think fat_read_at() should also be quiet. It is the task of the
> > > calling application to shout out if a file cannot be read.
> > >
> > > I have not checked if a test in test/py/tests/test_fs/ relies on the
> > > error message.
> > >
> > > > > > Failed to load EFI variables
> > >
> > > If there is a writable ESP, file ubootefi.var is created on first boot.
> >
> > Are you sure?  I think it's created the moment you create an EFI variable.
> > So on a board that uses preseeded files you'll always see the message.
>
> It's created once EFI variables are loaded from preseed.
> Later boots load the variables from the file.

Unless I am missing something, efi_set_variable_int() is the only
caller of efi_var_to_file() which creates that file. That only gets
called when a call to SetVariable is made.

Regards
/Ilias
>
> >
> > Thanks
> > /Ilias
> > > So this message should only once in the lifecycle of the board.
> > >
> > > Afterwards this message indicates that something is really wrong.
> > >
> > > If board vendors doesn't like it, they can create a dummy file.
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > > > >
> > > > > > This is not an error though since the file wasn't there to begin with.
> > > > > > So silence the warning by aborting the load if the file is not there,
> > > > > > instead of failing the load.
> > > > > >
> > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > ---
> > > > > >   lib/efi_loader/efi_var_file.c | 6 ++++++
> > > > > >   1 file changed, 6 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > > > > index 62e071bd8341..7d7141473634 100644
> > > > > > --- a/lib/efi_loader/efi_var_file.c
> > > > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > > > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> > > > > >                  return EFI_OUT_OF_RESOURCES;
> > > > > >          }
> > > > > >
> > > > > > +       ret = efi_set_blk_dev_to_system_partition();
> > > > > > +       if (ret != EFI_SUCCESS)
> > > > > > +               goto error;
> > > > > > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > > > > > +               goto error;
> > > > > > +
> > > > > >          ret = efi_set_blk_dev_to_system_partition();
> > > > > >          if (ret != EFI_SUCCESS)
> > > > > >                  goto error;
> > > > >
> > > > > This later call to efi_set_blk_dev_to_system_partition() can be
> > > > > removed since already done above.
> > > > >
> > > >
> > > > iirc fs_exists() unconditionally calls fs_close() so we need that double call
> > > >
> > > > Cheers
> > > > /Ilias
> > > > > Etienne
> > > > >
> > > > > > --
> > > > > > 2.38.1
> > > > > >
> > >

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Ilias Apalodimas]

Right,
I'll answer to myself here.

On Thu, 19 Jan 2023 at 15:26, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Etienne,
>
> On Thu, 19 Jan 2023 at 15:15, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > On Thu, 19 Jan 2023 at 13:53, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Heinrich,
> > > On Thu, Jan 19, 2023 at 01:42:20PM +0100, Heinrich Schuchardt wrote:
> > > > On 1/19/23 12:45, Ilias Apalodimas wrote:
> > > > > Hi Etienne,
> > > > >
> > > > > On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> > > > > <etienne.carriere@linaro.org> wrote:
> > > > > >
> > > > > > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > >
> > > > > > > When we try to load EFI variables from a file in the ESP partition and the
> > > > > > > file is missing We print a scary error looking like
> > > > > > > => printenv -e
> > > > > > > ** Unable to read file ubootefi.var **
> > > >
> > > > This message is in line fs/fat/fat.c:1297:
> > > > printf("** Unable to read file %s **\n", filename);
> > > >
> > > > The message is misplaced. If any message is written, it should be in
> > > > fat_read_at().
> > > >
> > > > But I think fat_read_at() should also be quiet. It is the task of the
> > > > calling application to shout out if a file cannot be read.
> > > >
> > > > I have not checked if a test in test/py/tests/test_fs/ relies on the
> > > > error message.
> > > >
> > > > > > > Failed to load EFI variables
> > > >
> > > > If there is a writable ESP, file ubootefi.var is created on first boot.
> > >
> > > Are you sure?  I think it's created the moment you create an EFI variable.
> > > So on a board that uses preseeded files you'll always see the message.
> >
> > It's created once EFI variables are loaded from preseed.
> > Later boots load the variables from the file.
>
> Unless I am missing something, efi_set_variable_int() is the only
> caller of efi_var_to_file() which creates that file. That only gets
> called when a call to SetVariable is made.
>

We do set the PlatformLang on u-boot.  But as Heinrich pointed out,
there's a check in that setvariable that shouldn't be there
(efi_obj_list_initialized == EFI_SUCCESS).  So the file ends up not
being created

Cheers
/Ilias
> Regards
> /Ilias
> >
> > >
> > > Thanks
> > > /Ilias
> > > > So this message should only once in the lifecycle of the board.
> > > >
> > > > Afterwards this message indicates that something is really wrong.
> > > >
> > > > If board vendors doesn't like it, they can create a dummy file.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > > > >
> > > > > > > This is not an error though since the file wasn't there to begin with.
> > > > > > > So silence the warning by aborting the load if the file is not there,
> > > > > > > instead of failing the load.
> > > > > > >
> > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > > ---
> > > > > > >   lib/efi_loader/efi_var_file.c | 6 ++++++
> > > > > > >   1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > > > > > index 62e071bd8341..7d7141473634 100644
> > > > > > > --- a/lib/efi_loader/efi_var_file.c
> > > > > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > > > > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> > > > > > >                  return EFI_OUT_OF_RESOURCES;
> > > > > > >          }
> > > > > > >
> > > > > > > +       ret = efi_set_blk_dev_to_system_partition();
> > > > > > > +       if (ret != EFI_SUCCESS)
> > > > > > > +               goto error;
> > > > > > > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > > > > > > +               goto error;
> > > > > > > +
> > > > > > >          ret = efi_set_blk_dev_to_system_partition();
> > > > > > >          if (ret != EFI_SUCCESS)
> > > > > > >                  goto error;
> > > > > >
> > > > > > This later call to efi_set_blk_dev_to_system_partition() can be
> > > > > > removed since already done above.
> > > > > >
> > > > >
> > > > > iirc fs_exists() unconditionally calls fs_close() so we need that double call
> > > > >
> > > > > Cheers
> > > > > /Ilias
> > > > > > Etienne
> > > > > >
> > > > > > > --
> > > > > > > 2.38.1
> > > > > > >
> > > >

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Etienne Carriere]

On Thu, 19 Jan 2023 at 14:47, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Right,
> I'll answer to myself here.
>
> On Thu, 19 Jan 2023 at 15:26, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Etienne,
> >
> > On Thu, 19 Jan 2023 at 15:15, Etienne Carriere
> > <etienne.carriere@linaro.org> wrote:
> > >
> > > On Thu, 19 Jan 2023 at 13:53, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > Hi Heinrich,
> > > > On Thu, Jan 19, 2023 at 01:42:20PM +0100, Heinrich Schuchardt wrote:
> > > > > On 1/19/23 12:45, Ilias Apalodimas wrote:
> > > > > > Hi Etienne,
> > > > > >
> > > > > > On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> > > > > > <etienne.carriere@linaro.org> wrote:
> > > > > > >
> > > > > > > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> > > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > > >
> > > > > > > > When we try to load EFI variables from a file in the ESP partition and the
> > > > > > > > file is missing We print a scary error looking like
> > > > > > > > => printenv -e
> > > > > > > > ** Unable to read file ubootefi.var **
> > > > >
> > > > > This message is in line fs/fat/fat.c:1297:
> > > > > printf("** Unable to read file %s **\n", filename);
> > > > >
> > > > > The message is misplaced. If any message is written, it should be in
> > > > > fat_read_at().
> > > > >
> > > > > But I think fat_read_at() should also be quiet. It is the task of the
> > > > > calling application to shout out if a file cannot be read.
> > > > >
> > > > > I have not checked if a test in test/py/tests/test_fs/ relies on the
> > > > > error message.
> > > > >
> > > > > > > > Failed to load EFI variables
> > > > >
> > > > > If there is a writable ESP, file ubootefi.var is created on first boot.
> > > >
> > > > Are you sure?  I think it's created the moment you create an EFI variable.
> > > > So on a board that uses preseeded files you'll always see the message.
> > >
> > > It's created once EFI variables are loaded from preseed.
> > > Later boots load the variables from the file.
> >
> > Unless I am missing something, efi_set_variable_int() is the only
> > caller of efi_var_to_file() which creates that file. That only gets
> > called when a call to SetVariable is made.
> >
>
> We do set the PlatformLang on u-boot.  But as Heinrich pointed out,
> there's a check in that setvariable that shouldn't be there
> (efi_obj_list_initialized == EFI_SUCCESS).  So the file ends up not
> being created

Ok, I saw the patch in the ML.
Thanks for the details.

etienne

>
> Cheers
> /Ilias
> > Regards
> > /Ilias
> > >
> > > >
> > > > Thanks
> > > > /Ilias
> > > > > So this message should only once in the lifecycle of the board.
> > > > >
> > > > > Afterwards this message indicates that something is really wrong.
> > > > >
> > > > > If board vendors doesn't like it, they can create a dummy file.
> > > > >
> > > > > Best regards
> > > > >
> > > > > Heinrich
> > > > >
> > > > > > > >
> > > > > > > > This is not an error though since the file wasn't there to begin with.
> > > > > > > > So silence the warning by aborting the load if the file is not there,
> > > > > > > > instead of failing the load.
> > > > > > > >
> > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > > > ---
> > > > > > > >   lib/efi_loader/efi_var_file.c | 6 ++++++
> > > > > > > >   1 file changed, 6 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > > > > > > index 62e071bd8341..7d7141473634 100644
> > > > > > > > --- a/lib/efi_loader/efi_var_file.c
> > > > > > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > > > > > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> > > > > > > >                  return EFI_OUT_OF_RESOURCES;
> > > > > > > >          }
> > > > > > > >
> > > > > > > > +       ret = efi_set_blk_dev_to_system_partition();
> > > > > > > > +       if (ret != EFI_SUCCESS)
> > > > > > > > +               goto error;
> > > > > > > > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > > > > > > > +               goto error;
> > > > > > > > +
> > > > > > > >          ret = efi_set_blk_dev_to_system_partition();
> > > > > > > >          if (ret != EFI_SUCCESS)
> > > > > > > >                  goto error;
> > > > > > >
> > > > > > > This later call to efi_set_blk_dev_to_system_partition() can be
> > > > > > > removed since already done above.
> > > > > > >
> > > > > >
> > > > > > iirc fs_exists() unconditionally calls fs_close() so we need that double call
> > > > > >
> > > > > > Cheers
> > > > > > /Ilias
> > > > > > > Etienne
> > > > > > >
> > > > > > > > --
> > > > > > > > 2.38.1
> > > > > > > >
> > > > >

Re: [PATCH] efi_loader: silence 'Failed to load EFI variables' if the file is missing

[Etienne Carriere]

On Thu, 19 Jan 2023 at 12:46, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Etienne,
>
> On Thu, 19 Jan 2023 at 13:17, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > When we try to load EFI variables from a file in the ESP partition and the
> > > file is missing We print a scary error looking like
> > > => printenv -e
> > > ** Unable to read file ubootefi.var **
> > > Failed to load EFI variables
> > >
> > > This is not an error though since the file wasn't there to begin with.
> > > So silence the warning by aborting the load if the file is not there,
> > > instead of failing the load.
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > ---
> > >  lib/efi_loader/efi_var_file.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > index 62e071bd8341..7d7141473634 100644
> > > --- a/lib/efi_loader/efi_var_file.c
> > > +++ b/lib/efi_loader/efi_var_file.c
> > > @@ -223,6 +223,12 @@ efi_status_t efi_var_from_file(void)
> > >                 return EFI_OUT_OF_RESOURCES;
> > >         }
> > >
> > > +       ret = efi_set_blk_dev_to_system_partition();
> > > +       if (ret != EFI_SUCCESS)
> > > +               goto error;
> > > +       if (!fs_exists(EFI_VAR_FILE_NAME))
> > > +               goto error;
> > > +
> > >         ret = efi_set_blk_dev_to_system_partition();
> > >         if (ret != EFI_SUCCESS)
> > >                 goto error;
> >
> > This later call to efi_set_blk_dev_to_system_partition() can be
> > removed since already done above.
> >
>
> iirc fs_exists() unconditionally calls fs_close() so we need that double call

Ok. Sorry.

etienne

>
> Cheers
> /Ilias
> > Etienne
> >
> > > --
> > > 2.38.1
> > >