?On Mon, Feb 20, 2023 at 8:00 AM Marvin Häuser <mhaeu...@posteo.de> wrote: > > > On 20. Feb 2023, at 08:52, Ard Biesheuvel <a...@kernel.org> wrote: > > On Fri, 17 Feb 2023 at 21:14, Pedro Falcato <pedro.falc...@gmail.com> wrote: > > > There have been reports[1] of failures to boot due to unicode collation > > protocols not being available at Ext4Dxe load time. Therefore, attempt > > to initialize unicode collation at Start() time, like done previously in > > FatPkg/EnhancedFatDxe. By doing so, we move collation initialization > > to BDS, where the module responsible for protocol installation should > > have already been loaded and ran. > > > [1]: https://edk2.groups.io/g/devel/message/100312 > > > Cc: Ard Biesheuvel <a...@kernel.org> > > Cc: Marvin Häuser <mhaeu...@posteo.de> > > Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.") > > Signed-off-by: Pedro Falcato <pedro.falc...@gmail.com> > > --- > > RFC-ish patch that implements one of the possibilities discussed in [1]. > > Please test and give feedback. > > > Odd that this issue was never caught in any other platform. Do the RPi > platforms have > > something funky going on? > > > > It could be something as simple as the order the drivers appear in the > .FDF file. > > Features/Ext4Pkg/Ext4Dxe/Collation.c | 18 +++++++++- > > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 51 +++++++++++++++------------- > > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 11 ++++++ > > 3 files changed, 56 insertions(+), 24 deletions(-) > > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Collation.c > b/Features/Ext4Pkg/Ext4Dxe/Collation.c > > index 91d172b1cb89..65c34c33ea93 100644 > > --- a/Features/Ext4Pkg/Ext4Dxe/Collation.c > > +++ b/Features/Ext4Pkg/Ext4Dxe/Collation.c > > @@ -1,7 +1,7 @@ > > /** @file > > Unicode collation routines > > > - Copyright (c) 2021 Pedro Falcato All rights reserved. > > + Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved. > > Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved. > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -9,6 +9,7 @@ > > > #include <Uefi.h> > > > +#include <Library/DebugLib.h> > > #include <Library/UefiLib.h> > > #include <Library/UefiBootServicesTableLib.h> > > #include <Library/MemoryAllocationLib.h> > > @@ -169,5 +170,20 @@ Ext4StrCmpInsensitive ( > > IN CHAR16 *Str2 > > ) > > { > > + ASSERT (gUnicodeCollationInterface != NULL); > > return gUnicodeCollationInterface->StriColl (gUnicodeCollationInterface, > Str1, Str2); > > } > > + > > +/** > > + Check if unicode collation is initialized > > + > > + @retval TRUE if Ext4InitialiseUnicodeCollation() was already called > successfully > > + @retval FALSE if Ext4InitialiseUnicodeCollation() was not yet called > successfully > > +**/ > > +BOOLEAN > > +Ext4IsCollationInitialized ( > > + VOID > > + ) > > +{ > > + return gUnicodeCollationInterface != NULL; > > +} > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c > b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c > > index 2a4f5a7bd0ef..de0d633febfb 100644 > > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c > > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c > > @@ -1,7 +1,7 @@ > > /** @file > > Driver entry point > > > - Copyright (c) 2021 Pedro Falcato All rights reserved. > > + Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > @@ -513,26 +513,18 @@ Ext4EntryPoint ( > > IN EFI_SYSTEM_TABLE *SystemTable > > ) > > { > > - EFI_STATUS Status; > > - > > - Status = EfiLibInstallAllDriverProtocols2 ( > > - ImageHandle, > > - SystemTable, > > - &gExt4BindingProtocol, > > - ImageHandle, > > - &gExt4ComponentName, > > - &gExt4ComponentName2, > > - NULL, > > - NULL, > > - NULL, > > - NULL > > - ); > > - > > - if (EFI_ERROR (Status)) { > > - return Status; > > - } > > - > > - return Ext4InitialiseUnicodeCollation (ImageHandle); > > + return EfiLibInstallAllDriverProtocols2 ( > > + ImageHandle, > > + SystemTable, > > + &gExt4BindingProtocol, > > + ImageHandle, > > + &gExt4ComponentName, > > + &gExt4ComponentName2, > > + NULL, > > + NULL, > > + NULL, > > + NULL > > + ); > > } > > > /** > > @@ -761,6 +753,19 @@ Ext4Bind ( > > BlockIo = NULL; > > DiskIo = NULL; > > > + // Note: We initialize collation here since this is called in BDS, when we > are likely > > + // to have the Unicode Collation protocols available. > > + if (!Ext4IsCollationInitialized ()) { > > + Status = Ext4InitialiseUnicodeCollation (BindingProtocol->ImageHandle); > > + if (EFI_ERROR (Status)) { > > + // Lets throw a loud error into the log > > + // It is very unlikely something like this may fire out of the blue. > Chances are either > > + // the platform configuration is wrong, or we are. > > + DEBUG ((DEBUG_ERROR, "[ext4] Error: Unicode Collation not available - > failure to Start() - error %r\n", Status)); > > + goto Error; > > + } > > + } > > + > > > This should work, although I'm not sure I see the point of the extra check. > > > Thanks for checking! Which extra check, IsInit? It’s just a weirdly abstract > shortcut to not go through protocol lookup again when it already succeeded > before. > > We can probably merge it as-is for now. Though, would you mind commenting on > this: https://edk2.groups.io/g/devel/message/100348 (the first part, about > static linking)? Thanks! > > Best regards, > Marvin
Marvin, Do you want me to spin up a quick v2 without the Ext4IsCollationInitialized? Just doing the check internally in Ext4InitialiseUnicodeCollation. -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100422): https://edk2.groups.io/g/devel/message/100422 Mute This Topic: https://groups.io/mt/97037199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
