rdrand implementation should enable the feature itself

[nagisa]

Current implementation of rdrand code fails to compile if the user does not enable the rdrand feature during compilation. Instead, the code should enable the feature for the relevant portions of the code itself with #[target_feature(enable="rdrand")].

[newpavlov]

I believe the current approach is correct (although indeed somewhat inconvenient). If you enable target feature for part of the code, you should make sure via runtime checks that it will not be called on platforms without used target feature(s) support.

We could add runtime detection and return error if RDRAND is not supported, but CPUID is not a cheap instruction and IIRC all currently existing SGX-capable platforms support RDRAND. How about proposing rdrand target feature (and maybe others?) to be enabled by default for SGX target?

[josephlr]

So right now the only supported rust target that would use getrandom's RDRAND implementation is x86_64-fortanix-unknown-sgx which does have the rdrand feature enabled.

However, if we want to have this implementation also run on platforms with target_os = "uefi" or target_os = "none", then we would need a check for RDRAND support (i.e. calling CPUID exactly once if the target doesn't have the rdrand feature).

[newpavlov]

if we want to have this implementation also run on platforms with target_os = "uefi" or target_os = "none", then we would need a check for RDRAND support

I am not familiar with uefi targets, but shouldn't we use EFI_RNG_ALGORITHM_RAW instead of RDRAND? Plus don't forget about #4, applications which target exotic platforms or want some special source of randomness can just overwrite getrandom.

Since right now only SGX uses RDRAND and rdrand target feature is already enabled by default, I think we should leave everything as-is and discuss how to handle future targets separately after their introduction.