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

fix field order for INVPCID descriptor #508

Merged
merged 1 commit into from
Nov 10, 2024
Merged

fix field order for INVPCID descriptor #508

merged 1 commit into from
Nov 10, 2024

Conversation

Freax13
Copy link
Member

@Freax13 Freax13 commented Nov 3, 2024

Both Intel's and AMD's manuals describe the INVPCID descriptor as a 128-bit value with the linear address stored in the upper half and the PCID stored in the lower half. x86 uses little-endian byte ordering and so the lower half (pcid) should be stored before the upper half (address). Previously, our InvpcidDescriptor type stored the halves in the opposite order with the address before the pcid.
This patch fixes the order, so that the pcid is stored before the address. This new order is also the order used by Linux and OpenBSD:
https://github.com/torvalds/linux/blob/3e5e6c9900c3d71895e8bdeacfb579462e98eba1/arch/x86/include/asm/invpcid.h#L8
https://github.com/openbsd/src/blob/4e368faebf86e9a349446b5839c333bc17bd3f9a/sys/arch/amd64/include/cpufunc.h#L173
It's beyond me how this wasn't noticed earlier. The previous incorrect layout could lead to TLB entries not being flushed and #GP faults.

Both Intel's and AMD's manuals describe the INVPCID descriptor as a
128-bit value with the linear address stored in the upper half and the
PCID stored in the lower half. x86 uses little-endian byte ordering and
so the lower half (pcid) should be stored before the upper half
(address). Previously, our `InvpcidDescriptor` type stored the halves
in the opposite order with the address before the pcid. This patch
fixes the order, so that the pcid is stored before the address. This
new order is also the order used by Linux and OpenBSD:
https://github.com/torvalds/linux/blob/3e5e6c9900c3d71895e8bdeacfb579462e98eba1/arch/x86/include/asm/invpcid.h#L8
https://github.com/openbsd/src/blob/4e368faebf86e9a349446b5839c333bc17bd3f9a/sys/arch/amd64/include/cpufunc.h#L173
It's beyond me how this wasn't noticed earlier. The previous incorrect
layout could lead to TLB entries not being flushed and #GP faults.
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, thanks a lot!

@Freax13 Freax13 merged commit f85f015 into master Nov 10, 2024
8 of 13 checks passed
@Freax13 Freax13 deleted the fix/pcid-order branch November 10, 2024 08:55
@Freax13
Copy link
Member Author

Freax13 commented Nov 10, 2024

I briefly changed the branch protection rules, so that I could merge this PR without CI passing (there's already a fix for CI in #507).

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.

2 participants