rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.11k stars 12.55k forks source link

Tracking issue for core::arch::{x86, x86_64}::has_cpuid #60123

Closed gnzlbg closed 2 months ago

gnzlbg commented 5 years ago

This issue tracks the stabilization of the has_cpuid intrinsic.

The "mini-RFC" is part of the stabilization PR: https://github.com/rust-lang-nursery/stdsimd/pull/730

cc @rust-lang/libs @alexcrichton

alexcrichton commented 5 years ago

@rfcbot fcp merge

There's a longer description of what's being stablilized on the PR, so looking for an official sign-off now!

rfcbot commented 5 years ago

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

briansmith commented 5 years ago

In what circumstance would somebody call has_cpuid()? Does Rust support any x86 or amd64 targets that don't have CPUID? I've never even considered the possibility that CPUID would be unavailable.

The original RFC mentioned me as somebody who might be interested in this, but really what I'm interested in is the actual CPUID and related intrinsics.

nagisa commented 5 years ago

Yes. SGX target is notable modern target which does not support CPUID.

briansmith commented 5 years ago

Yes. SGX target is notable modern target which does not support CPUID.

Right. However, in practice we need to know "Is this SGX?" not just vaguely "Is CPUID available?" In particular, if the target is SGX then we'll assume certain features are available that we wouldn't normally assume are available by default otherwise.

FWIW, I have code that uses CPUID extensively on all non-SGX x86 and amd64 targets, but I still don't have any use for this function, AFAICT, because it doesn't provide information useful to make a decision. (I mention this because I was mentioned in the other RFC that motivated this.)

petrochenkov commented 5 years ago

Yes. SGX target is notable modern target which does not support CPUID.

Heh, I wonder what result the EFLAGS.ID-based CPUID detection procedure returns inside an enclave. I suspect it's still going to be true.

On a similar note, if you are calling CPUID inside a VMX-based hypervisor, it will be rerouted to the hypervisor and the hypervisor can do whatever it wants in theory, including eating your laundry :)

dtolnay commented 5 years ago

Registering Brian's point above in https://github.com/rust-lang/rust/issues/60123#issuecomment-485037823. Do we know of someone with a use case that benefits from this function? How did it come to be added originally?

FWIW, I have code that uses CPUID extensively on all non-SGX x86 and amd64 targets, but I still don't have any use for this function, AFAICT, because it doesn't provide information useful to make a decision.

@rfcbot concern not useful

Centril commented 5 years ago

@alexcrichton Please cancel the FCP and include T-Lang. (I've noted this on many occasions -- exposing intrinsics adds new fundamental abilities that cannot be done in Rust code otherwise and all such decisions needs T-Lang approval)

gnzlbg commented 5 years ago

@briansmith

In what circumstance would somebody call has_cpuid()?

When you want to know whether the hardware you are running on has the cpuid instruction available, so that you can know whether using cpuid is safe or not.

Does Rust support any x86 or amd64 targets that don't have CPUID?

Many. CPUID was introduced with the Pentium and i486-SL architectures, and Rust does support both i486 and i386 targets - we even ship some i386 targets with rustup like i386-apple-ios although more are available via cargo xbuild (e.g. i386 linux targets).

I've never even considered the possibility that CPUID would be unavailable.

Lucky you I guess. As the mini-RFC mentions, all mainstream compilers do CPUID detection in their CPUID intrinsics. i386 and i486 are modern enough that all modern toolchains can support it without many issues, but old enough to predate the introduction of the CPUID instruction.


@nagisa

SGX target is notable modern target which does not support CPUID.

On SGX doing x86_64 hardware feature detection via the CPUID instruction is not something you can do. Still, has_cpuid returning false would prevent any code in std from trying to do that.

Heh, I wonder what result the EFLAGS.ID-based CPUID detection procedure returns inside an enclave. I suspect it's still going to be true.

Yeah, the current implementation is incorrect for SGX targets. That target was added relatively recently, and none of the people working on SGX has run into this (cc @jethrogb), (EDIT: that is incorrect, we do support SGX targets, see next comment) I suspect because as @briansmith mentions std::detect is not a very useful module inside an SGX enclave. Knowing whether CPUID available is not enough. What you want to know is whether you are actually in an SGX enclave or not.

gnzlbg commented 5 years ago

That target was added relatively recently, and none of the people working on SGX has run into this (cc @jethrogb), I suspect because as @briansmith mentions std::detect is not a very useful module inside an SGX enclave

Wait, this is wrong, the current implementation does properly support SGX and it does the obvious thing that was suggested above already:

https://github.com/rust-lang-nursery/stdsimd/blob/master/crates/core_arch/src/x86/cpuid.rs#L87

jethrogb commented 5 years ago

The motivation for this intrinsic is still unclear to me. Specifically, for SGX, feature detection might work when using is_x86_feature_detected even though has_cpuid returns false. Everyone should be using std::is_x86_feature_detected as much as possible such that feature detection need only be implemented once.

gnzlbg commented 5 years ago

Specifically, for SGX, feature detection might work when using is_x86_feature_detected even though has_cpuid returns false.

I've commented on that issue, since I think the claim is not 100% precise, but puting us back on topic: implementing run-time feature detection for SGX is orthogonal from being able to tell whether a CPU supports the CPUID instruction or not.

For SGX, has_cpuid returns false, and that is correct AFAICT (asm!("cpuid" ) fails there).

Everyone should be using std::is_x86_feature_detected as much as possible such that feature detection need only be implemented once.

If your application is #![no_std] you can't do that - we'd need to force everyone to use std to make that true. For most targets (x86 is one of them), run-time feature detection requires OS support. If you don't have an OS, there are many ways of implementing run-time feature detection that are incompatible with each other depending on what your application is doing (your application becomes the OS), so we can't provide something that works for everybody in #![no_std], which is why people can build std::detect from crates.io, and configure it for their use case if they can't use std.

briansmith commented 5 years ago

Rust does support both i486 and i386 targets - we even ship some i386 targets with rustup like i386-apple-ios although more are available via cargo xbuild (e.g. i386 linux targets).

OK, then what we really need to know is why CPUID isn't available: because the target is too old, or because SGX, because the toolchain has been configured to recommend/require only static compile-time feature detection, or for some other reason.

gnzlbg commented 5 years ago

If you need to know any of that (where one of the things you mention can’t happen, and two are the same, so there are only two cases) has_cpuid is one of the building blocks you can use to query that. But the std cpuid APIs are unsafe, and std only needs to know whether they are safe to call.

On Sun 21. Apr 2019 at 04:01, Brian Smith notifications@github.com wrote:

Rust does support both i486 and i386 targets - we even ship some i386 targets with rustup like i386-apple-ios although more are available via cargo xbuild (e.g. i386 linux targets).

OK, then what we really need to know is why CPUID isn't available: because the target is too old, or because SGX, because the toolchain has been configured to recommend/require only static compile-time feature detection, or for some other reason.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/60123#issuecomment-485217534, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG43JW77ML7OKRDUDTDFNDPRPDI5ANCNFSM4HHHEDMQ .

briansmith commented 5 years ago

If you need to know any of that (where one of the things you mention can’t happen,

What is the thing that can't, and will never, happen?

and two are the same, so there are only two cases) has_cpuid is one of the building blocks you can use to query that.

I can get all the same information without has_cpuid, except for the thing that hasn't been implemented yet (target explicitly configured to disable dynamic feature probing).

But the std cpuid APIs are unsafe, and std only needs to know whether they are safe to call.

I don't know what this means. If you're saying that the current implementation of libstd needs this for some reason, then libstd can already use it without it being stable.

gnzlbg commented 5 years ago

because the toolchain has been configured to recommend/require only static compile-time feature detection

If you don't want to do run-time feature detection, you can just not use is_x86_feature_detected!. If you only want to do compile-time feature detection, cfg(target_feature) has always done that.

because the target is too old,

You can run i386 binaries on skylake, which does have CPUID. What does knowing that the target is i386 and does not have CPUID tell you about the hardware the binary actually runs on ?

I can get all the same information without has_cpuid,

How can you tell whether the hardware an i386 or i486 binary runs on supports the CPUID instruction without has_cpuid on stable Rust? Can you show an example ?

briansmith commented 5 years ago

OK, I didn't realize that this was doing runtime detection (using EFLAGS bit 21 to detect whether CPUID is available) vs. determining it statically.

A few things I noticed:

  1. One of the references cited in the implementation for this suggested to avoid doing this detection in C because compilers often stomp on EFLAGS when doing inline assembly, yet the implementation is in inline assembly. If it were up to me, I would move it out of Rust and into a standalone assembly module to be safe.

  2. The implementation of the runtime detection this feature does is conditional on #[cfg(not(target_feature = "sse"))], but that seems wrong for i586 targets. i586 targets shouldn't (AFAICT) should be presumed to support CPUID since it was available on (AFAICT) all Pentiums. Changing the implementation to avoid the EFLAGS-based conditional logic for 586 and later seems like a good idea to me.

  3. There are no i386 or i486 targets at https://forge.rust-lang.org/platform-support.html because i386-apple-ios. I think every machine running i386-apple-ios can actually execute CPUID just fine. So, I'm not sure any (officially) supported target actually benefits from this feature.

The runtime detection is conditional on #[cfg(not(target_feature = "sse"))] but SSE is the one of the minimum features my code needs, and every SSE-having CPU has CPUID, I think. This is one reason I don't need this function, the other being that I already need to do different things for SGX specifically. I hope this clarifies my statement about why it isn't useful for me. However, it may be useful to people targeting 386 and 486. (I doubt the reliability of LLVM-generated code running on a 386 or 486 though, due to general lack of attention.)

gnzlbg commented 5 years ago

One of the references cited in the implementation for this suggested to avoid doing this detection in C because compilers often stomp on EFLAGS when doing inline assembly, yet the implementation is in inline assembly. If it were up to me, I would move it out of Rust and into a standalone assembly module to be safe.

Iff this is the case with the current implementation, please fill a bug in rust-lang/rust about it since that would mean that the implementation of asm! is broken.

The implementation of the runtime detection this feature does is conditional on #[cfg(not(target_feature = "sse"))], but that seems wrong for i586 targets.

Rust i686 targets do have SSE, what they don't have is SSE2. Doing better on i586 is possible (https://github.com/rust-lang-nursery/stdsimd/issues/497), PRs welcome.

dtolnay commented 5 years ago

The runtime detection is conditional on #[cfg(not(target_feature = "sse"))] but SSE is the one of the minimum features my code needs, and every SSE-having CPU has CPUID, I think. This is one reason I don't need this function, the other being that I already need to do different things for SGX specifically. I hope this clarifies my statement about why it isn't useful for me. However, it may be useful to people targeting 386 and 486.

Can someone confirm that this is the entire intended target audience for this function, or are there other reasons someone might want to call it outside of 386 and 486?

It's strange to me that the motivation in https://github.com/rust-lang-nursery/stdsimd/pull/730 refers to @briansmith but Brian won't need this function. I am still trying to work out whether anyone else would ever want to call this.

How does this typically work in C++? The PR mentions that neither clang nor gcc expose this functionality. What do people do instead?

If there is an implementation of has_cpuid available through crates.io, do we know of anyone calling it?

dtolnay commented 5 years ago

@Centril:

Please cancel the FCP and include T-Lang. (I've noted this on many occasions -- exposing intrinsics adds new fundamental abilities that cannot be done in Rust code otherwise and all such decisions needs T-Lang approval)

Could you take a look at https://github.com/rust-lang-nursery/stdsimd/pull/730 and let me know whether you still would like T-lang involved? This isn't an intrinsic in the sense of extern "rust-intrinsic" -- this is a library function implemented using target_env / target_arch / target_feature / asm.

Centril commented 5 years ago

This isn't an intrinsic in the sense of extern "rust-intrinsic" -- this is a library function implemented using target_env / target_arch / target_feature / asm.

As far as I can tell, the function is implemented using asm!(...), an unstable language feature. As far as I know, it cannot be implemented using stable Rust (linking to assembly files doesn't count for the standard library cause then you could add all sorts of behaviors to the operational semantics...). As such, it requires language team sign-off. (Not that I would be opposed to adding this or anything)

dtolnay commented 5 years ago

@rfcbot fcp cancel

rfcbot commented 5 years ago

@dtolnay proposal cancelled.

dtolnay commented 5 years ago

core::arch::{x86, x86_64}::has_cpuid with the following signature:

fn has_cpuid() -> bool;

Please refer to the stabilization PR https://github.com/rust-lang-nursery/stdsimd/pull/730#issue-271958476 for the motivation, reference, implementation details, prior art, alternatives.

(Relaunching proposed-fcp with both T-libs + T-lang as this library feature exposes something that cannot be implemented today using stable Rust.)

@rfcbot fcp merge

rfcbot commented 5 years ago

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

dtolnay commented 5 years ago

Registering the earlier concern from https://github.com/rust-lang/rust/issues/60123#issuecomment-485052736 and https://github.com/rust-lang/rust/issues/60123#issuecomment-490598320.

@rfcbot concern not useful

gnzlbg commented 5 years ago

It's strange to me that the motivation in rust-lang-nursery/stdsimd#730 refers to @briansmith but Brian won't need this function.

IIRC @briansmith wanted to use (or was exploring using?) a re-configured std::detect via crates.io that dropped support for older libc's and this API is IIRC the only thing missing that prevented this from working on stable Rust. There are some other alternatives, like bumping the libc version of the rust targets (although I don't think this happened?), re-implementing something like std::detect without the parts that require nightly Rust (e.g. assuming certain target characteristics), or just calling C and doing run-time feature detection there using inline assembly, etc. I don't know if they pursued any of those instead.

Using std_detect from crates.io is what #![no_std] users have to do anyways, since there is no core::detect module in libcore.

How does this typically work in C++?

In standard C++ this does not work. In C++ that works on all major compilers, people just use the compiler functionality to query cpuid features, and that functionality, like ours, does the has_cpuid check for you. C++ programs for which this functionality is inappropriate just use inline assembly. That's not part of standard C++, but all compilers provide inline assembly as a language extension with strong backward compatibility guarantees, so if you control your compilers you can rely on that working in practice.

joshtriplett commented 5 years ago

Can we clarify the use case for this, other than 386 and 486 processors?

gnzlbg commented 5 years ago

This API returns true if the CPU the binary runs on supports executing the CPUID instruction. Otherwise, this returns false, and that can currently only happen on 486 and earlier since those are AFAIK the only x86 CPUs that do not support this instruction.

nagisa commented 5 years ago

x86 in an enclave do not “support” cpuid either.

gnzlbg commented 5 years ago

Yes, in SGX enclaves this API also returns false.

nikomatsakis commented 4 years ago

Based on the fact that nobody has spoken up in favor of this feature, I would like to propose we remove it -- I'm actually indifferent, it seems harmless enough, but I'd prefer for it not to stay in limbo.

(Also, I personally don't know that lang team needs to be tagged on this, but it doesn't seem very important.)

gnzlbg commented 4 years ago

I'd stop releasing std_detect on crates.io then. Those who want to customize its behavior, e.g., to avoid File I/O (IIRC @briansmith was the one that wanted it) can just recompile libstd instead or maybe change libstd defaults. If that ends up being a problem for somebody, we can revisit this with new user data.

npmccallum commented 4 years ago

I think the larger question that hasn't been answered by this is what to consider safe.

Currently, the Intel Developer Manual states that one must do what has_cpuid() does in order to invoke CPUID. This is why __cpuid() and its siblings are marked as unsafe because it is expected that you have to first test for CPU support before calling them. Otherwise, you will trigger a CPU exception.

AFAICS, everyone currently issuing CPUID without has_cpuid() or equivalent is probably lying about safety. So, how does one get safe code that calls __cpuid()?

If has_cpuid() returns true, it is safe to issue CPUID. This is true on all platforms. This means that you can write code like this:

pub fn max_foo() -> Option<u32> {
    if core::arch::x86_64::has_cpuid() {
        Some(unsafe { core::arch::x86_64::__cpuid(FOO).ebx })
    }

    None
}

Notice that the above code is always safe on every platform. And can be marked as such. Now, there are platforms where it may be possible to issue CPUID even though has_cpuid() returns false (i.e. SGX and virtualized platforms can both trap the exception and provide synthetic results). But such is never a safe operation and shouldn't be marked as such (don't lie about safety!).

The only other option that I see for any sanity is to mark __cpuid() and its peers as safe. What is implied by this is that Rust only commits to supporting platforms where CPUID works. This would mean that the current Fortanix SGX target would either need to grow exception handling and return synthetic CPUID values or we would have to remove it.

But as things currently stand, it is impossible to write safe CPUID code without lying. And that is a problem.

npmccallum commented 4 years ago

@gnzlbg Would you consider reopening this issue until we have a way to write safe CPUID code?

briansmith commented 2 years ago

It seems to me that the best way to do this is to add a "cpuid" target feature to the language so that people can treat CPUID just like any other conditionally-available CPU instruction with target_feature / cfg_target_feature / cfg_feature_enabled. This would allow people to detect at compile time or at runtime whether they can execute CPUID instructions. Ideally the definitions of each target would be updated appropriately.

Maybe we don't even know which targets guarantee that it is OK to execute CPUID? I would say that any SSE-capable Linux/unix/Windows target guarantees it is safe to execute CPUID, but SGX and some other weird targets would not guarantee that.

I'm not sure what the process is for adding new target features. If somebody can help me, and people generally agree, I would help move this forward.

Amanieu commented 1 year ago

I'm reopening this since the code for has_cpuid is still in stdarch and doesn't have a tracking issue. This issue should be closed only of has_cpuid is removed or it is stabilized.

briansmith commented 11 months ago

Here is my proposal for this:

briansmith commented 10 months ago

The function is documented as only "Does the host support the cpuid instruction?" However, I doubt that this is particularly useful for most people as even if the host allows executing CPUID, you can't necessarily use CPU features that are reported by CPUID as being supported. For example, I think we're allowed to execute CPUID on x86_64-unknown-none but that doesn't help us in determining whether we can safely use vector registers or other CPU features.

workingjubilee commented 10 months ago

That is because CPUID doesn't actually determine what CPU state is actually enabled, as you note. However, CPUID is a gateway to determine whether or not you can use the instruction that can answer that question: XGETBV. The returned answer from XGETBV is the correct answer to use.

I do not know of a situation where XGETBV's CPUID bit is enabled but the instruction is not available? It is an unprivileged instruction and shouldn't require conditionally enabling anything.

briansmith commented 10 months ago

I do not know of a situation where XGETBV's CPUID bit is enabled but the instruction is not available? It is an unprivileged instruction and shouldn't require conditionally enabling anything.

Within an operating system kernel (e.g. Linux) you cannot use vector registers except between calls to kernel_fpu_begin and kernel_fpu_end.

workingjubilee commented 10 months ago

You can do whatever you like, actually, if you don't care about clobbering userspace registers. Those two functions map roughly to XSAVEOPT and XRSTOR (and other schemes that amount to the same). They save and preserve register state so the userspace registers aren't clobbered. And in order to use the XSAVE family, you have to have enabled saving that register state.

While I have many concerns about the way our target_feature code works today, especially in relation to kernels, it is simply not the job of the Rust standard library to be the sole enforcer of the Linux kernel's coding practices for it. Notably, if someone wants to implement a kernel that doesn't preserve extended register states (and thus doesn't support them in userspace), I don't see why we should stop them.

briansmith commented 10 months ago

@workingjubilee My point is that the documentation for this function is too vague and misleading. It is my fault for being pedantic and then not being precise in my description of the problem. The problem is that users of libcore want a function that returns true if __cpuid is safe to call (see the discussion above). What does "safe" mean? That's what we need to clarify, and that clarification should happen in the documentation and/or in the API design:

Here is a program that, if run on a CPU that supports the cpuid_fault feature (Intel Tiger Lake CPU or later, but not any AMD, AFAICT) on Linux 4.12 or later, will segfault in __cpuid(), I think (Unfortunately, my Linux machines don't support cpuid_fault so I can't test this in a "working" configuration):

#![feature(stdsimd)]

#[cfg(target_arch = "i686")]
use std::arch::x86 as arch;
#[cfg(target_arch = "x86_64")]
use std::arch::x86_64 as arch;

fn main() {
    const ARCH_SET_CPUID: libc::c_long = 0x1012;
    const ZERO: libc::c_long = 0;

    if unsafe { libc::syscall(libc::SYS_arch_prctl, ARCH_SET_CPUID, ZERO) } != 0 {
        println!("Error: {:?}, ", std::io::Error::last_os_error());
        std::process::exit(1);
    }

    let has_cpuid = arch::has_cpuid();
    println!("has_cpuid() returned {}", has_cpuid);

    if has_cpuid {
        let _ = unsafe { arch::__cpuid(0) };
    }

    println!("Done.");
}

Note that it is expected that ARCH_SET_CPUID would only be used by software that installs a segfault handler that emulates CPUID, such as the rr debugger or a VM(-ish) system.

workingjubilee commented 10 months ago

Oh that's wild, I didn't even know about CPUID_FAULT. Yes, that certainly, at minimum, complicates things.

RalfJung commented 10 months ago

My assumption was that the only way we support to check whether some target feature is available, is is_$TARGET_feature_detected!. That macro we can patch to take into account target- and Rust-specific concerns, giving us the liberty to define ourselves when we consider a target feature "available" without being tied to any existing definition. (The existing target feature situation e.g. in LLVM is such a mess that being able to control this ourselves seems quite important to me.)

If programs can use cpuid, do we need to warn that even if the CPU says AVX is available, for Rust purposes we could still consider this feature unavailable? Or was the plan that we officially standardize that certain target features correspond to what can be queried with cpuid?

tarcieri commented 10 months ago

@RalfJung the biggest problem with is_$TARGET_feature_detected! is that it's in std and we would like to support CPU feature detection on no_std targets as well

RalfJung commented 10 months ago

Where can I read more about the plan here -- how would one write is_x86_feature_detected!("avx") in no_std with this proposal? What would it take to make the existing macros available in core, why is that not viable? What are the trade-offs around letting people infer target features from raw CPUID information -- a new stable language-level guarantee? (As such I think this also needs t-lang involvement.)

workingjubilee commented 3 months ago

@RalfJung

Where can I read more about the plan here -- how would one write is_x86_feature_detected!("avx") in no_std with this proposal?

As briansmith has noted, you cannot do so without additional asm! or cfg mechanisms.

What would it take to make the existing macros available in core, why is that not viable?

It would be fine if we're willing to just do so. As far as I understand, I think we've avoided doing so out of a reluctance to include what is target-specific code in core.

What are the trade-offs around letting people infer target features from raw CPUID information -- a new stable language-level guarantee? (As such I think this also needs t-lang involvement.)

The CPUID information, when accessed from userspace, constitutes a kind of special calling convention with the kernel, as the kernel is allowed to pick the answers. It is not exactly "raw", in that sense, unless accessed from the privileged context, where one is allowed to see the true answers (and change them).

workingjubilee commented 3 months ago

T-lang has already made some decisions re: target-features that implicitly relate to CPUID: https://github.com/rust-lang/rust/issues/73631#issuecomment-654431592

workingjubilee commented 3 months ago

cc @rust-lang/libs-api

I am requesting that libs-api advance a new FCP to remove has_cpuid and close this issue. Note that I am asking only for a judgement regarding has_cpuid(): I believe this should not reflect negatively or positively on @briansmith's alternative proposal, which I believe is a new (and possibly better) feature. Rather, asm! is now stable, and as @briansmith notes, this intrinsic essentially does not provide an answer to the "is it 'safe' to call CPUID and rely on its results?" question.

...Note that key detail: it is never possible to simply invoke CPUID with impunity, due to the existence of cpuid_fault, so we should not stabilize this function with this signature if we expect the following code to be written:

if has_cpuid() {
    // SAFETY: see previous line
    let cpuid_result = unsafe { __cpuid(some_leaf) };
    // the rest of the code may never be reached because of a fault,
    // and our fate is decided by the platform's fault handlers
}

I don't think a function that returns a bool for which true is "sure, you can call that unsafe fn safely, but it may abort, or do whatever the fault handler picks, or return, whatever lol" is something we should really bother with. The CPUID instruction invokes something we should regard as arbitrary magic that essentially says whatever the architecture and OS (or hypervisor!) wants it to say, and has arbitrary effects on control flow based on the microarchitecture.

One can make the argument that because these behaviors are relatively well-specified (for the CPUs that have CPUID, anyways), that this is "sound" nonetheless, because amb(x, y, z) is a safe function in Rust, so it is allowed to have a function that is, basically

if cpu_chosen() {
   jump_to(cpu_specific_code)
} else if os_chosen() {
   jump_to(os_specific_code)
} else {
   let eax, ebx, ecx, edx;
   asm!(..);
   CpuidResult { eax, ebx, ecx, edx }
}

But if we are arguing that, then we should just have the courage to make __cpuid() a safe function on x86-64 CPUs, and make this function reachable only on 32-bit x86. Except even that wouldn't apply, due to SGX existing. In truth, the problem is this: Intel is quite clearly willing to alter the deal. We can only pray they do not change it further!

Let us render unto asm! what is asm!'s, and unto cfg what is cfg's.