rust-osdev / x86_64

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

Unifying physical and virtual address types across the rust OS development ecosystem. #498

Open jounathaen opened 1 week ago

jounathaen commented 1 week ago

Hi,

I think having the semantic differentiation between a PhysicalAddress and a VirtualAddress type provides a lot of value for OS development. What I think would be even better, is to have a [Physical|Virtual]Address type that is coherent across multiple crates AND across different architectures. This could simplify the reuse of code and the compatibility between projects.

Currently, the situation looks as follows:

So I was thinking of moving the addresses into a separate crate that is not x86_64 specific anymore. The types from that crate could be used and re-exported in the x86_64 crate without changing any interfaces. I guess it is possible to make it compatible with the x86 crate's types as well. And as I'm on it, I could also implement rust-vmm's traits and add a guest physical address type as well.

However, this only makes sense if there is interest in adopting this approach. Otherwise, we'll just end up with a XKCD927 situation. So, what are your thoughts?

Freax13 commented 6 days ago

Can you elaborate on what specific types/traits/etc would go into the common crate, what types/implementations would remain in this crate (or the other arch-specific crates), and how this would enable the reuse of code?

jounathaen commented 5 days ago

Sure, so I don't have a protoype for this yet, but my approach would be to copy-paste https://github.com/rust-osdev/x86_64/blob/master/src/addr.rs and add the functions from the x86 crate so that it'd be a drop-in replacement for both crates. x86_64 could then simply replace addr.rs with pub use addresscrate::{VirtAddr, PhysAddr}

To illustrate code reuse and interoperability: In HermitOS, we use numerous functions from the x86_64 crate, but as we target other architectures as well, we have to add our own physical/virtual address types. So anytime we call a function in the x86_64 crate, we have to do something like VirtAddress::new(addr.as_u64()). Also, we are writing our own hypervisor and have defined the hypercalls and data layouts in a separate crate to reduce code duplication. Parameters in the hypercalls are physical addresses, and it would be nice to use a type here that can seamlessly integrate with the x86_64 functions as well as the respective functionality for other architectures.

Freax13 commented 5 days ago

I'm not yet entirely clear how moving the address type alone helps reusability across architectures: In this crate, the VirtAddr isn't dependent on the build-platform, we expose the same functions with the same x86_64-specific behavior even if the target platform isn't x86_64. To me, this implies that the x86_64-specific VirtAddr would have to remain a separate type and other types would be needed for other architectures. If there are different types for different platforms, I'm not sure how they could be reusable. What's your take on this?

jounathaen commented 5 days ago

Ok, so what you want to say is that you also want the x86_64 crate to export the x86_64 address type (u64), even if it is compiled on some other architecture (like for example when programming an emulator), right?

That's a good point. I was thinking of something like this:

pub mod arch {
  pub mod x86_64 {
    #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    #[repr(transparent)]
    pub struct VirtAddr(u64);
    // ...
  }

  pub mod x86 {
    // Has the same functions (unless architecture specific) as arch::x86_64::VirtAddr.
    #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    #[repr(transparent)]
    pub struct VirtAddr(u32);
    // ...
  }
}

#[cfg(target_arch=x86_64)]
pub use crate::arch::x86_64::VirtAddr;

#[cfg(target_arch=x86)]
pub use crate::arch::x86::VirtAddr;

// Fallback type. Should probably not be used but prevents CI build/clippy errors when building for arch=none
#[cfg(not(any(target_arch = "x86_64", target_arch = "x86")))]
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(transparent)]
pub struct VirtAddr(usize);

and the _x8664 crate would then pub use addcrate::arch::x86_64::VirtAddr.

Freax13 commented 5 days ago

Ok, so what you want to say is that you also want the x86_64 crate to export the x86_64 address type (u64), even if it is compiled on some other architecture (like for example when programming an emulator), right?

That's what we do right now, and we even have CI set up to test that.

That's a good point. I was thinking of something like this:

pub mod arch {
  pub mod x86_64 {
    #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    #[repr(transparent)]
    pub struct VirtAddr(u64);
    // ...
  }

  pub mod x86 {
    // Has the same functions (unless architecture specific) as arch::x86_64::VirtAddr.
    #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    #[repr(transparent)]
    pub struct VirtAddr(u32);
    // ...
  }
}

#[cfg(target_arch=x86_64)]
pub use crate::arch::x86_64::VirtAddr;

#[cfg(target_arch=x86)]
pub use crate::arch::x86::VirtAddr;

// Fallback type. Should probably not be used but prevents CI build/clippy errors when building for arch=none
#[cfg(not(any(target_arch = "x86_64", target_arch = "x86")))]
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(transparent)]
pub struct VirtAddr(usize);

and the _x8664 crate would then pub use addcrate::arch::x86_64::VirtAddr.

That code structure wouldn't necessarily require the address type to be moved into the common crate though, would it? Couldn't you just re-export the crate from this type in the common crate?

jounathaen commented 5 days ago

First, I think this contradicts the idea of a unification, as it makes it way harder to keep different architecture variants on-par. Second, if the code is not part of that common crate, it is not possible to add functions that are necessary to keep compatibility with other crates (take the split function from x86 as a random example)

mkroening commented 5 days ago

My approach to this (which I have not had the time to iron out yet) is similar to what @Freax13 has been describing:

The purpose of the “common” crate is to have types to differentiate between virtual and physical addresses in cross-architecture operating systems, while being able to easily interface with crates such as x86_64. This makes the types dependent on the compile target. The x86-64 crate explicitly uses u64 everywhere to make its architecture-specific definitions usable from all compilation targets.

What I am thinking of would look something like this:

VirtAddr(usize);

impl From<crate::VirtAddr> for usize;
// More arch-independent helper functionality

#[cfg(all(feature = "x86_64", target_arch = "x86_64"))]
{
    impl From<crate::VirtAddr> for x86_64::VirtAddr;
    impl From<x86_64::VirtAddr> for crate::VirtAddr;
    // Checks for validity according to `x86_64::VirtAddr`
    impl TryFrom<usize> for crate::VirtAddr;
}

This keeps the invariants of the x86_64 types intact while allowing easy cross-platform functionalities and interfacing with arch-independent abstractions that care about the kind of address as well as with arch-dependent abstractions from crates such as x86_64.

So I think such a type would be a bad fit for x86_64 by design (wanting target-arch-dependence and thus wanting usize instead of u64, etc.), but a common type outside x86_64 for arch-dependence may be useful while still allowing interoperating with arch-independent code such as x86_64.

Freax13 commented 5 days ago

First, I think this contradicts the idea of a unification, as it makes it way harder to keep different architecture variants on-par.

On the one hand, I get that keeping different crates in sync is difficult if not impossible, but on the other hand, if someone using the x86_64 crate wants to change the interface of an address type (e.g. add a method), I wouldn't want them to be forced to support all other architectures as well.

Second, if the code is not part of that common crate, it is not possible to add functions that are necessary to keep compatibility with other crates (take the split function from x86 as a random example)

I'm not sure that this is a good argument though: I get why this would be very desirable for people targeting multiple platforms, but adding methods that don't necessarily make sense for one target just to be compatible with the method from another target doesn't feel right to me if you only care about one platform.

Here's another question: Do you care about methods working with const? If not, instead of moving all types into the common crate, can we have only traits + implementations + a re-export of the target's address types live in the common crate? That way the common crate could define whatever methods desired including those that only exist to be compatible with other architecture, but none of the architecture-specific crates are forced to implement a specific interface.

mkroening commented 4 days ago

can we have only traits + implementations + a re-export of the target's address types live in the common crate

I don't think we can do without at least a fallback type for architectures without specific support-crate. At that point, I would personally lean towards a common type with easy conversion, such as the one I drafted above. I am not sure, though. To me, it feels like deciding between a few .into() calls and additional trait imports while also thinking about implementation complexity. Using .into() might be easier to extend for new architectures than having to implement all the methods via the trait again.