x86_64 icon indicating copy to clipboard operation
x86_64 copied to clipboard

0.15 Tracking issue

Open josephlr opened this issue 4 years ago • 17 comments

This issue tracks any breaking changes we want to make for 0.15.

NOTE: All breaking changes should be made against the next branch, this branch should then be merged in right before releasing 0.15. Also, remember to update Changelog.md.

  • [x] Change software_interrupt!() macro to be a cosnt generic function software_interrupt<const NUM: u8>(). Done in #227 (and also #259 and #260)
  • [x] Change the idt::Entry and idt::EntryOptions structures to move the code selector into EntryOptions. Also, introduce the InterruptFn trait which allows for abstracting over all function pointers that can be used as interrupt handlers. Done in #226 (and also in #261)
  • [ ] Use step_trait to implement Step for PhysFrame and Page. See #212 . This may or may not be a breaking change, depending on how we handle PageRange/PageRangeInclusive/etc....
    • Given that step_trait isn't going to be stable for awhile, this doesn't seem like something we need to do for 0.15.
  • [x] Use correct types for InterruptStackFrameValue. See #263:
    • code_segment should be a SegmentSelector (what about padding?)
    • cpu_flags should be a RFlags #324
    • stack_segment should be a SegmentSelector (what about padding?)
    • What about non-canonical rip and rsp?
  • [x] Rename control flags to have better names
    • Cr4Flags::PROTECTION_KEY => Cr4Flags::PROTECTION_KEY_USER
    • XCr0Flags::YMM => XCr0Flags::AVX
    • Initial PR: https://github.com/rust-osdev/x86_64/pull/272
    • Revised PR (without breaking changes): https://github.com/rust-osdev/x86_64/pull/273
  • [ ] #448
    • Should there be a common abstraction? Some way to traverse the two formats of XSAVE data?
    • This is additive, so it can wait until after we release 0.15
  • [x] Should we change the behavior of VirtAddr::try_new
    • Tracked in #299
    • Done in #370
  • [x] We may also want to change VirtAddrNotValid and PhysAddrNotValid to actually contain the invalid address instead of just containing the bad high bits. Done in #347 against the master branch.
  • [x] Remove usize impls for Add/Sub on VirtAddr/PhysAddr, see https://github.com/rust-osdev/x86_64/issues/293#issuecomment-901804330 Breaking change done in #364
  • [x] Make 1.59 the unconditional MSRV. Done in #359
  • [x] #333 Done in #360
  • [x] #363
  • [x] Interior mutability of the GDT
    • Pointed out in #322
    • Initial fix in #323
    • Add entry type in #380
    • Allow shared reference in #381
    • Use an array with from_raw_entries ?

josephlr avatar Jun 04 '21 09:06 josephlr

Note that const_fn_fn_ptr_basics and const_fn_trait_bound were stabilized in 1.61

If we support only 1.61 and later, we could then remove most of our const_fn! from this crate.

@phil-opp @Freax13 would we want increase our minimum rust version to be 1.61 for v0.15.0? Would we feel comfortable increasing the minimum rust version in a patch release? Do we have a policy on this?

josephlr avatar Mar 24 '22 07:03 josephlr

Ah, that's great! Increasing the minimum Rust version for v0.15 sounds fine to me in general, as it is a semver-incompatible release with breaking changes. However, it's still 8 (?) weeks until Rust 1.61 becomes stable, so requiring it would mean that we have to postpone this release until May if we want it to keep supporting stable Rust. So I think it would be better to do this as part of a future v0.16 release. Until then, we could use the rustversion macro to provide the new const functionality when a new enough compiler is used.

phil-opp avatar Mar 24 '22 08:03 phil-opp

Oh that crate looks amazing, it would let us get rid of our const_fn! macro entirely!!

If we don't need Rust 1.61 for v0.15, then I think we should have the MSRV be 1.56 so that we can use the rust-version field in our crate.

josephlr avatar Mar 24 '22 08:03 josephlr

If we don't need Rust 1.61 for v0.15, then I think we should have the MSRV be 1.56 so that we can use the rust-version field in our crate.

Sure, sounds good to me!

phil-opp avatar Mar 24 '22 08:03 phil-opp

Oh wait we use inline assembly, so the min stable version is already 1.59 if the "instructions" feature is used. We should probably just make that the unconditionally minimum supported version for 0.15

josephlr avatar Mar 24 '22 09:03 josephlr

One thing I was considering for 0.15 were changes to VirtAddr's underlying representation on 64-bit platforms. If we changed VirtAddr to look like:

#[repr(transparent)]
pub struct VirtAddr(*const ());

on 64-bit platforms, we could then have the following const fn methods:

impl VirtAddr {
    // Safety: raw pointer has to be canonical
    #[cfg(target_pointer_width = "64")]
    pub const unsafe fn from_ptr_unsafe<T>(ptr: *const T) -> Self {
        Self(ptr as *const ())
    }

    #[cfg(target_arch = "x86_64")]
    pub const fn from_ref<T>(r: &T) -> Self {
        // Safety: references point to valid data, so are canonical.
        unsafe { Self::from_ptr_unsafe(r) }
    }
}

This would make it possible to construct something like a TaskStateSegment, DescriptorTablePointer, or some other structure containing a VirtAddr at compile time.

The downside of this (other than implementation complexity) is that the following methods would have to be made non-const:

  • VirtAddr::as_u64()
  • VirtAddr::page_offset()
  • VirtAddr::p{1,2,3,4}_index() / VirtAddr::page_table_index()
  • The various alignment methods can't yet const, but making them const might be harder if the underlying datatype is pointer

I'm not sure how bad this downside would be.

josephlr avatar Mar 31 '22 07:03 josephlr

We probably need to think about pointer provenance at some point. There is currently an open proposal to make the usize to pointer conversion more strict. See the Rust's Unsafe Pointer Types Need An Overhaul post for some background information.

I'm not sure what the correct approach for our VirtAddr type would be. On one hand, we treat it like a pointer, e.g. in the VirtAddr::as_ptr method. On the other hand, the type should be allowed to cross address space bounds (e.g. kernels that keep track of the stack pointers of preempted threads), which might (?) be invalid under strict provenance rules. So maybe we need to split this type up at some point, e.g. by removing all pointer-related methods form VirtAddr and using pointer types in more places.

Given this issue, I don't think that it's a good idea to switch VirtAddr between pointer-based and u64-based depending on the platform. It would lead to more confusion and less clear semantics.

phil-opp avatar Mar 31 '22 08:03 phil-opp

Given this issue, I don't think that it's a good idea to switch VirtAddr between pointer-based and u64-based depending on the platform. It would lead to more confusion and less clear semantics.

I agree with that, having some methods only be const on 64-bit targets seems unintuitive. That being said I'd still be interested in potential use cases.

Freax13 avatar Mar 31 '22 17:03 Freax13

Those articles are very interesting, and I agree that changing things in this space is really complicated. It definitely seems like keeping VirtAddr as a u64 is the best bet for now.

So maybe we need to split this type up at some point, e.g. by removing all pointer-related methods form VirtAddr and using pointer types in more places.

This is a good idea, I'll see if I can put some examples together in a separate tracking issue.

josephlr avatar Apr 02 '22 19:04 josephlr

@phil-opp @Freax13 is there anything else we want to get in before 0.15? All the currently pending PRs are additive, so could be done after we release 0.15

josephlr avatar Apr 16 '22 20:04 josephlr

I can't think of anything.

Freax13 avatar Apr 17 '22 12:04 Freax13

Me neither!

phil-opp avatar Apr 18 '22 10:04 phil-opp

What's the state of this? Should we finally prepare the 0.15 release?

phil-opp avatar Mar 08 '23 07:03 phil-opp

We could wait for #404, but this also wouldn't be a deal breaker for me, especially because we don't seem to have come to a final design on the changes just yet.

Other than that, I think we're ready.

Freax13 avatar Mar 08 '23 12:03 Freax13

I'll get started on rebasing next onto master.

Freax13 avatar Mar 08 '23 12:03 Freax13

I'll get started on rebasing next onto master.

Hm, never mind, I forgot we don't do rebases. I'll get started on merging next into master instead.

Freax13 avatar Mar 08 '23 12:03 Freax13

I'll get started on rebasing next onto master.

Hm, never mind, I forgot we don't do rebases. I'll get started on merging next into master instead.

I'm done with that. Depending on whether on not we want to wait for some other breaking prs to also be included, I can create a pr.

Freax13 avatar Mar 08 '23 13:03 Freax13