feat(addr): add 57-bit and runtime-validated VirtAddr types

[aarkegz]

This PR is part of #599 and introduces the generic VirtAddrGen struct, the sealed VirtAddrValidity trait, and the specialized types VirtAddr48 (the 48-bit VA, corresponding to the old VirtAddr), VirtAddr57 (the 57-bit VA; requires the virt_addr_57 feature), and VirtAddrRT (the runtime-validated VA; requires the virt_addr_rt feature).

VirtAddr is now a type alias for VirtAddr48 for compatibility. Alternatively, enabling the default_virt_addr_57 feature makes VirtAddr an alias for VirtAddr57, so the 57-bit VA becomes the default. When default_virt_addr_57 is enabled, most data structures and functions in this crate automatically use the 57-bit VA, except for the paging module (which requires further consideration and implementation about page levels; I think that work should be done in a separate PR).

[aarkegz]

@Freax13 BTW I've just noticed #600 , is it possible for this PR and follow-up PRs to make it into this release?

[phil-opp]

Thanks a lot for your extensive work on this!

I've just noticed #600 , is it possible for this PR and follow-up PRs to make it into this release?

Could you clarify which parts of your proposal are breaking changes? Or is it possible to keep things completely backwards compatible?

My preference would be to get #600 out soon and not rush this complex 57-bit work to be included. I'd rather do another semver-breaking release if necessary.

[aarkegz]

Thanks for your reply!

Could you clarify which parts of your proposal are breaking changes? Or is it possible to keep things completely backwards compatible?

Yes. Based on my testing and review so far, when default_virt_addr_57 is not enabled, the new VirtAddr48 (and the new VirtAddr alias) is a drop-in replacement for the current VirtAddr. My goal is to add full support for VirtAddr57 and VirtAddrRT to this crate. For now, I think this work should be done in three steps:

1. Adding VirtAddr57/VirtAddrRT support and allowing VirtAddr57 to be used as the default type. (This PR)
2. Adding 5-level page table support and a page iterator for the 57-bit virtual address space. This is likely to be backwards compatible.
3. Making existing data structures and functions work with VirtAddrRT. As discussed in Draft: explore LA57-aware virtual address validity #599, this would be difficult and unlikely to be backwards compatible.

[aarkegz]

My preference would be to get #600 out soon and not rush this complex 57-bit work to be included. I'd rather do another semver-breaking release if necessary.

Thanks for the thoughtful reply. I completely understand your preference to get #600 out soon and not rush the complex 57-bit work, and I agree that a separate semver-breaking release may be the safer path for full 57-bit support.

That said, I wonder if we could find a narrow compromise. If we can agree that this PR (step 1, and possibly step 2) is sufficiently backwards compatible, and that it won't become an obstacle to any future implementation of full 57-bit support regardless of how that is eventually done, would you be open to including step 1 (and maybe step 2) in this release? I have a fairly immediate need for these features, so it would help me a lot.

If necessary, I can also prioritize step 2 and complete it as soon as possible to make this more feasible. Of course, I'm happy to adjust the scope or provide more details to make it safe. I'd appreciate your thoughts.

[phil-opp]

I took a quick look, not a full review.

We would also need to adjust the Page code because it also includes code for the address space gap, e.g. in the PageRange iterator impl.

[phil-opp]

This function is safe, but the bits is not checked either, is it? So why does the safety requirement from try_new_with_bits not apply here?

/// The caller must ensure that bits is valid for the selected validity policy. This is not
/// checked.

[aarkegz]

Fixed

[phil-opp]

This seems dangerous because cargo unifies features. Imagine that you leave the default_virt_addr_57 disabled deliberately, but some other crate in your cargo workspace (maybe even in an unrelated package) enables it. When you build the whole workspace, the feature is now enabled for both packages. But when you build the packages separately (or install them via crates.io), the feature stays disabled for your package.

So cargo features should be additive and not change behavior. Otherwise your code might panic/fail depending on which cargo build command you use.

[phil-opp]

As noted above, behavior-changing features are dangerous. This here is especially dangerous because it is an alias that seems to indicate backwards compatibility. However, it might silently change types if any other crate in the workspace enables the feature. Given that existing users of x86_64 relied on the fact that VirtAddr is 48 bits, their code will probably break in that case.

So if we add an alias for backwards compatibility we need to make sure that it actually is backwards compatible.

[aarkegz]

Hi @phil-opp , thanks for your review!

I've added unsafe to new_truncate_with_bits.

For the default_virt_addr_57 feature, I've discussed with @Freax13 (here, here, and here). I completely agree that behavior-changing features are dangerous and should be avoided when possible. However, with it most data structures and functions in this crate automatically get 57-bit VA support, and it's much more complex without it.

[phil-opp]

most data structures and functions in this crate automatically get 57-bit VA support

They don't really get "support" for it though, they are silently switched to 57-bit addresses (and you don't fully control the switch). But this switch requires some code changes, so you can easily end up with something broken.

For example, consider OffsetPageTage. Imagine that you have existing code that uses level_4_table to get a reference to the outermost table and traverse it manually 4 times to get to the mapped page. When the feature is enabled now this code will be broken.

So I don't think that there is a way around making these Virtaddr-dependent structures also generic over the virtual address space size. I.e. types like Page, OffsetPageTable, etc

[phil-opp]

For the register types, we could probably just always use the 57bit addresses, given that all 48 bit addresses are also valid 57 bit addresses. So no need to make them generic. We should provide From/TryFrom impls for this.

If we want to be fancy, we could gate the 57bit writes to registers on some CPUID token that checks whether the CPU supports 5 level paging. I.e. a empty struct that checks CPUID in its constructor and then can be passed to the 57bit write functions. (Apparently it's allowed to write 57bits even if it's not turned on.)