Re: [PATCH v5 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

["Laszlo Ersek" <lersek@redhat.com>]

On 08/12/20 04:21, matthewfcarlson@gmail.com wrote:
> From: Matthew Carlson <macarl@microsoft.com>
> 
> Updates the DSC's for Ovmf based platforms to add a RngLib that uses the
> TimerLib. This is due to a later change that adds TimerLib as a dependency
> for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
> previously and it is recommended to switch to a better source of
> entropy than the system's performance counter.
> 
> Ref: https://github.com/tianocore/edk2/pull/845
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> ---
>  OvmfPkg/Bhyve/BhyvePkgX64.dsc | 1 +
>  OvmfPkg/OvmfPkgIa32.dsc       | 1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc    | 2 ++
>  OvmfPkg/OvmfPkgX64.dsc        | 1 +
>  OvmfPkg/OvmfXen.dsc           | 1 +
>  5 files changed, 6 insertions(+)
> 
> diff --git a/OvmfPkg/Bhyve/BhyvePkgX64.dsc b/OvmfPkg/Bhyve/BhyvePkgX64.dsc
> index 8eba48d109a3..4b1fbb361b28 100644
> --- a/OvmfPkg/Bhyve/BhyvePkgX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyvePkgX64.dsc
> @@ -185,6 +185,7 @@
>  !else
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>  !endif
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    PlatformSecureLib|OvmfPkg/Bhyve/Library/PlatformSecureLib/PlatformSecureLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 9178ffeb71cb..fc1c8014eba1 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -189,6 +189,7 @@
>  !else
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>  !endif
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index a665f78f0dc7..c76765723e2e 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -194,6 +194,8 @@
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>  !endif
>  
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> +
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
>    AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf

This hunk, unlike the other hunks, inserts an unwarranted-for empty line
between the openssl lib resolution(s) and the new lib class resolution.
This makes it more difficult to diff the DSC files against each other
(which is something I do frequently).

The discrepancy is also seen in the diffstat (2 lines for
"OvmfPkgIa32X64.dsc", 1 line for every other DSC file).

With the spurios newline removed; for this patch:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 17f345acf4ee..6992cfd98b70 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -193,6 +193,7 @@
>  !else
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>  !endif
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 782803cb2787..416f81f06a04 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -179,6 +179,7 @@
>  !else
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>  !endif
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>  
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
>    VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
>