Use `is_x86_feature_detected` instead of CPUID

[jethrogb]

Rust provides a macro is_x86_feature_detected to figure out what features are supported. It knows about platform-specific ways to detect features and caches the results for future invocations. This should be used instead of calling cpuid directly.

[nagisa]

I remember not using it for some reason, but can’t remember what it was, exactly. That macro’s std-only, isn’t it?

[jethrogb]

Indeed it is. And this crate wants to support no_std of course. I'd still suggest using that macro if std is available (following the default-enabled std feature convention).

[nagisa]

Well, caching hasn’t much value for this crate as the CPUID instruction is only called when constructing an instance of RdRand/RdSeed.

Adding std feature and extra code just for the sake of using is_x86_feature_detected seems excessive. I don’t think it is worth it unless some other feature that would necessitate addition of std feature appears.

[jethrogb]

Some platforms such as SGX don't support CPUID. It would be good to standardize on a single way to get capability information (preferably using is_x86_feature_detected) instead of each crate rolling its own.

[nagisa]

Well, I don’t see why is_x86_feature_detected could not be in core, but as long as it isn’t, it won’t be standardized on.

Perhaps make an issue about that on rust-lang/rust?

[jethrogb]

It's not in core because some platforms need OS support to provide this information, such as reading /proc/cpuinfo. This mostly applies to non-x86 targets, but as mentioned SGX also can't support it in core.

[nagisa]

Okay, so after some thinking is_x86_feature_detected cannot exactly be in libcore because of things like AVX, where not only hardware, but also OS support is necessary (and thus /proc/cpuinfo must be read/parsed).

For rdrand it is not a problem, as no OS support is necessary and thus cpuid is a sole indicator necessary for checking whether the feature is supported. In fact, parsing /proc/cpuinfo will fail to identify rdrand support on older kernels which do not look for the bit in question. (edit: not an actual problem, because is_x86_feature_detected knows where it needs to look)

With regards to the instruction specifically, no matter how much I’m thinking about it, -sgx is simply not x86_64. At best it could be considered i386, since cpuid is a required instruction in i486 onward and it does not have any documented failure conditions. There seem to be instructions (e.g. RDTSC) other than cpuid that are not supported in -sgx as well.

It seems that Intel people install a signal handler and emulate such instructions instead. This to me, makes much more sense than trying to eradicate use of these fairly standard instructions from any Rust code base that exists out there.

All that being said, I will gladly accept PRs that add support for this weird target, provided they do not make code unmaintainable and, hopefully, are tested somehow.

/me grumbles about people misusing target triples.

[nagisa]

Solution wise, to me it seems much more sensible to just add

#[cfg(target_abi="sgx")] sgx_cpuid(...) where we call libcore’s cpuid, than trying to make this crate dependent on std in a nice way.

[jethrogb]

Well, caching hasn’t much value for this crate as the CPUID instruction is only called when constructing an instance of RdRand/RdSeed.

Sure it does, "constructing" and RNG with RDRAND is cheap. CPUID might not be (VM emulation, etc.).

as long as [is_x86_feature_detected isn't in core], it won’t be standardized on.

I don't understand your point here. There are plenty of things that are not in core but are in std that are accepted standards in the Rust community. Vec, the traits in std::io, etc. There is a big efficiency gain when using standard methods: crate authors have to do less work (if no_std support is not needed) and crate users can be assured that things play nicely together. For people who do want no_std support, adding hacky workarounds or half-implemented APIs is commonplace.

It seems that Intel people install a signal handler and emulate such instructions instead. This to me, makes much more sense than trying to eradicate use of these fairly standard instructions from any Rust code base that exists out there.

This is a hack, instructions can't be securely emulated with SGX. It's like asking someone over the phone whether you can drive or not. In particular, the x86_64-fortanix-unknown-sgx target (which recently gained Tier 3 support upstream) does not support such emulation.

We've filed PRs #6 and #7 for your consideration. I have an extremely strong preference for merging #6.

[nagisa]

This has been fixed by #6.

[nagisa]

@jethrogb @akash-fortanix v0.4.0 has been released.