Hi Matt,
1) BaseRngLibTimerLib
a) The comments incorrectly list delays in ms instead of us.
b) Did you consider use of GetPerformanceCounterProperties()?
I also do not seen an explanation of the delay values used.
(why not smaller or larger values). At a minimum, the file
header should state it only works if the rate of the perf
counter from TimerLib is much greater than 1MHz.
2) BaseRngLibDxe
a) This is not a lib of type Base. I recommend the name
DxeRngLibRngProtocol.
b) Has a "MU_CHANGE" comment that can be removed
c) GenerateRandomNumberViaNist800Algorithm() assigns values in
declaration. Init should be moved into statements.
d) How would gBS aver be NULL? The INF lists the BootServicesTableLib
as a dependency, so the constructer is always run before the
services are used. I think these checks can be removed.
e) Minor code style issues. if statements should have { as end of line.
With the addition of DxeRngLibRngProtocol to MdePkg, I think the CryptoPkg
DSC can be updated to use this RngLib instance from the Crypto DXE mododule.
Best regards,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Matthew Carlson
> Sent: Friday, July 31, 2020 1:27 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v3 0/3] Use RngLib instead
> of TimerLib for OpensslLib
>
> From: Matthew Carlson <mac...@microsoft.com>
>
> Fixes Bugzilla#1871
> https://github.com/tianocore/edk2/pull/845
>
>
> Matthew Carlson (3):
> CryptoPkg: OpensslLib: Use RngLib to generate entropy
> in rand_pool
> MdePkg: TimerRngLib: Added RngLib that uses TimerLib
> MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
>
> CryptoPkg/Library/OpensslLib/rand_pool.c
> | 203 ++----------------
> CryptoPkg/Library/OpensslLib/rand_pool_noise.c
> | 29 ---
> CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c
> | 43 ----
> MdePkg/Library/BaseRngLibDxe/RngDxeLib.c
> | 216 ++++++++++++++++++++
> MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> | 154 ++++++++++++++
> CryptoPkg/CryptoPkg.dsc
> | 1 +
> CryptoPkg/Library/OpensslLib/OpensslLib.inf
> | 15 +-
> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> | 15 +-
> CryptoPkg/Library/OpensslLib/rand_pool_noise.h
> | 29 ---
> MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf
> | 38 ++++
>
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.in
> f | 38 ++++
>
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> i | 17 ++
> MdePkg/MdePkg.dsc
> | 5 +-
> 13 files changed, 489 insertions(+), 314 deletions(-)
> delete mode 100644
> CryptoPkg/Library/OpensslLib/rand_pool_noise.c
> delete mode 100644
> CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c
> create mode 100644
> MdePkg/Library/BaseRngLibDxe/RngDxeLib.c
> create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> delete mode 100644
> CryptoPkg/Library/OpensslLib/rand_pool_noise.h
> create mode 100644
> MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf
> create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.in
> f
> create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> i
>
> --
> 2.27.0.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63599): https://edk2.groups.io/g/devel/message/63599
Mute This Topic: https://groups.io/mt/75915445/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
