Right, I'll answer to myself here. On Thu, 19 Jan 2023 at 15:26, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Etienne, > > On Thu, 19 Jan 2023 at 15:15, Etienne Carriere > <etienne.carri...@linaro.org> wrote: > > > > On Thu, 19 Jan 2023 at 13:53, Ilias Apalodimas > > <ilias.apalodi...@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.carri...@linaro.org> wrote: > > > > > > > > > > > > On Wed, 18 Jan 2023 at 17:12, Ilias Apalodimas > > > > > > <ilias.apalodi...@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.apalodi...@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 > > > > > > > > > > >