Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use core::arch::x86_64::has_cpuid #3271

Closed
JonathanWoollett-Light opened this issue Nov 17, 2022 · 5 comments
Closed

Use core::arch::x86_64::has_cpuid #3271

JonathanWoollett-Light opened this issue Nov 17, 2022 · 5 comments
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Blocked Indicates that an issue or pull request cannot currently be worked on Type: Enhancement Indicates new feature requests

Comments

@JonathanWoollett-Light
Copy link
Contributor

JonathanWoollett-Light commented Nov 17, 2022

It would be preferable to use the more general core::arch::x86_64::has_cpuid to check for CPUID support.

Blocked on: rust-lang/rust#48556

@JonathanWoollett-Light JonathanWoollett-Light added Quality: Improvement Status: Blocked Indicates that an issue or pull request cannot currently be worked on labels Nov 17, 2022
@roypat
Copy link
Contributor

roypat commented Nov 17, 2022

Mh, the rust-lang issue you linked is closed, and it seems that there is some bug marking the cpuid function as nightly even though it should be stable rust-lang/rust#98253

@JonathanWoollett-Light JonathanWoollett-Light added Type: Enhancement Indicates new feature requests Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` and removed Quality: Improvement labels Mar 24, 2023
@roypat
Copy link
Contributor

roypat commented Jun 19, 2024

The issue tracking this on rust's side has yet again changed: rust-lang/rust#60123

@roypat
Copy link
Contributor

roypat commented Jun 19, 2024

That being said, Wikipedia has this to say about the cpuid instruction:

It was introduced by Intel in 1993 with the launch of the Pentium and SL-enhanced 486 processors.[1]

And it also seems that either x86_64 CPUs without CPUID support don't exist, or even the linux kernel wouldn't work on them (https://www.geoffchappell.com/studies/windows/km/cpu/cpuid/index.htm), so why do we need to ever do such a check in the first place? 🤔

@workingjubilee
Copy link

You probably do not, and if you ever did want to actually know about the CPU's cpuid support, you would want to also ask questions about e.g. whether the cpuid_fault feature exists, or else it would be a dangerously incomplete answer. This is why has_cpuid is a bad function: it provides a useless answer to a useless question.

@roypat
Copy link
Contributor

roypat commented Jun 20, 2024

Hi @workingjubilee,
thanks for your input! I agree that this is not a question that makes sense for us to ask. We have a clearly defined set of supported architectures, and all of those have the cpuid instruction. But even beyond that, Firecracker, being a virtual machine manager, needs VMX on Intel, which is even newer than CPUID. So lots of things would go wrong before we even hit a CPUID instruction in the first place (not that I can even find a single call to __cpuid in our code anyway. I am assuming KVM internally uses it). I also see now a comment on rust-lang/rust#60123 from a few days ago that Rust doesn't have targets without CPUID anymore. Yet another reason why Firecracker cannot work on such platforms.

So I am going ahead with closing this issue :)

@roypat roypat closed this as completed Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Blocked Indicates that an issue or pull request cannot currently be worked on Type: Enhancement Indicates new feature requests
Projects
None yet
Development

No branches or pull requests

3 participants