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

mpk: reenable MPK support with vendor string check #7513

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

abrown
Copy link
Collaborator

@abrown abrown commented Nov 9, 2023

In #7446 I disabled MPK support temporarily due to failures in CI runs. Looking into this further in #7445, I discovered that it is due to how has_cpuid_bit_set works on different x86 machines: Intel's CPUID instruction reports support for MPK in a certain leaf bit, AMD does it some other (unknown?) way. The CI problem boiled down to occasional runs on AMD machines that would fail with SIGILL because the AMD machine reported that it had MPK support when it really did not. This change fixes the issue by first checking if the CPU vendor string is GenuineIntel before inspecting the MPK CPUID leaf bit.

Closes #7445.

In bytecodealliance#7446 I disabled MPK support temporarily due to failures in CI runs.
Looking into this further in bytecodealliance#7445, I discovered that it is due to how
`has_cpuid_bit_set` works on different x86 machines: Intel's `CPUID`
instruction reports support for MPK in a certain leaf bit, AMD does it
some other (unknown?) way. The CI problem boiled down to occasional runs
on AMD machines that would fail with `SIGILL` because the AMD machine
reported that it had MPK support when it really did not. This change
fixes the issue by first checking if the CPU vendor string is
`GenuineIntel` before inspecting the MPK `CPUID` leaf bit.

Closes bytecodealliance#7445.
@abrown abrown requested a review from a team as a code owner November 9, 2023 17:16
@abrown abrown requested review from fitzgen and removed request for a team November 9, 2023 17:16
crates/runtime/src/mpk/pkru.rs Outdated Show resolved Hide resolved
@abrown abrown enabled auto-merge November 9, 2023 18:27
@abrown abrown mentioned this pull request Nov 9, 2023
@abrown abrown added this pull request to the merge queue Nov 9, 2023
Merged via the queue into bytecodealliance:main with commit b3da3c3 Nov 9, 2023
18 checks passed
@abrown abrown deleted the pku-intel-check branch November 9, 2023 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mpk: some GitHub CI runners have MPK enabled but fail to run MPK code
2 participants