Re: [edk2-devel] [PATCH edk2-platforms 1/1] SbsaQemu: use FEAT_RNG for EFI_RNG_PROTOCOL

["Ard Biesheuvel" <ardb@kernel.org>]

On Mon, 1 Jul 2024 at 15:28, Leif Lindholm <quic_llindhol@quicinc.com> wrote:
>
> On 2024-07-01 13:58, Marcin Juszkiewicz wrote:
>
> >>> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> >>> b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> >>> index 9306986bf7c0..3463e5c7a635 100644
> >>> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> >>> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> >>> @@ -148,7 +148,9 @@ [LibraryClasses.common]
> >>>     #
> >>
> >> Since sbsa-ref still supports processors without FEAT_RNG, this may
> >> cause unexpected breakages for some users.
> >
> > That's why I sent it as more of RFC than changes for merging.
> >
> >> Could we first of all conditionalise this change:
> >>
> >> [Defines]
> >> ...
> >>    DEFINE_DEBUG_PRINT_ERROR_LEVEL = ...
> >>    DEFINE FEATRNG_ENABLE         = TRUE
> >>
> >> so that someone who still wishes to run tests against older cpus can
> >> still do so through a rebuild with -D FEATRNG_ENABLE=FALSE
> >
> > Is there a way to load both BaseRngLib and BaseRngLibTimerLib and switch
> > between them depending on availability of FEAT_RNG?
>
> Not without severe hackery. The library is statically linked into RngDxe.
>
> >>> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> >>> b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> >>> index b35f42e11aa4..51a1ef8519f9 100644
> >>> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> >>> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
> >>> @@ -192,6 +192,7 @@ [FV.FvMain]
> >>>     INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> >>>     INF OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> >>>     INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> >>> +  INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
> >>
> >> Second:
> >> What is the failure mode of running the BaseRngLib flavour on cpus
> >> that don't support FEAT_RNG? RngDxe itself seems to do the right
> >> thing, but do we get any warning messages or will certain operations
> >> now fail silently?
> >
> > On FEAT_RNG cores we get:
> >
> > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 101FAD68798
> > ProtectUefiImageCommon - 0xFAD683C0
> >    - 0x00000101FBBDB000 - 0x0000000000007000
> > ArmTrngLib could not be correctly initialized.
> > InstallProtocolInterface: 3152BCA5-EADE-433D-862E-C01CDC291F44 101FBBE0020
> > Loading driver B601F8C4-43B7-4784-95B1-F4226CB40CEE
> >
> >
> > On core without FEAT_RNG:
> >
> > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 101FAD68798
> > ProtectUefiImageCommon - 0xFAD683C0
> >    - 0x00000101FBBDB000 - 0x0000000000007000
> > ArmTrngLib could not be correctly initialized.
> > Error: Image at 101FBBDB000 start failed: 00000001
> > Loading driver B601F8C4-43B7-4784-95B1-F4226CB40CEE
>
> But it still keeps booting correctly after that? With some missing
> functionality?
>
> > So there is some kind of information but you need to know what
> > to look for ;(
>
> Ard: would you be opposed to putting a DEBUG print and/or an ASSERT in
> BaseRngLibContructor if mRndrSupported == 0?
>
> An alternative would be to place a test and noisy warning inside
> SbsaQemuPlatformDxe.
>

I'm not sure I follow what we are trying to fix here. RngDxe will
simply not load if BaseRngLib does not find FEAT_RNG support.

If the issue is other, existing users of RngLib that are currently
served by BaseRngTimerLib, we could make the library class resolutions
introduced here local the RngDxe driver, using

SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf {
  <LibraryClasses>
    RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf
    ArmTrngLib|ArmPkg/Library/ArmTrngLib/ArmTrngLib.inf
    ArmMonitorLib|ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.inf
}


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119738): https://edk2.groups.io/g/devel/message/119738
Mute This Topic: https://groups.io/mt/106909459/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-