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

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On 2024-06-27 15:22, Marcin Juszkiewicz wrote:
> By default we have Neoverse-N2 cpu which supports FEAT_RNG feature.
> 
> Commit 5de5e230a80bed083360da95ba16a2c4a001620d (in EDK2) enabled that for
> ArmVirt platform.
> 
> RNDR is implemented by both Neoverse-N2 and 'max' cpu implemented by QEMU.
> Other cpu models lack it which prevents the RngDxe driver from running,
> resulting in the same situation as before.
> 
> TRNG is not implemented in TCG mode but is required by RngDxe to run.

This commit also adds RngDxe for this platform, which neither the short 
nor the long description mentions.

> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 6 +++++-
>   Platform/Qemu/SbsaQemu/SbsaQemu.fdf | 1 +
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> 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.

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

>     IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>     OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf

!if $(FEATRNG_ENABLE) == TRUE
   RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf
!else
   RngLib|MdeModulePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
!endif
   ArmTrngLib|ArmPkg/Library/ArmTrngLib/ArmTrngLib.inf
   ArmMonitorLib|ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.inf

>     BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>   
>     #
> @@ -660,6 +662,8 @@ [Components.common]
>     OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
>     MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>     Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuHighMemDxe/SbsaQemuHighMemDxe.inf
> +  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
> +

Spurious added newline.

>   
>     #
>     # FAT filesystem + GPT/MBR partitioning
> 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?

/
     Leif

>   
>     #
>     # FAT filesystem + GPT/MBR partitioning + UDF filesystem



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119735): https://edk2.groups.io/g/devel/message/119735
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]
-=-=-=-=-=-=-=-=-=-=-=-