On 5/4/21 3:09 PM, Sami Mujawar wrote:
+EFIAPI
+BaseRngLibConstructor (
+  VOID
+  )
+{
+  UINT64 Isar0;
+  //
+  // Determine RNDR support by examining bits 63:60 of the ISAR0 register 
returned by
+  // MSR. A non-zero value indicates that the processor supports the RNDR 
instruction.
+  //
+  Isar0 = ArmReadIdIsar0 ();
+  ASSERT ((Isar0 & RNDR_MASK) != 0);
+  (void)Isar0;
[SAMI] ASSERTs will vanish in the release builds. So, I think this needs to be an if condition. If RNDR is not supported RETURN_UNSUPPORTED should be returned. However, it appears thatthe auto generated function ProcessLibraryConstructorList() disregards the error code returned by the constructor (see Build\...\AutoGen.c files). So it looks like the loading operation would continue in release builds despite of an error. I am not aware if this is the desired behavior or why the status code returned by the constructor is disregarded.

However, this would be a probem in the current case as subsequent calls to generate random numbers will result in an undefined instruction exception.
To prevent this, I think the above check should be done in either
    - ArmRndr()/ArmRndrrs()
   or
   - preferably in ArchGetRandomNumberXX(), which should return an error code EFI_UNSUPPORTED, EFI_NOT_READY or EFI_SUCCESS. However, the impact on IA32/x64 code needs to be evaluated.

[/SAMI]

Yes, I noticed that but the same problem exists for the IA32/x64 code, since it might not support RDRAND/RDSEED. Should I change the BaseRngLib interface to return an EFI_STATUS instead of BOOLEAN? Or would that be too disruptive?

--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74772): https://edk2.groups.io/g/devel/message/74772
Mute This Topic: https://groups.io/mt/82440612/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to