Implement conversion traits for PhysAddr and VirtAddr

[mikeleany]

Implements the following conversions:

• TryFrom<u64> for VirtAddr and PhysAddr
• TryFrom<usize> for VirtAddr and PhysAddr
• From<u32> for VirtAddr and PhysAddr
• TryFrom<*const T> for VirtAddr
• TryFrom<*mut T> for VirtAddr
• From<&T> for VirtAddr
• From<&mut T> for VirtAddr
• From<VirtAddr> for u64, *const T and *mut T
• From<PhysAddr> for u64

I restricted pointer and usize conversions with #[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]. This restriction isn't strictly necessary for TryFrom, but without it we would need a different error type than what try_new uses.

Closes #268. See also #293 (comment) by @josephlr.

[phil-opp]

Thanks for the PR! Looks all reasonable to me.

cc @josephlr @Freax13 What do you think about these new impls?

[Freax13]

Do we want to deprecate the old conversion methods/constructors?

[josephlr]

Do we want to deprecate the old conversion methods/constructors?

I think the only issue here would be that some of those are const, while the trait implementations might not be (at least for now).

Otherwise the idea seems reasonable to me.

[Freax13]

We should unify the cfg gates for pointer conversions and similar operations, we use four different variations:

• cfg(target_pointer_width = "64")
  • VirtAddr::as_ptr, VirtAddr::as_mut_ptr
  • <VirtAddr as Add<usize>>::add, <VirtAddr as AddAssign<usize>>::add_assign, <VirtAddr as Sub<usize>>::sub, <VirtAddr as SubAssign<usize>>::sub_assign, <PhysAddr as Add<usize>>::add, <PhysAddr as AddAssign<usize>>::add_assign, <PhysAddr as Sub<usize>>::sub, <PhysAddr as SubAssign<usize>>::sub_assign
    • These implementations are removed on the next branch because we implement these traits for u64.
  • <*const T as From<VirtAddr>>::from, <*mut T as From<VirtAddr>>::from
    • introduced by this pr
• cfg(any(target_pointer_width = "32", target_pointer_width = "64"))
  • VirtAddr::from_ptr
    • There's a comment stating that we do this for backward compatibily reasons, this has already been changed to cfg(target_pointer_width = "64") on the next branch.
• cfg(any(target_pointer_width = "16", target_pointer_width = "32", target_pointer_width = "64"))
  • <VirtAddr as TryFrom<usize>>::try_from, <VirtAddr as TryFrom<*const T>>::try_from and <VirtAddr as TryFrom<*mut T>>::try_from
    • introduced by this pr
• cfg(any(target_arch = "x86", target_arch = "x86_64"))
  • <VirtAddr as From<&T>>::from and <VirtAddr as From<&mut T>>::from
    • introduced by this pr

It's not entirely clear to me which variation we should use.

[mikeleany]

@Freax13

We should unify the cfg gates for pointer conversions and similar operations, we use four different variations:

It's not entirely clear to me which variation we should use.

For conversions from pointers (and references), I think cfg(any(target_arch = "x86", target_arch = "x86_64")) makes the most sense to me because

• there's a legitimate use case to convert x86 pointers to a VirtAddr when running 16-bit or 32-bit system initialization code prior to entering long mode,
• but a pointer on any other architecture is not likely to correspond in any way with a virtual address on x86_64, even if the pointers happen to be the same size.

For conversions to pointers target_arch = "x86" cannot be allowed for infallible conversions. Perhaps we could implement From when on x86_64 and TryFrom on x86 if people want that.

usize is a little different since it's an integer rather than a pointer. However, as I think about it, I think it makes more sense to require conversion from a u64 rather than usize on non-x86 based architectures, even if usize happens to be 64 bits. Doing otherwise would encourage non-portable code.

---

So I think the following cfg gates might be the most reasonable in my view:

cfg(any(target_arch = "x86", target_arch = "x86_64")) whenever converting from pointers or usize to VirtAddr

cfg(target_arch = "x86_64) whenever performing infallible conversions from VirtAddr to pointers or usize

However, if I modified the cfg gates on the existing inherent methods, that would technically be a breaking change, though I'm not sure if that would actually affect anybody.

[Freax13]

For conversions from pointers (and references), I think cfg(any(target_arch = "x86", target_arch = "x86_64")) makes the most sense to me because

• there's a legitimate use case to convert x86 pointers to a VirtAddr when running 16-bit or 32-bit system initialization code prior to entering long mode,

• but a pointer on any other architecture is not likely to correspond in any way with a virtual address on x86_64, even if the pointers happen to be the same size.

For conversions to pointers target_arch = "x86" cannot be allowed for infallible conversions. Perhaps we could implement From when on x86_64 and TryFrom on x86 if people want that.

:+1:

usize is a little different since it's an integer rather than a pointer. However, as I think about it, I think it makes more sense to require conversion from a u64 rather than usize on non-x86 based architectures, even if usize happens to be 64 bits. Doing otherwise would encourage non-portable code.

We could also just drop the usize impls. There's already some precedence for preferring u64 impls over usize impls: https://github.com/rust-osdev/x86_64/pull/364.

So I think the following cfg gates might be the most reasonable in my view:

cfg(any(target_arch = "x86", target_arch = "x86_64")) whenever converting from pointers or usize to VirtAddr

cfg(target_arch = "x86_64) whenever performing infallible conversions from VirtAddr to pointers or usize

:+1:

However, if I modified the cfg gates on the existing inherent methods, that would technically be a breaking change, though I'm not sure if that would actually affect anybody.

Yes, breaking changes should only be merged into the next branch. This could be done in a follow up pr.