Re: [edk2-devel] [PATCH v4 1/1] MdePkg/BaseRngLib: Add a smoketest for RDRAND and check CPUID

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Wed, Apr 26, 2023 at 10:54 PM Benjamin Doron
<benjamin.doron00@gmail.com> wrote:
>
> Hi,
> Is there merit to removing the assert statement? When this instance of the RngLib class is used, the platform builder says there's RNG support. Asserts are a little easier to see than debug prints, especially when they stall the platform. I think that leaving it in is better. If asserts don't call CpuDeadLoop() or are removed from the build entirely, then `ArchIsRngSupported()` will ensure safe behaviour (but incorrect functionality).

Certain platforms may not know if the CPU has rdrand or if it's broken
at all. For instance, OVMF (1st case) or firmware for the AMD AM4
mobos (where they had zen1, 2 and 3 with some RDRAND breakage in
between).
Also, as you mentioned, ASSERTs can be entirely removed from the
build. So I don't think asserts are a good idea here. Unless you want
to suggest hiding the ASSERT with a PCD, in which case, yuck?

> Also, I'm slightly concerned that the check for minimum RDRAND changes will succeed if it returns 0s, then 1s, then 0s... Though this seems like an RDRAND broken too conveniently, not a true errata.

Ack, I don't think the algorithm is perfect, but it's what Jason wrote
for Linux and suggested; I guess it may be adjusted if we so need, but
at the moment it definitely detects the egregious AMD breakage.

-- 
Pedro