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

Laszlo Ersek

Hi Ard!

On 08/11/20 10:22, Ard Biesheuvel wrote:
On 8/11/20 4:21 AM, matthewfcarlson@... wrote:
From: Matthew Carlson <macarl@...>
How am I supposed to review this change? The commit log is empty and I
was not cc'ed on the cover letter.
Cover letter:

[edk2-devel] [PATCH v4 0/5] Use RngLib instead of TimerLib for OpensslLib


Unfortunately, the cover letter doesn't much explain the approach
either. The latest comments in the BZ should be helpful though.

My understanding is that the timer-based "pseudo-random" generation is
factored out of "CryptoPkg/Library/OpensslLib/rand_pool_noise*" to the
new BaseRngLibTimerLib instance (see patches #1 and #5). In the middle,
platforms native to the edk2 tree and currently using "rand_pool_noise*"
are diverted to the new lib instance. (Patches #3 and #4.)

So I think the intent is to introduce no change in behavior for those
platforms, only make OpensslLib depend on the RngLib class.

Patch#2 adds BaseRngLibDxe, which depends on gEfiRngProtocolGuid.

I think the structure of the series is correct.


In edk2, we have two RNG protocol implementations,
"OvmfPkg/VirtioRngDxe" and "SecurityPkg/RandomNumberGenerator/RngDxe".
While it would be nice to use the "BaseRngLibDxe" instance in OvmfPkg
and ArmVirtPkg, *in the longer term*, I have some doubts:

- I don't know whether or how "SecurityPkg/RandomNumberGenerator/RngDxe"
applies to virtual machines.

- OvmfPkg/VirtioRngDxe does not produce gEfiRngProtocolGuid if there is
no virtio-rng-(pci|device) device configured in QEMU. So a strict depex
would not work; we'd again need some kind of OR depex.

- The ArmVirtQemu and OVMF PlatformBootManagerLib instances connect
virtio-rng-(pci|device) devices after signaling EndOfDxe. That's good
enough for boot loaders and the Linux kernel's UEFI stub, but possibly
not good enough for platform DXE drivers that need randomness before

- The "BaseRngLibDxe" instance from patch#2 only accepts one of the
"Sp80090Ctr256", "Sp80090Hmac256", and "Sp80090Hash256" algorithms, and
"OvmfPkg/VirtioRngDxe" provides none of those.
("SecurityPkg/RandomNumberGenerator/RngDxe" seems to provide

But, anyway, these are just longer-term points for OvmfPkg and
ArmVirtPkg; they aren't a problem with this patch set.

In general, please try to muster up the energy to write at least one
sentence that describes *why* the patch is needed, complementing the
subject line, which in this case summarizes correctly *what* the patch

And, in addition to the minimally one-sentence commit message body, each
commit message should reference

I'd be very happy if you could review this patch series; personally I
can only formally review patches #3 and #4.


Join to automatically receive all group messages.