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 >