Re: [edk2-devel] [PATCH 2/3] MdePkg: Refactor BaseRngLib to support AARCH64 in addition to X86

["Rebecca Cran" <rebecca@nuviainc.com>]

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