On Fri, 17 Feb 2023 at 21:14, Pedro Falcato <pedro.falcato@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 <ardb@kernel.org>

Cc: Marvin Häuser <mhaeuser@posteo.de>

Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.")

Signed-off-by: Pedro Falcato <pedro.falcato@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

- );

-

- if (EFI_ERROR (Status)) {

- return Status;

- }

-

- return Ext4InitialiseUnicodeCollation (ImageHandle);

+ return EfiLibInstallAllDriverProtocols2 (

+ ImageHandle,

+ SystemTable,

+ &gExt4BindingProtocol,

+ ImageHandle,

+ &gExt4ComponentName,

+ &gExt4ComponentName2,

+ 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.