From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web11.8191.1676645702211468526 for ; Fri, 17 Feb 2023 06:55:02 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=MtfDXR3e; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 732CE240411 for ; Fri, 17 Feb 2023 15:54:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1676645699; bh=sr4p14Snt+qRSL76F6aMdEsx95UNf3DaNd4MZ0YeCM8=; h=From:Subject:Date:Cc:To:From; b=MtfDXR3eSoNogufbpbmwxsQlB+BGcJYwbSHZGpZS0nY05522SYiixWQxcWP2ZK3nm Iqa1/wSRy8h6+fRIOTvAYWdmKAp0xxPMBFQEO3v++9dgedaC80IRAirDmkkJlh16mq rTGyyTEgQsPo3DMNS80v0kYtKR0C0oMjiNBkz+tB1+oTfRL5liFIxzKutaR4CgfGkA kOJH/FkbZFwPkHaYr75hmrya9ylWfFwlEs/BqEoqe8iUQKWCHOsZai/RChD1Ccrqfa LRkutNC2dq3PTJpToT6LNvE85vTdXohLPdoT3KXS7efInj2yLIPImJRCz0EMWADnxD mlnCrWrQPrqZA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4PJFGV0WJKz9rxL; Fri, 17 Feb 2023 15:54:58 +0100 (CET) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Mime-Version: 1.0 (1.0) Subject: Re: [edk2-devel] [PATCH edk2-platforms 1/4] Ext4Pkg: Use depex for unicode collation protocols Date: Fri, 17 Feb 2023 14:54:57 +0000 Message-Id: <2DE5E6BC-4AAE-4E31-91E2-B47B2F93DA43@posteo.de> References: Cc: devel@edk2.groups.io, Pedro Falcato In-Reply-To: To: Ard Biesheuvel Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > On 17. Feb 2023, at 15:29, Ard Biesheuvel wrote: >=20 > =EF=BB=BFOn Fri, 17 Feb 2023 at 15:05, Marvin H=C3=A4user wrote: >>=20 >> Hi Ard, >>=20 >> Thank you! Is "1/4" a mistake or did I miss the other 3? :) >=20 > Oops. >=20 > It was part of some RPi4 patches but I decided to send it out by itself. >=20 >=20 >> Comments inline. >>=20 >> On 17. Feb 2023, at 12:12, Ard Biesheuvel wrote: >>=20 >> The Unicode collation protocols, however, are different: loading the >> driver will fail if neither of those are present. So they are not >> TO_START protocols, and they need to be mentioned in the DEPEX so that >> the DXE core will not dispatch the driver before the producers of the >> prerequisite protocols have been dispatched. Also, mark them as >> SOMETIMES_CONSUMES, as only one of the two is required. >>=20 >>=20 >> Right. FatPkg solves this by probing for the protocol in Start() [1], whi= ch should guarantee that all entry points have been executed first, right? I= 'd prefer a universal and consistent solution to the issue and this looks fi= ne, honestly. >>=20 >> [1] >> https://github.com/tianocore/edk2/blob/02fcfdce1e5ce86f1951191883e7e30de5= aa08be/FatPkg/EnhancedFatDxe/Fat.c#L381 >> https://github.com/tianocore/edk2/blob/02fcfdce1e5ce86f1951191883e7e30de5= aa08be/FatPkg/EnhancedFatDxe/Fat.inf#L19 >>=20 >=20 > I'd prefer using existing features for this, rather than open coding it. It=E2=80=98s not like we replicate a feature, we just move a function call t= o suit the control flow better. I=E2=80=98d like a consistent solution and t= he FatPkg one looks fine to me. >=20 >=20 >> - MODULE_TYPE =3D UEFI_DRIVER >> + MODULE_TYPE =3D DXE_DRIVER >>=20 >>=20 >> Is it not unidiomatic to use the UEFI Driver Binding model (UEFI) in a DX= E driver (UEFI PI)? >>=20 >=20 > Perhaps. But having a hard dependency on protocols beyond the default > set defined for UEFI drivers is arguably even worse. This is still very much a UEFI driver in a logical sense, this is just abusi= ng the types to utilise DXE concepts. I=E2=80=98m not opposed to such things= in general, but here it looks unnecessary. It doesn=E2=80=99t help I=E2=80=99= m not a big fan of the DXE dispatcher to begin with. :) I agree the dependency is awkward, but I have to check the reason and altern= atives later. In the end, it=E2=80=98s Pedro=E2=80=98s call anyway. >=20 >=20 >> +[Depex] >> + gEfiUnicodeCollationProtocolGuid OR gEfiUnicodeCollation2ProtocolGuid >>=20 >>=20 >> Hmm, this will have the side effect that Ext4Dxe may load before (some of= ) the architectural protocols, right (modulo implicit dependencies via the U= C protocols)? This would need some careful analysis, or we need to add all o= f the architectural protocols to preserve the old behaviour. >>=20 >=20 > The ext4 driver does nothing except install protocols in its > entrypoint, and everything else that happens is under the control of > the UEFI driver model, afaict. I guess. There also is a chance that libraries like DebugLib enable advanced= features only as core services become available. But probably not a big dea= l. Best regards, Marvin=