Draft: explore LA57-aware virtual address validity

[aarkegz]

Purpose of this draft

This is a working prototype for LA57-aware virtual address types. It is meant to make the API choices and their effects reviewable in code, not to claim that all of those choices are final.

The work is inspired by #435 and #586. In particular, it agrees with the central idea in #586 that address validity should be represented by a generic parameter. It explores some additional questions that became visible while propagating that design through the crate and integrating it into my own OS program.

The current branch implements the runtime-valid-by-default end of the design space. That is useful as a prototype because it demonstrates what is required for a program to use the same address types while running in either LA48 or LA57 mode. It is not necessarily the best compatibility choice for the final API.

This PR is organized as a list of technical decisions. I have marked them as:

• Seems agreed: a direction that appears to be shared by the existing discussion and implementations.
• Proposed: a direction that seems technically sound and that I recommend.
• Alternative designs: multiple viable choices with different compatibility or semantic trade-offs.
• Open question: a point where more discussion is needed before the API can be considered final.

Scope

This prototype covers:

• fixed LA48 and LA57 virtual address validity;
• runtime validity based on the active value of CR4.LA57;
• construction, conversion, arithmetic, and const behavior;
• direct users of virtual addresses, such as descriptor structures, interrupt
  structures, basic page/range types, TLB operations, and register accessors;

It deliberately does not add P5 page-table traversal, P5 indices, or LA57 mapping support. Page-table APIs should be handled separately after the address model is settled. It also does not generalize PhysAddr: LA57 changes virtual address canonicality, not the architectural physical-address width.

[aarkegz]

1. The VirtAddr type

1.1 Which validity models are needed? — Seems agreed

There are three useful virtual-address validity models:

• VirtAddr48: canonical according to the fixed 48-bit rule;
• VirtAddr57: canonical according to the fixed 57-bit rule;
• VirtAddrRT: canonical according to the address-space mode that is active when the address is created.

The fixed variants can be checked without reading machine state, so their checked constructors are conceptually const. Whether the generic methods can actually be declared const at the crate's Rust 1.59 MSRV is a separate language-version issue discussed below. The runtime variant needs to read CR4.LA57 for checked construction, canonicalization, or any operation that produces and checks a new address. Those operations therefore require an x86_64 target, the instructions feature, and Ring 0 execution.

Operations that do not need to check canonicality remain available for all three types. For example, values can still be stored, compared, formatted, and converted to u64 without reading CR4.

If compatibility, const construction, privilege restrictions, and the cost of reading CR4 were ignored, using VirtAddrRT throughout the crate would be the most natural model: it directly describes the address space in which the program is currently executing. The other variants remain important because those constraints cannot be ignored in a general-purpose crate.

1.2 Representing validity with a sealed generic parameter — Seems agreed / Proposed

Both #586 and this prototype represent these models through a generic validity parameter, so the generic approach itself seems agreed. This prototype further proposes sealing the validity trait. A reduced form of the implementation is:

pub trait VirtAddrValidity: sealed::VirtAddrValiditySealed {}

#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct FixedValidity<const BITS: usize>;

#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct RuntimeValidity;

impl VirtAddrValidity for FixedValidity<48> {}
impl VirtAddrValidity for FixedValidity<57> {}
impl VirtAddrValidity for RuntimeValidity {}

#[repr(transparent)]
pub struct VirtAddr<V: VirtAddrValidity>(u64, PhantomData<V>);

The validity trait is sealed. The proposed API supports only these three policies, and allowing downstream implementations would make it harder for VirtAddr to rely on a closed set of invariants.

Keeping the implementations known to the crate also avoids requiring const_trait_impl. Fixed-width const constructors can use ordinary const helpers parameterized by the number of bits:

const fn canonicalize_with_bits(addr: u64, bits: usize) -> u64 {
    let shift = 64 - bits;
    ((addr << shift) as i64 >> shift) as u64
}

impl<const BITS: usize> VirtAddr<FixedValidity<BITS>>
where
    FixedValidity<BITS>: VirtAddrValidity,
{
    pub const fn try_new_const(addr: u64) -> Result<Self, VirtAddrNotValid> {
        try_new_with_bits(addr, BITS)
    }
}

Rust 1.59 rejects some generic const functions whose enclosing impl has the trait bounds required by this design. The prototype therefore uses #[rustversion::attr(since(1.61), const)]: the methods are callable on Rust 1.59, but become const only on Rust 1.61 and newer.

This leaves an explicit upstream choice. We can raise the MSRV to 1.61, accept that these generic methods are not const on the oldest supported compilers, or add concrete specialized methods where preserving Rust 1.59 const use is important. A concrete LA48 compatibility facade makes the specialized option possible without weakening the bound on the generic type.

1.3 Which model should be the default? — Alternative designs (Q1)

There are two defensible defaults.

Default LA48

Making the existing API resolve to the fixed LA48 type provides the strongest compatibility:

• existing const construction can remain available, subject to the Rust 1.59 generic-const limitation described in section 1.2;
• existing code retains the same validity invariant;
• code that only uses 48-bit addresses continues to work in both LA48 and LA57 mode, because every canonical LA48 address is also canonical under LA57;
• runtime-compatible code must opt in to VirtAddrRT and propagate it through its data structures.

This is the conservative choice and is probably the most appropriate choice if source compatibility is the primary constraint.

Default runtime validity

Making the existing API resolve to VirtAddrRT gives new and migrated programs the most natural runtime semantics:

• addresses read from the CPU have the expected type;
• the same program can store addresses from either LA48 or LA57 mode;
• validity follows the active machine mode without requiring two monomorphized versions of the program.

However, it removes const checked construction from the default type and makes some operations unavailable without x86_64 + instructions + Ring 0. It is therefore a source-incompatible change even if most runtime call sites can be migrated mechanically.

The current branch implements this second option to evaluate its complete effect. My preference for an upstream API is to preserve LA48 compatibility at the existing top-level path while making runtime validity an explicit and well-supported choice.

1.4 A default generic parameter does not preserve constructor syntax — Open question (Q2)

There is a Rust inference issue that is easy to miss. A declaration such as:

pub struct VirtAddr<V: VirtAddrValidity = FixedValidity<48>> {
    addr: u64,
    marker: PhantomData<V>,
}

impl<V: VirtAddrValidity> VirtAddr<V> {
    pub const fn zero() -> Self {
        Self { addr: 0, marker: PhantomData }
    }
}

let addr = VirtAddr::zero(); // error: the validity type cannot be inferred

does not apply the default type parameter when resolving an associated function whose result has no other type context. The same problem affects calls such as VirtAddr::new_unsafe(...). This behavior occurs on both the current compiler and the crate's Rust 1.59 MSRV.

Consequently, merely writing V = FixedValidity<48> does not fully preserve the existing API. #586 intends to add a default validity parameter and would be affected by the same issue wherever an associated function has no other type context. This is independent of which validity is selected as that default.

There are at least two viable ways to separate the concrete compatibility name from the generic type.

Option A: a generic type in a new public submodule

pub mod generic {
    pub struct VirtAddr<V: VirtAddrValidity>(u64, PhantomData<V>);
}

pub type VirtAddr = generic::VirtAddr<FixedValidity<48>>;
pub type VirtAddr48 = VirtAddr;
pub type VirtAddr57 = generic::VirtAddr<FixedValidity<57>>;
pub type VirtAddrRT = generic::VirtAddr<RuntimeValidity>;

The exact new submodule name is open. The important point is that the existing x86_64::addr::VirtAddr and its top-level re-export must retain their current LA48 meaning, for example by making both paths aliases of the fixed instantiation above. The generic type itself should not be placed at x86_64::addr::VirtAddr<V>: that would change the meaning of an existing public path and would therefore be a breaking change even if x86_64::VirtAddr were kept as an alias. The generic type should not have its own default parameter, because that would recreate the same inference ambiguity.

Option B: rename the generic type

pub struct GenericVirtAddr<V: VirtAddrValidity>(u64, PhantomData<V>);

pub type VirtAddr = GenericVirtAddr<FixedValidity<48>>;
pub type VirtAddr48 = VirtAddr;
pub type VirtAddr57 = GenericVirtAddr<FixedValidity<57>>;
pub type VirtAddrRT = GenericVirtAddr<RuntimeValidity>;

This also preserves the concrete API, but names such as GenericVirtAddr or VirtAddrWithValidity are less natural in generic signatures.

Either option can instead make the top-level VirtAddr alias refer to VirtAddrRT, but that only fixes the inference problem. It does not restore the old const constructors or the LA48 type invariant.

I currently prefer Option A with both existing VirtAddr paths preserved as LA48 compatibility aliases, but the new submodule name needs maintainer input.

1.5 Constructor names and const behavior — Proposed, conditional on the default

For an explicit fixed-width type, the prototype uses names that make const construction visible:

VirtAddr48::new_const(addr)
VirtAddr57::new_const(addr)
VirtAddr48::try_new_const(addr)
VirtAddr57::try_new_const(addr)
VirtAddr48::new_truncate_const(addr)
VirtAddr57::new_truncate_const(addr)

In the current runtime-valid-by-default prototype, the runtime type uses the conventional names:

VirtAddrRT::new(addr)
VirtAddrRT::try_new(addr)
VirtAddrRT::new_truncate(addr)

If the top-level VirtAddr instead remains a concrete LA48 compatibility alias, the old new, try_new, and new_truncate names can remain const methods on that specific type. In that design, the corresponding VirtAddrRT constructors can use distinct names, for example new_runtime, try_new_runtime, and new_truncate_runtime, so that fixed and runtime checking are not confused.

new_unsafe is conceptually const for every validity type. It does not perform a validity check, so it does not need CR4 or special treatment for RuntimeValidity. In the current bounded generic impl it uses the same conditional Rust 1.61 const attribute described in section 1.2.

1.6 Arithmetic and operations that produce addresses — Proposed

Arithmetic that produces a new address must preserve the selected validity invariant:

• it is always available for VirtAddr48 and VirtAddr57;
• it is available for VirtAddrRT only when the active mode can be read;
• subtraction of two addresses to produce a numeric distance does not create a new address and does not need the same restriction.

The prototype expresses this with an internal capability trait implemented for both fixed policies and, under x86_64 + instructions, for runtime validity. This applies to Add, AddAssign, Sub<u64>, SubAssign, and Step.

The same restriction must be propagated through wrapper types. For example, Page arithmetic and PageRange iteration also produce new virtual addresses and must not accidentally expose runtime arithmetic on configurations where the active mode cannot be checked.

1.7 Validity after CR4.LA57 changes — Open question (Q5)

The prototype gives VirtAddrRT the following invariant:

Validity is checked when an address is created. A later address-space mode change does not retroactively invalidate or modify existing values.

This keeps VirtAddrRT a plain, copyable, eight-byte value. Tracking the mode in every address would be expensive and would fundamentally change the type.

The consequence is that an address created while LA57 is enabled might not be valid if LA57 is later disabled. The reverse transition is not problematic for canonical LA48 addresses. The prototype provides is_valid_currently() under x86_64 + instructions so that an existing address can be checked explicitly.

What remains open is where revalidation belongs:

• The address type itself can reasonably make only a creation-time promise.
• A safe API that passes an address to hardware may need a stronger promise at the point of interaction.
• VirtAddr48 is canonical in both LA48 and LA57 and does not need such a check.
• VirtAddr57 may contain an LA57-only value.
• VirtAddrRT may have been created before a mode change.

We should decide whether safe hardware-consuming APIs revalidate when needed, accept only a type that is unconditionally valid for the operation, or document the absence of an intervening mode change as part of their contract.

1.8 Type availability and feature flags — Proposed

All three concrete address types and all three validity markers should be available without adding any new address-mode feature. In particular, a feature must not make the same public name denote a different type or give an existing operation different validity semantics. Cargo features are transitive and unified across a dependency graph, so such a feature could silently change an unrelated downstream user's address model.

Using mutually exclusive la48, la57, and runtime crate modes would make the public API depend on Cargo feature resolution and would interact poorly with feature unification. The validity type already expresses the caller's choice, so separate global address-space-mode features do not appear necessary.

A feature that only removes a particular new address type would be less surprising, because it would cause unavailable code to fail to compile instead of changing its meaning. It still does not appear necessary here. The existing instructions feature can continue to control access to operations that read CR4 or execute other privileged instructions; it must not select which validity type an unchanged API name represents.

[aarkegz]

2. Types and functions that depend on VirtAddr

2.1 General propagation rule — Proposed (Q3)

The current prototype deliberately propagates the validity parameter as far as possible in order to evaluate the full affected surface. This is an analysis strategy, not a proposal that every one of those generic parameters belongs in the final API.

I propose the following narrower rules:

1. A type should carry V when it stores VirtAddr<V> and that validity is a real invariant of the stored data.
2. If only one function temporarily constructs or returns an address, the validity choice should normally stay on that function instead of becoming a parameter of the entire containing type.
3. A pointer() method is a concrete example: the validity of the temporary pointer it returns does not necessarily justify making the pointed-to table type generic. A single V must not conflate an object's own address, addresses stored inside the object, and addresses later written by the CPU.
4. Addresses supplied by the active CPU naturally have runtime validity.
5. Addresses consumed by hardware must be valid in the mode active at the time of the interaction. Creation-time validity and current validity are not always the same guarantee.

2.2 Types for which a validity parameter is natural — Proposed

The following types directly store virtual addresses, so a validity parameter describes a real invariant:

DescriptorTablePointer<V> // stores the descriptor table base
TaskStateSegment<V>       // stores RSP and IST entries
Page<S, V>                // stores the page's start address
PageRange<S, V>           // stores Page<S, V> endpoints
PageRangeInclusive<S, V>
InvPcidCommand<V>         // the Address variant stores VirtAddr<V>
InvlpgbFlushBuilder<S, V> // stores PageRange<S, V>

Adding a default parameter can reduce migration noise in explicit type positions, but it does not by itself solve associated-function inference. For types with no-argument constructors, the same concrete-facade or split-method problem described for VirtAddr can occur.

Where a method parameter already determines V, there is usually no need to invent a _with_validity name. For example, both fixed and runtime page types can expose Page::containing_address(address) in disjoint impl blocks; the fixed implementation can be const and the runtime implementation can perform a runtime check.

2.3 Types that should probably not carry validity — Proposed change to the prototype

The current prototype gives GlobalDescriptorTable a validity parameter, but the GDT does not store a VirtAddr<V>. Its entries are raw descriptor values, and the parameter is only used through PhantomData when constructing a temporary pointer to the table itself.

This does not describe a useful invariant:

• the validity of addresses encoded in descriptors is not uniformly represented by the GDT's V;
• the address of the GDT object is independent from addresses stored in a TSS;
• choosing a type parameter cannot prove that the object happens to be located in the corresponding range.

The same issue appears if a TSS descriptor uses the validity of TaskStateSegment<V>'s internal stack fields to classify the address of the TSS object itself. These are separate addresses with separate validity questions.

I therefore propose that GlobalDescriptorTable remain non-generic. Its pointer() or load() implementation can construct a runtime-valid pointer to the table at the point where that pointer is needed. A generic Descriptor::tss_segment<V>(&TaskStateSegment<V>) can infer V from its argument without making the resulting raw descriptor or the GDT generic.

2.4 CPU-produced addresses — Alternative designs (Q4)

Several APIs read addresses directly from current CPU state:

• Cr2::read;
• FsBase::read, GsBase::read, and KernelGsBase::read;
• LStar::read;
• Segment64::read_base;
• read_rip;
• sgdt and sidt.

These results are naturally VirtAddrRT because they are addresses used by the currently active machine mode. Returning VirtAddr<V> chosen by the caller can be incorrect, as @phil-opp noted in the review of #586: a register can contain an LA57-only value while the caller requests the LA48 instantiation.

#586 suggested returning a non-generic RawVirtAddr and letting callers convert it into a checked address type. That remains a viable design, especially if the crate wants register reads to remain usable without assuming that the raw register contents satisfy a Rust address invariant.

The main alternatives are therefore:

1. Return VirtAddrRT, checking the value against the active mode. Where an instruction can expose a non-canonical raw value, the API can return Result<VirtAddrRT, _> instead of constructing an invalid address.
2. Return RawVirtAddr and require an explicit checked conversion. This avoids propagating generic methods and makes potentially unchecked hardware data visible, but introduces another public address wrapper. Such a type should remain a boundary representation rather than replacing semantic virtual addresses throughout the crate.
3. Preserve an LA48 compatibility method and add an explicit read_runtime method. This minimizes source breakage but duplicates many register APIs and leaves the old method unable to represent every valid value on LA57.

The current prototype primarily implements option 1. I think it is the most natural new API, while option 2 deserves further discussion for registers whose contents might legitimately be non-canonical or otherwise unchecked.

2.5 CPU-consuming APIs — Open question (Q5)

The corresponding write and instruction APIs include:

• FS/GS base and model-specific register writes;
• loading descriptor-table pointers;
• TLB invalidation for an address or page range;
• installing handler and stack addresses that the CPU will later consume.

Accepting any VirtAddr<V> through a generic or Into<RawVirtAddr> API is ergonomic, but the Rust type alone does not always prove that the value is valid in the mode active when the instruction executes.

There is an important asymmetry:

• every VirtAddr48 value is canonical in both LA48 and LA57 mode;
• a VirtAddr57 value may be valid only in LA57 mode;
• a VirtAddrRT value is valid for the mode in which it was created, but might need revalidation after a mode change.

Possible API rules include accepting only VirtAddrRT, accepting VirtAddr48 without checks and revalidating the other variants, or providing an unsafe generic entry point whose contract requires current validity. This choice should be made consistently across hardware-facing APIs.

2.6 Descriptor tables — Proposed direction

DescriptorTablePointer<V> should remain generic because it stores a base address. However, the high-level table objects require more care:

• GlobalDescriptorTable should probably remain non-generic, as described in section 2.3.
• sgdt() and sidt() read bases selected by the active CPU and should return either runtime-valid or raw descriptor pointers.
• lgdt() and lidt() consume a pointer in the current mode, so their safe or unsafe contract must account for current validity.
• A high-level GDT/IDT pointer() function only needs to choose validity for that returned pointer; it does not necessarily justify adding V to the table object.

2.7 IDT entries and interrupt stack frames — Open invariant/API question

The prototype currently propagates one validity parameter through the complete IDT type family:

InterruptDescriptorTable<V>
  -> Entry<HandlerFunc<V>, V>
  -> HandlerFunc<V>
  -> InterruptStackFrame<V>
  -> VirtAddr<V>

This is probably too broad because the parameter represents three different sources of addresses:

1. handler addresses written by software into IDT entries;
2. the address of the IDT object passed to lidt;
3. RIP and RSP values written by the CPU into an interrupt stack frame.

The third case is especially important. In LA57 mode, the CPU may push an LA57-only RIP or RSP. Exposing that frame to a handler as InterruptStackFrame<FixedValidity<48>> would create a value that violates the claimed VirtAddr48 invariant before user code has a chance to check it.

This suggests that the normal extern "x86-interrupt" handler types and the CPU-provided InterruptStackFrame should use runtime validity rather than an arbitrary fixed validity. Handler addresses stored in entries can be considered separately, and the address of the IDT itself should be handled when it is loaded.

A likely final shape is therefore closer to:

InterruptDescriptorTable<V>  // if V is retained, it describes handler addresses only
  -> Entry<HandlerFunc, V>
  -> HandlerFunc
  -> InterruptStackFrame     // runtime-valid CPU frame

It may be even simpler to make handler addresses runtime-valid as well and keep the entire existing IDT type non-generic. The current prototype intentionally keeps this issue visible, but it should not be interpreted as a final claim that generic fixed-validity interrupt frames are sound.

2.8 Basic pages and ranges — Proposed

Page<S, V>, PageRange<S, V>, and PageRangeInclusive<S, V> have real validity invariants because they store virtual addresses. Their address arithmetic and iterators must follow the same availability rules as VirtAddr<V> arithmetic.

The existing four-level page-table mapping APIs remain explicitly LA48 in this prototype. Adding a validity parameter to the basic Page value does not imply that the existing mapper implementations can traverse or modify five-level tables. That work should be designed independently.

[aarkegz]

3. Prototype status and validation

The prototype implements all three validity models and propagates them widely enough to expose the affected API surface. It has been checked across the relevant feature, target, documentation, lint, and Rust 1.59 configurations. A branch of my own OS program builds against the current prototype and it works as expected.

I would prefer to settle these questions before expanding the implementation to five-level page-table traversal. I am happy to revise the prototype in whichever direction reaches consensus.

[Freax13]

Thank your for your contribution!

Sorry for the late review. This is a massive PR and haven't had the time to do a full review yet. Here are some first comments.

If have a couple of concerns with the generic parameter approach:

1. Not everyone will use LA57, but with the current design everyone will have to explicitly mention the validity type in a lot of places. This adds quite a bit of boilerplate even though the vast majority of users never want anything other than LA48.
2. To my understanding one of the advantages of this approach is that users can use all of the validity types within the same binary and are not locked into any validity type (48 vs 57 vs RT). This is in contrast to the cargo feature based approach where the validity type/address space size is set at compile time. My concern with this is that I still don't understand what use cases require multiple validity types within the same binary. Are users not expected to use the same validity type everywhere? If there are use-cases expected to use several validity types, do we consider those important enough to potentially justify worse ergonomics for everyone else who doesn't need this?

For my own curiosity: Do you have a public project making use of this code and what does a migration from the old API to the new one look like?

[Freax13]

Shouldn't flush_pcid have a generic parameter for the validity?

What's the purpose of flush_pcid_inner?

[Freax13]

Can we make this (and the other mapper/translate types/traits) work with VirtAddr57 and VirtAddrRT?

[Freax13]

Suggested change

	/// # use x86_64::FixedValidity;
	/// # use x86_64::structures::paging::{
	/// # Mapper, Page, PhysFrame, FrameAllocator,
	/// # Size4KiB, OffsetPageTable, page_table::PageTableFlags,
	///# FixedValidity
	/// # };

[Freax13]

I'm not a fan of having separate const functions though I understand that we may not have much of a choice until const traits are stabilized.

[aarkegz]

Yes, it's a bitter choice

[Freax13]

Do we really need to re-check the validity of the aligned address? Given that we know that S::SIZE is smaller than 1<<47, isn't it guaranteed that the down-aligned address is always valid regardless of the address space size?

[Freax13]

We should add impls for the other validity types.

[aarkegz]

OK

[Freax13]

Isn't that just all validity types?

[Freax13]

Suggested change

	impl GlobalDescriptorTable<8, RuntimeValidity> {
	impl GlobalDescriptorTable {

[Freax13]

It seems we now have empty and empty_with_validity. AFAICT we don't do that in most other places. Why are we doing it here?

[aarkegz]

Thank your for your contribution!

Sorry for the late review. This is a massive PR and haven't had the time to do a full review yet. Here are some first comments.

If have a couple of concerns with the generic parameter approach:

1. Not everyone will use LA57, but with the current design everyone will have to explicitly mention the validity type in a lot of places. This adds quite a bit of boilerplate even though the vast majority of users never want anything other than LA48.
2. To my understanding one of the advantages of this approach is that users can use all of the validity types within the same binary and are not locked into any validity type (48 vs 57 vs RT). This is in contrast to the cargo feature based approach where the validity type/address space size is set at compile time. My concern with this is that I still don't understand what use cases require multiple validity types within the same binary. Are users not expected to use the same validity type everywhere? If there are use-cases expected to use several validity types, do we consider those important enough to potentially justify worse ergonomics for everyone else who doesn't need this?

For my own curiosity: Do you have a public project making use of this code and what does a migration from the old API to the new one look like?

Hi Freax13 and thanks for your reviewing and comments!

I was (until my graduation this June) a graduate student of Tsinghua University and a developer of the ArceOS project, where this great x86_64 crate is extensively used for x86-64 hardware operations. And personally I'm developing a derivative project named ecraOS aimed at adding runtime virtual address space detection and enabling.

For your first concern, I agree that compatibility should be of the primary priority when adding LA57 support. For now I think the best solution is to make VirtAddr a type alias such as pub type VirtAddr = VirtAddrGeneric<FixedValidity<48>>. This should eliminate most of (if not all, I haven't tested it thoroughly yet) the boilerplate codes, and the LA48-only users should be able to use this crate in the exact same way as before.

And for your second concern, I think OS kernels (and other bare metal applications) designed to be adaptive to different VA bits may find it useful to have access to all 3 validities. There's another thing to consider, that VirtAddrRT is by definition not possible to construct in const contexts, therefore VirtAddrRT-users still need VirtAddr48/VirtAddr57 for constants. And for ergonomics, I think the type alias mentioned above could (with careful consideration and design) be compatible and convenient enough for users that do not need and care about VirtAddr57/RT?

In my opinion, cargo features may not be a good choice here because features are additive and adding a dependency may change the final feature list in a completely unexpected way (e.g. an indirect dependency just enables a feature and you cannot disable it). It would be a serious problem if we use features to control the behavior of the VirtAddr struct. However, I think it's okay to add a feature to control the existence of VirtAddr57/RT, allowing VA48 users to get rid of them completely.

As for other types changed in this PR, maybe we should discuss them later? After we have a first-step agreement on how should VirtAddr looks like? I'm going to push a newer version to make VirtAddr48 the default, maybe later this week.

[aarkegz]

I missed one thing in the last comment, the cost of CR4 read. I'm not sure which option is better now, maybe we should just postpone the validation until the value is used? Or should we cache it in som way?

[Freax13]

For your first concern, I agree that compatibility should be of the primary priority when adding LA57 support. For now I think the best solution is to make VirtAddr a type alias such as pub type VirtAddr = VirtAddrGeneric<FixedValidity<48>>. This should eliminate most of (if not all, I haven't tested it thoroughly yet) the boilerplate codes, and the LA48-only users should be able to use this crate in the exact same way as before.

This works for VirtAddr, but what about all the other types and traits (e.g. Page or Mapper)? We could maybe add aliases for types (though that'd require a lot of them), but I don't think the same is possible for traits.

And for your second concern, I think OS kernels (and other bare metal applications) designed to be adaptive to different VA bits may find it useful to have access to all 3 validities. There's another thing to consider, that VirtAddrRT is by definition not possible to construct in const contexts, therefore VirtAddrRT-users still need VirtAddr48/VirtAddr57 for constants. And for ergonomics, I think the type alias mentioned above could (with careful consideration and design) be compatible and convenient enough for users that do not need and care about VirtAddr57/RT?

You raise a good point about const contexts.

In my opinion, cargo features may not be a good choice here because features are additive and adding a dependency may change the final feature list in a completely unexpected way (e.g. an indirect dependency just enables a feature and you cannot disable it). It would be a serious problem if we use features to control the behavior of the VirtAddr struct.

I disagree with this. Yes, if dependencies start randomly enabling the feature for this, that'd be a problem, but they just shouldn't do that unless the library crate cannot work without one of LA48/LA57 for some weird reason. These features should also be enabled by the binary crate.

The concept of "features that should only be enabled by the top level binary crate" isn't new. For example, sqlx uses features to chose the underlying async runtime and tracing has features to control the minimum log level. In both cases, bad things will happen if a library crate enables features that disagree with the choice by the top level binary crate, but there's no reason for library crates to do so.

As for other types changed in this PR, maybe we should discuss them later? After we have a first-step agreement on how should VirtAddr looks like? I'm going to push a newer version to make VirtAddr48 the default, maybe later this week.

Sure, I'm fine with working on this step by step.

I missed one thing in the last comment, the cost of CR4 read. I'm not sure which option is better now, maybe we should just postpone the validation until the value is used? Or should we cache it in som way?

I'm not sure what you mean by postponing the validation. Caching seems like a straightforward solution.

[aarkegz]

This works for VirtAddr, but what about all the other types and traits (e.g. Page or Mapper)? We could maybe add aliases for types (though that'd require a lot of them), but I don't think the same is possible for traits.

It's actually a little bit tricky. Ideally we should use default generic parameters like struct VirtAddr<V: VirtAddrValidity = FixedValidity<48>>(...) for all types involved, but Rust's type inference will cause compiler errors when there's only one generic parameter, while it works perfectly when there are more than one generic parameters and at least one is specified, e.g.:

fn foo() {
    // Failed to compile, trying to infer the generic parameter.
    let addr = VirtAddr::new(0x4200_0000);
    // It actually infers `V` from addr, but it compiles as well for methods that does not have any arg with V in it.
    let page = Page::<Size4KiB>::from_start_address(addr);
}

Therefore, I chose to use type aliases for types. That does create many boilerplate aliases (may be simplified by an internal macro?) but that's the only way I can find to support different validities while preserving compatibility. For traits, I have just checked them:

• Traits that require no modification:
  • Sealed.
  • PortAccess/PortReadAccess/PortWriteAccess/PortRead/PortWrite.
  • DebugAddressRegister.
  • Segment.
  • FrameAllocator/FrameDeallocator/PageSize/NotGiantPageSize/PageTableFrameMapping.
• Traits that require modification:
  • Segment64: it reads from/writes to VirtAddr. We can add two methods with default implementation read_base_to<V>/write_base_from<V> for VirtAddr types other than the default one. We can safely use new_unsafe to avoid the validation of the default VirtAddr type.
  • Mapper/MapperAllSizes/CleanUp: Adding a generic parameter seems to be unavoidable, need more consideration.
  • HandlerFuncType:
    • Firstly, itself returns a VirtAddr in to_virt_addr.
    • Also, handler functions implementing HandlerFuncType all receive an InterruptStackFrame as their first argument, which have several fields of type VirtAddr.
    • Two options are possible, first, add <V> to it and all handler function types, second, just use the default VirtAddr and let the user who want to be aware of different va-bits to perform the conversion.

I must say that maybe the best way is to introduce an unchecked VirtAddrRaw type for all hardware-interaction methods and types, and ask the user to explicitly convert VirtAddrRaw from and to VirtAddr types. But it's impossible to do so since we want compatibility. Maybe we can use the default VirtAddr as VirtAddrRaw? After all if the hardware returns a virtual address outsides of the range of the default VirtAddr, it must be the user's fault.

I disagree with this. Yes, if dependencies start randomly enabling the feature for this, that'd be a problem, but they just shouldn't do that unless the library crate cannot work without one of LA48/LA57 for some weird reason. These features should also be enabled by the binary crate.

The concept of "features that should only be enabled by the top level binary crate" isn't new. For example, sqlx uses features to chose the underlying async runtime and tracing has features to control the minimum log level. In both cases, bad things will happen if a library crate enables features that disagree with the choice by the top level binary crate, but there's no reason for library crates to do so.

I understand your point and that makes total sense. I prefer to keep VirtAddr48/VirtAddr57/VirtAddrRT separate type aliases and use features to gate the availability of VirtAddr57 and VirtAddrRT (to avoid compiling code legacy LA48-only users do not need) and to change the definition of pub type VirtAddr = ..., like:

[features]
# new features, none of them enabled by default
virt_addr_57 = []
virt_addr_rt = []
default_virt_addr_57 = ["virt_addr_57"] # Optional, useful but a little bit dangerous

This preserves compatibility and flexibility.

I'm not sure what you mean by postponing the validation. Caching seems like a straightforward solution.

There are pros and cons for both approaches:

• Postponing the validation, checking CR4 only when writing VirtAddrs to IDT/GDT/CR/...
  • Pros:
    • VirtAddrs are always 8-byte long.
    • Checks CR4 only when needed.
  • Cons:
    • Breaks the contract that it panics when constructing VirtAddrs with non-canonical addresses.
    • Requires updating IDT/GDT codes.
• Caching CR4.LA57
  • Pros:
    • Straightforward.
    • Keeps the current contract.
  • Cons:
    • VirtAddrRT is 16-byte long.
    • Maybe we could use the unused bits to store it, but it requires many bit operations.

Caching seems to be better but I'm not 100% sure. I haven't benchmarked the cost of CR4 seriously but reading it so often is (I feel too) worrisome. But doubling the size of VirtAddr or adding bit operations also introduces costs (though smaller than reading CR4). Caching it globally seems to be even worse.

[Freax13]

The more I think about all of this the more I come to the conclusion that there just isn't a clean solution. I guess if we want to support LA-57 we have to be content with a solution that may be a bit rough around the edges.

---

[...] But doubling the size of VirtAddr or adding bit operations also introduces costs (though smaller than reading CR4). Caching it globally seems to be even worse.

I'm not suggesting that we cache the value inside the VirtAddr. I'm suggesting that we cache it in a static variable.

I must say that maybe the best way is to introduce an unchecked VirtAddrRaw type for all hardware-interaction methods and types, and ask the user to explicitly convert VirtAddrRaw from and to VirtAddr types.

IMO I think we should avoid a VirtAddrRaw if possible. I'd prefer not to give up on strong types. That's just my opinion though.

But it's impossible to do so since we want compatibility.

We can make breaking changes for PRs targeting the next branch. This PR should target the next branch.

[aarkegz]

The more I think about all of this the more I come to the conclusion that there just isn't a clean solution. I guess if we want to support LA-57 we have to be content with a solution that may be a bit rough around the edges.

I agree...It won't be easy and clean.

I'm not suggesting that we cache the value inside the VirtAddr. I'm suggesting that we cache it in a static variable.

I get it. Maybe we should add a function like refetch_virt_addr_bits and ask VirtAddrRT-prefer users to call it after CR4 being updated.

IMO I think we should avoid a VirtAddrRaw if possible. I'd prefer not to give up on strong types. That's just my opinion though.

I've been giving this some more thought on this. For VirtAddr48/57-only users, we actually don't need to modify things other than VirtAddr itself, they can just use the VirtAddr type that the user desires. Two groups of users need the modified version of other types/functions: a. VirtAddrRT users (since VirtAddrRT is not allowed to be the default), and b. users who want to work with a non-current virtual address width. That's still quite complex but maybe it's possible to work on them one by one.

[aarkegz]

@Freax13 I've pushed an updated version, containing some designs we have talked above. Could you please take a look at the addr module (the updated VirtAddr)? Thanks.

For other types/methods, the current version is still temporary and I'll update them later.

[Freax13]

@Freax13 I've pushed an updated version, containing some designs we have talked above. Could you please take a look at the addr module (the updated VirtAddr)? Thanks.

For other types/methods, the current version is still temporary and I'll update them later.

Yeah, doing this piece by piece is a very good idea.

Here's a review for the the changes in the addr module. It's mostly smaller concerns. Overall, I think I'm happy with the direction it's taking so far.

[Freax13]

How well do this work with type inference? Is this sufficient to let the compiler figure out the generic parameter? Ideally, I'd like to see some code migrated to this new API to see how it impacts users (i.e. what changes they have to make during the transition).

[Freax13]

This function needs to be marked as unsafe. There's no guarantee that bits is correct. Passing in an incorrect value (i.e. 64) would allow creating uncanonical addresses.

[aarkegz]

OK

[Freax13]

We should add a p5_index getter.

[Freax13]

Suggested change

	allow(dead_code)
	expect(dead_code)

[aarkegz]

OK

[Freax13]

Should we even allow using RuntimeValidity if that's not the case? RuntimeValidity can't be used if that's not the case, can it?

[aarkegz]

Only a small subset of methods like new_unsafe, zero, as_u64, etc. I actually cannot imagine a scenario where VirtAddrRT should be used when instructions is not enabled or the target is not x86_64, but I think it's also not something bad to allow it.

[Freax13]

Suggested change

	let mask = (1u64 << <V>::bits()) - 1;
	let mask = (1u64 << V::bits()) - 1;

Here and elsewhere.

[aarkegz]

OK

[Freax13]

Let's move this conversion impl to the other ones.

[aarkegz]

It's here because it's #[cfg(all(feature = "instructions", target_arch = "x86_64"))].

[Freax13]

Duplicating methods like this isn't great (both for usability and maintainability), but it might be the best we can do.

If/when const traits ever get stabilized, we can probably merge to two implementations again.

[aarkegz]

Yes, this is the best we can do for now.

[aarkegz]

@Freax13 I've pushed an updated version, containing some designs we have talked above. Could you please take a look at the addr module (the updated VirtAddr)? Thanks.
For other types/methods, the current version is still temporary and I'll update them later.

Yeah, doing this piece by piece is a very good idea.

Here's a review for the the changes in the addr module. It's mostly smaller concerns. Overall, I think I'm happy with the direction it's taking so far.

Very happy to receive your review. I have made some fixes. Can we split the addr module into a separate PR so we can merge it into next earlier?