On Wed, Apr 26, 2023 at 10:54 PM Benjamin Doron <benjamin.doro...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103719): https://edk2.groups.io/g/devel/message/103719 Mute This Topic: https://groups.io/mt/95207204/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
