crates: move host CPUID queries from cpuid-gen to cpuid-utils #843
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `cpuid-gen` utility contains logic that reads and pretty-prints bhyve's default CPUID values (or the host proecssor's raw CPUID outputs). Move the logic that reads these values into the `cpuid-utils` crate, re-express it in terms of `cpuid-utils`'s CPUID types, and make `cpuid-gen` call the new logic. Future changes will make propolis-server use this logic to set default CPUID values for VMs whose instance specs contain no CPUID settings. Fix a bit shifting bug in `collect_cpuid`'s handler for standard leaf D: this leaf outputs 64-bit values across two output registers, and the old logic wasn't shifting edx into the high 32 bits before OR'ing it with its counterpart register. Finally, simplify PHD's `cpuid_boot_test`: since it can now ask bhyve for CPUID settings that bhyve thinks are valid, it no longer needs to try to stitch together its own minimum viable CPUID profile. This test now passes with a Windows Server 2022 guest.
pfmooney
approved these changes
Jan 28, 2025
| } | ||
|
|
||
| /// Queries the supplied CPUID leaf on the caller's machine. | ||
| #[cfg(target_arch = "x86_64")] |
There was a problem hiding this comment.
nit: could use x86 here, rather than demanding x86-64, but it's not the first thing the cpuid area to be like that
There was a problem hiding this comment.
I gave this a try but discovered that rustc appears to have no 32-bit illumos target!
$ rustc --version
rustc 1.84.0 (9fc6b4312 2025-01-07)
$ rustc --print=target-list | grep illumos
aarch64-unknown-illumos
x86_64-unknown-illumos
I don't think we're actually querying host CPUID in any platform-agnostic tests that one might want to run against an i686 target, so I left this as-is. (I think supporting x86 would also require defining these functions twice, since the CPUID intrinsic is in core::arch::x86 when compiling against that target, unless there's some subtlety that I'm missing.)
iximeow
reviewed
Jan 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
cpuid-genutility contains logic that reads and pretty-prints bhyve's default CPUID values (or the host processor's raw CPUID outputs). Move the logic that reads these values into thecpuid-utilscrate, re-express it in terms ofcpuid-utils's CPUID types, and makecpuid-gencall the new logic. Future changes will make propolis-server use this logic to set default CPUID values for VMs whose instance specs contain no CPUID settings.Fix a bit shifting bug in
collect_cpuid's handler for standard leaf D: this leaf outputs 64-bit values across two output registers, and the old logic wasn't shifting edx into the high 32 bits before OR'ing it with its counterpart register.Finally, simplify PHD's
cpuid_boot_test: since it can now ask bhyve for CPUID settings that bhyve thinks are valid, it no longer needs to try to stitch together its own minimum viable CPUID profile. This test now passes with a Windows Server 2022 guest.Tests: cargo test; PHD with Debian 11 and WS2022 guests; manually compared text and TOML output from the old and new cpuid-gen binaries.
Fixes #793. Related to #835.