Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release v0.15 #446

Closed
wants to merge 97 commits into from
Closed

release v0.15 #446

wants to merge 97 commits into from

Conversation

Freax13
Copy link
Member

@Freax13 Freax13 commented Jan 14, 2024

This PR prepares the v0.15 release.

We had two PRs #261 and #439 implementing a trait for interrupt handler functions. I resolved conflicts in favor of #439 to avoid unnecessary breakages.
Some of the changes from #317 conflicted with #422. I resolved these by using a larger integer type such that underflows are impossible and we never need to return an error for underflows.

TODO:

Freax13 and others added 30 commits June 4, 2021 00:30
This is a *breaking change*, also add note about safety.
Followup to rust-osdev#259. Code previously merged as part of rust-osdev#227.

Signed-off-by: Joe Richey <[email protected]>
This moves the code_selector to the `EntryOptions` structure.

While rebasing this, I also updated the `Debug` implementation to
actually print out useful information instead of just hex codes.

I left the type field as binary, as that's what the SDM does.

Signed-off-by: Joe Richey <[email protected]>
This makes the value easier to use and slightly improves the `Debug`
implementation. Also, change the `InterruptStackFrame` to
`#[repr(transparent)]`. It doesn't really matter but makes it clear that
`InterruptStackFrame` and `InterruptStackFrameValue` are ABI compatible.

Note that this now makes `InterruptStackFrameValue`s imposible to
construct, but that seems fine.

Signed-off-by: Joe Richey <[email protected]>
implement `Index<u8>` for IDT instead of `Index<usize>` and add implementations for indexing with ranges
`ltr` atomically writes to memory when it changes the referenced
tss entry's type to Busy 64-bit TSS. This means that the inline assembly
in `load_tss` should not have the `nomem` option and that the `table`
field in `GlobalDescriptorTable` must use a type that is interiorly
mutable.
…types

change `cpu_flags`'s type to `RFlags`
Co-authored-by: Philipp Oppermann <[email protected]>
fix `load_tss` and `GlobalDescriptorTable` and add `SingleUseCell`
Changelog.md Outdated

## Other improvements

- [idt: Fixup Options structure and cleanup set_handler_fn](https://github.com/rust-osdev/x86_64/pull/226)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why this PR was merged into the next branch. AFAICT it doesn't introduce any breaking changes and doesn't depend on any changes in the next branch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the behavior of set_handler_function/set_handler_addr was changed in a subtle way. Before #226, only the present was added to the entry options. Since #226, the whole options field is reinitialized, which means that the additional options are reset (if they were set before).

We should probably add the doc comment introduced in #226 to both set_handler_function and set_handler_addr:
https://github.com/rust-osdev/x86_64/pull/261/files#diff-399920f192f3fe5242eba18a5bdecd151fe662e3432e4836a34d11a7b8076b54R630-R638

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this part of the set_handler_fn comment is no longer true:

/// This method is only usable with the abi_x86_interrupt feature enabled. Without it, the
/// unsafe [Entry::set_handler_addr] method has to be used instead.

The function is no longer gated on the abi_x86_interrupt feature, only the impls for HandlerFunc* are. So users can still use this function if they provide their own HandlerFuncType implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if they provide their own HandlerFuncType implementation

Speaking of which, we should probably make the HandlerFuncType an unsafe trait, given that we trust the to_virt_addr output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I forgot that we only use the trait with specific types defined in InterruptDescriptorTable. So there is no reason for users to implement this trait. Still, an unsafe trait would probably better represent our intentions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this part of the set_handler_fn comment is no longer true:

/// This method is only usable with the abi_x86_interrupt feature enabled. Without it, the
/// unsafe [Entry::set_handler_addr] method has to be used instead.

The function is no longer gated on the abi_x86_interrupt feature, only the impls for HandlerFunc* are. So users can still use this function if they provide their own HandlerFuncType implementation.

It's still only usable if abi_x86_interrupt is active because the HandlerFuncType is gated behind abi_x86_interrupt:

macro_rules! impl_handler_func_type {
($f:ty) => {
#[cfg(feature = "abi_x86_interrupt")]
impl HandlerFuncType for $f {
#[inline]
fn to_virt_addr(self) -> VirtAddr {
VirtAddr::new(self as u64)
}
}
};
}
impl_handler_func_type!(HandlerFunc);
impl_handler_func_type!(HandlerFuncWithErrCode);
impl_handler_func_type!(PageFaultHandlerFunc);
impl_handler_func_type!(DivergingHandlerFunc);
impl_handler_func_type!(DivergingHandlerFuncWithErrCode);

@Freax13 Freax13 marked this pull request as ready for review January 14, 2024 14:52
@Freax13
Copy link
Member Author

Freax13 commented Jan 14, 2024

There are still a bunch of deprecated functions on master. We'll want to get rid of them before releasing 0.15. @phil-opp can we still use #447 to get next up to date with master, so that we can create a PR targeting next removing all those deprecated functions?

…recate

remove deprecated from_bits_unchecked functions
@Freax13
Copy link
Member Author

Freax13 commented Jan 14, 2024

I think this PR is ready now.

@Freax13
Copy link
Member Author

Freax13 commented Jan 14, 2024

Should we do a beta release first?

@phil-opp
Copy link
Member

I'm fine with doing a beta release, but let's give @josephlr some time to take a look before we do the stable.

I created one more PR to update the docs for set_handler_fn: #451

Update docs to clarify new `set_handler_fn` behavior
@phil-opp
Copy link
Member

phil-opp commented Feb 6, 2024

@josephlr Friendly ping! Are you fine with releasing v0.15?

@josephlr
Copy link
Contributor

josephlr commented Feb 6, 2024

Sorry for not being very responsive over the past month. I will try to take a look today, but overall a I have no issue with releasing 0.15

Copy link
Contributor

@josephlr josephlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thanks @Freax13 and @phil-opp for all the work.

There are a few Clippy findings which should either be fixed or silenced, but I don't think that should block release. I noticed that this PR is actually out of sync with the next branch (doesn't include #451), should we reopen a fresh PR which uses rust-osdev:next as the source branch?

@Freax13
Copy link
Member Author

Freax13 commented Feb 8, 2024

I noticed that this PR is actually out of sync with the next branch (doesn't include #451), should we reopen a fresh PR which uses rust-osdev:next as the source branch?

Sounds good to me.

@Freax13 Freax13 closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.15 Tracking issue
3 participants