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.
Acked-by: Ard Biesheuvel <a...@kernel.org>
> Status = gBS->OpenProtocol (
> ControllerHandle,
> &gEfiDiskIoProtocolGuid,
> @@ -774,7 +779,7 @@ Ext4Bind (
> goto Error;
> }
>
> - DEBUG ((DEBUG_INFO, "[Ext4] Controller supports DISK_IO\n"));
> + DEBUG ((DEBUG_INFO, "[ext4] Controller supports DISK_IO\n"));
>
> Status = gBS->OpenProtocol (
> ControllerHandle,
> @@ -787,7 +792,7 @@ Ext4Bind (
> // It's okay to not support DISK_IO2
>
> if (DiskIo2 != NULL) {
> - DEBUG ((DEBUG_INFO, "[Ext4] Controller supports DISK_IO2\n"));
> + DEBUG ((DEBUG_INFO, "[ext4] Controller supports DISK_IO2\n"));
> }
>
> Status = gBS->OpenProtocol (
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> index 786e20f49fe1..93f6cf01fe06 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -947,6 +947,17 @@ Ext4InitialiseUnicodeCollation (
> EFI_HANDLE DriverHandle
> );
>
> +/**
> + 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
> + );
> +
> /**
> Does a case-insensitive string comparison. Refer to
> EFI_UNICODE_COLLATION_PROTOCOL's StriColl for more details.
> --
> 2.39.2
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100360): https://edk2.groups.io/g/devel/message/100360
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]
-=-=-=-=-=-=-=-=-=-=-=-
