From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.8218.1676879572488155887 for ; Sun, 19 Feb 2023 23:52:52 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=SJ7Ql6EU; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E90D060C79 for ; Mon, 20 Feb 2023 07:52:51 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 55D30C4339B for ; Mon, 20 Feb 2023 07:52:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1676879571; bh=3lGy9NXWWvXQldPE5Q4MPcC8cZq98FvouF9amRFmCuk=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=SJ7Ql6EUcz0Lm5qi3woSuEXyR11EOZxki67ynh84cRC2y6bOSspWMg8rcnmuLcov1 LlPVESi9k/X6NUArN7HjLePnvl8qwykH/ucMILQCtRQ5Kfh1QHICoqPVGlv3Txy/UJ 3krlB0dBK92jFGjthUd9BcXdhL41+kBno9Utmv09QTB4i/mkB7pi4ncG1Owp+UzYcs 2dP3GRggm1M1YUCMeRtToqTLejaXbU5E7Km2u09vUcp6WCOmRkQ506qNhFJjVuxl3y dfCs0BdF/pASOtdhIht0BOJxqvHUBc1r4znj2/LSP1KqIfZbhSaRuODfVZSFpVonWW YjYTpoqImncuQ== Received: by mail-lj1-f180.google.com with SMTP id z5so261879ljc.8 for ; Sun, 19 Feb 2023 23:52:51 -0800 (PST) X-Gm-Message-State: AO0yUKWtz7vuaUnd9l2t0IOIFs/9M3Dqki5eEQ5gOXz1wbQ2QXqCGr14 9vCK9ersbx3RQ70lqyTuj+b/TDlDDknJ/1wavDc= X-Google-Smtp-Source: AK7set8p7s+LNynEV0sq4x/NdRvketOZxx+KpoCJUlsXWMl9W+edT07FvnTK6LnFyArkb4OW21A1xiULXxLX8U471Dc= X-Received: by 2002:a2e:a99b:0:b0:293:4ba5:f626 with SMTP id x27-20020a2ea99b000000b002934ba5f626mr882940ljq.2.1676879569324; Sun, 19 Feb 2023 23:52:49 -0800 (PST) MIME-Version: 1.0 References: <20230217201427.139581-1-pedro.falcato@gmail.com> In-Reply-To: <20230217201427.139581-1-pedro.falcato@gmail.com> From: "Ard Biesheuvel" Date: Mon, 20 Feb 2023 08:52:40 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/1] Ext4Pkg: Move unicode collation initialization to Start() To: Pedro Falcato Cc: devel@edk2.groups.io, =?UTF-8?Q?Marvin_H=C3=A4user?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 17 Feb 2023 at 21:14, Pedro Falcato 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 > Cc: Marvin H=C3=A4user > Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.") > Signed-off-by: Pedro Falcato > --- > 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 p= latforms 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/Ext4= Dxe/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 > > +#include > #include > #include > #include > @@ -169,5 +170,20 @@ Ext4StrCmpInsensitive ( > IN CHAR16 *Str2 > ) > { > + ASSERT (gUnicodeCollationInterface !=3D NULL); > return gUnicodeCollationInterface->StriColl (gUnicodeCollationInterfac= e, Str1, Str2); > } > + > +/** > + Check if unicode collation is initialized > + > + @retval TRUE if Ext4InitialiseUnicodeCollation() was already called s= uccessfully > + @retval FALSE if Ext4InitialiseUnicodeCollation() was not yet called = successfully > +**/ > +BOOLEAN > +Ext4IsCollationInitialized ( > + VOID > + ) > +{ > + return gUnicodeCollationInterface !=3D NULL; > +} > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c b/Features/Ext4Pkg/Ext4Dx= e/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 =3D 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 =3D NULL; > DiskIo =3D NULL; > > + // Note: We initialize collation here since this is called in BDS, whe= n we are likely > + // to have the Unicode Collation protocols available. > + if (!Ext4IsCollationInitialized ()) { > + Status =3D Ext4InitialiseUnicodeCollation (BindingProtocol->ImageHan= dle); > + if (EFI_ERROR (Status)) { > + // Lets throw a loud error into the log > + // It is very unlikely something like this may fire out of the blu= e. Chances are either > + // the platform configuration is wrong, or we are. > + DEBUG ((DEBUG_ERROR, "[ext4] Error: Unicode Collation not availabl= e - 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 > Status =3D 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 =3D gBS->OpenProtocol ( > ControllerHandle, > @@ -787,7 +792,7 @@ Ext4Bind ( > // It's okay to not support DISK_IO2 > > if (DiskIo2 !=3D NULL) { > - DEBUG ((DEBUG_INFO, "[Ext4] Controller supports DISK_IO2\n")); > + DEBUG ((DEBUG_INFO, "[ext4] Controller supports DISK_IO2\n")); > } > > Status =3D gBS->OpenProtocol ( > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dx= e/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 s= uccessfully > + @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 >