> On 17. Feb 2023, at 15:29, Ard Biesheuvel <a...@kernel.org> wrote: > > On Fri, 17 Feb 2023 at 15:05, Marvin Häuser <mhaeu...@posteo.de> wrote: >> >> Hi Ard, >> >> Thank you! Is "1/4" a mistake or did I miss the other 3? :) > > Oops. > > It was part of some RPi4 patches but I decided to send it out by itself. > > >> Comments inline. >> >> On 17. Feb 2023, at 12:12, Ard Biesheuvel <a...@kernel.org> wrote: >> >> 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. >> >> >> Right. FatPkg solves this by probing for the protocol in Start() [1], which >> 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 fine, >> honestly. >> >> [1] >> https://github.com/tianocore/edk2/blob/02fcfdce1e5ce86f1951191883e7e30de5aa08be/FatPkg/EnhancedFatDxe/Fat.c#L381 >> https://github.com/tianocore/edk2/blob/02fcfdce1e5ce86f1951191883e7e30de5aa08be/FatPkg/EnhancedFatDxe/Fat.inf#L19 >> > > I'd prefer using existing features for this, rather than open coding it.
It‘s not like we replicate a feature, we just move a function call to suit the control flow better. I‘d like a consistent solution and the FatPkg one looks fine to me. > > >> - MODULE_TYPE = UEFI_DRIVER >> + MODULE_TYPE = DXE_DRIVER >> >> >> Is it not unidiomatic to use the UEFI Driver Binding model (UEFI) in a DXE >> driver (UEFI PI)? >> > > 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 abusing the types to utilise DXE concepts. I‘m not opposed to such things in general, but here it looks unnecessary. It doesn’t help I’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 alternatives later. In the end, it‘s Pedro‘s call anyway. > > >> +[Depex] >> + gEfiUnicodeCollationProtocolGuid OR gEfiUnicodeCollation2ProtocolGuid >> >> >> Hmm, this will have the side effect that Ext4Dxe may load before (some of) >> the architectural protocols, right (modulo implicit dependencies via the UC >> protocols)? This would need some careful analysis, or we need to add all of >> the architectural protocols to preserve the old behaviour. >> > > 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 deal. Best regards, Marvin -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100322): https://edk2.groups.io/g/devel/message/100322 Mute This Topic: https://groups.io/mt/97025926/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
