search

rust-osdev / x86_64

Library to program x86_64 hardware.
https://docs.rs/x86_64
Apache License 2.0
802 stars 133 forks source link

Inconsistency with bitmask of `addr` function in `PageTableEntry` #511

Closed sarahspberrypi closed 1 week ago

sarahspberrypi commented 1 week ago

Hi,

I noticed an inconsistency of the bitmask in the addr function for PageTableEntry and the requirements of VirtAddr::new(). The bitmask in question is 0x000f_ffff_ffff_f000 which indicates 5-level paging support. However, VirtAddr itself expects a 47 bit long 4-level address. This actually breaks when using SEV, as that sets bit 51 (C-Bit): PageTableEntry::addr does not mask this bit, but VirtAddr::new panics. Since the C-Bit for AMD SEV can be either at bit 47 (older machines) or bit 51 (newer machines), I think we should adapt VirtAddr to allow bits 47-51 to be non-matching as well.

Side note: If we align the bitmasks here with VirtAddrs requirements, we can safely use VirtAddr::new_unchecked() at this location and save a few CPU cycles.

Kind regards, Sarah

sarahspberrypi commented 1 week ago

Upon further investigation, I realized that the error itself seems to be coming from somewhere else. I have to find out more about this and will close this issue in the meantime.

Freax13 commented 6 days ago

I noticed an inconsistency of the bitmask in the addr function for PageTableEntry and the requirements of VirtAddr::new().

That's because PageTableEntry::addr returns a physical address, not a virtual address. The page tables only store physical addresses, they do not contain any virtual addresses.

The bitmask in question is 0x000f_ffff_ffff_f000 which indicates 5-level paging support.

This bitmask is completely unrelated to 4-level and 5-level paging support. Page tables are architecturally defined to contain a 40-bit field containing bits [51:12] of a physical address (see, AMD64 Architecture Programmer’s Manual, Volume 2, 5.4 Page-Translation-Table Entry Fields, 5.4.1 Field Definitions). The bitmask you noticed corresponds to these bits. The bitmask does not depend on whether or not 4 or 5 level paging is active, it always covers bits 51:12 (some CPUs don't support all 52 bits, but that's independent from 5-level paging).

However, VirtAddr itself expects a 47 bit long 4-level address.

VirtAddr expects a 48-bit long address with the highest bit being sign-extended into the upper bits.

This actually breaks when using SEV, as that sets bit 51 (C-Bit): PageTableEntry::addr does not mask this bit, but VirtAddr::new panics. Since the C-Bit for AMD SEV can be either at bit 47 (older machines) or bit 51 (newer machines), I think we should adapt VirtAddr to allow bits 47-51 to be non-matching as well.

Again, the C bit used with SEV (or the S bit used with TDX) is part of the physical address, not the virtual address. Our type used for physical addresses, PhysAddr, supports setting the 51st bit.

Personally, I've been using this crate with both SEV and TDX guests and can attest that it's compatible with both technologies. The linux-svsm and Enarx (two projects making use of SEV) both use this crate without issue.

Upon further investigation, I realized that the error itself seems to be coming from somewhere else. I have to find out more about this and will close this issue in the meantime.

If you have any more questions about this or AMD SEV/Intel TDX, feel free to ask.