-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add support for the x86-interrupt calling convention #39832
Add support for the x86-interrupt calling convention #39832
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
I like it! There was a lot of reverse engineering to get interrupt handlers to compile correctly for Redox, it would be nice if the compiler did more work |
While I appreciate that /cc @nikomatsakis who IIRC had some small concerns about naked fn, as well as @nagisa I think? |
oh and @hawkw |
This is definitely going to make my life much easier, thanks @phil-opp! I can confirm the difficulties you mention with naked functions; it's not always obvious when using Rust code in a function marked as I'll be awaiting this PR eagerly. |
So my opinion on interrupt CCs haven't changed at all since the last interrupt convention PR, so I won't bother with arguing about it yet again. I want to call out some points though:
At this point there's just about two things I'm worried about. How hard is it to support interrupt CCs for less common platforms such as ppc, MIPS etc. And how willing will we be to remove current buggy impl of naked functions. |
Seems like the RFC should be revisited (if only with a "we'll call it experimental with no plans to stabilize" outcome) if this is going to be supported. I'm happy to update it if there's demand. |
// feature gate is not used. | ||
|
||
extern "x86-interrupt" fn foo() {} | ||
//~^ ERROR x86-interrupt ABI is experimental and subject to change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This separate test is redundant? feature-gate-abi.rs already tests everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it required for tidy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can one file (feature-gate-abi.rs in this case) contain several // gate-test-NAME-OF-THE-FEAT comments?
Yes. Its not required to be in a separate file, and just as ok to put it into feature-gate-abi.rs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then I'm going to remove this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact it already has multiple such lines.
I didn't want to imply that assembly-free kernel are possible with this. My point was that you don't need inline assembly for interrupt handlers (compared to the naked function approach).
AFAIK, the x86-interrupt cc is very similar to preserve_all. It just adds some additional magic such as checking for a valid function signature, passing the exception stack frame pointer/error code, popping the error code off the stack before returning, and using So the x86-interrupt cc does not save all registers, but only the registers used by the callee. For example, consider this code: extern "x86-interrupt" fn handler(stack_frame: &ExceptionStackFrame) {
use core::sync::atomic::{AtomicUsize, Ordering};
static C: AtomicUsize = AtomicUsize::new(0);
C.fetch_add(1, Ordering::Relaxed);
} The function only increases an atomic counter. The compiler generates the following code with optimizations:
The function uses no registers, so there's no need to save/restore them.
The alignment problem is caused by the error code that is pushed by the CPU for certain exceptions. Thus the stack does not have the 16-byte alignment that the x86_64 ABI requires on function entry. So using preserve_all would have the same problem. I'm not sure about the cause of the xmm issue, though. It might be because x86_64 requires SSE support, so that nobody thought of the "x86-64 without sse" case. I suspect that the preserve_all calling convention has the same issue. |
You're missing my point entirely I think. I'm not proposing use of of preserveall as a direct interrupt handler at all. Instead what I was suggesting is use of a naked function trampoline into such preserve all function, which avoids all the pushing and popping cost described in the PR description. Such trampoline will realign stack pointer just fine avoiding the misaligned push problem as well. If anything, all you managed to do is convincing me that generic interrupt CC with automatically generated trampolines to a preserveall function is a good idea. |
5dc31cd
to
fddcdea
Compare
Ah, now I get it :). I agree that "naked wrapper trampoline to preserve_all function" is a good and more general solution. However, the performance will be a bit worse than with the Both x86-interrupt and preserve_all have direct LLVM support and only require minimal changes, so I would propose to add support for both. |
My two cents is that this is the right way to go. I can see use-cases for both, and if the implementation isn't too difficult, why not provide multiple options? The performance benefit from |
I can confirm I wasn't considering that case when I wrote the code implementing the interrupt CC. Couldn't say about preserve_all without looking into it. |
nominating because I don't really know what to do with this PR. Main points to bring up:
|
It is a good PR to discuss during the mtg, indeed. @pnkfelix do not forget to also tag the team when nominating an issue. |
r? @nagisa (update: manually assigning; nagisa pointed out that he is not on bors' list, and I cannot quickly determine the syntax, if any, to tell bors to assign the PR to nagisa.) |
During yesterday’s meeting we’ve decided to proceed with landing this for now, however this will not be considered for stabilisation without a RFC. I, myself, will be investigating a general "interrupt" ABI wherein the compiler does the right thing depending on the target platform (falling back to generated trampolines) and maybe write an RFC if the results are favourable. The implementation looks fine to me. I could importune a test of some sort, but it seems to me like one cannot really compose anything other than a codegen test or a test that calls the interrupt-abi function from rust to rust. Both of these are kind of useless, so, with or without a test… r=me |
fff2f15
to
03fad81
Compare
I added a simple codegen test and squashed the commits. |
@bors r=nagisa |
📌 Commit 03fad81 has been approved by |
Added |
@bors r=nagisa |
📌 Commit ba66d92 has been approved by |
⌛ Testing commit ba66d92 with merge 4ea6605... |
💔 Test failed - status-travis |
☔ The latest upstream changes (presumably #40206) made this pull request unmergeable. Please resolve the merge conflicts. |
Tracking issue: rust-lang#40180 This calling convention can be used for definining interrupt handlers on 32-bit and 64-bit x86 targets. The compiler then uses `iret` instead of `ret` for returning and ensures that all registers are restored to their original values. Usage: ``` extern "x86-interrupt" fn handler(stack_frame: &ExceptionStackFrame) {…} ``` for interrupts and exceptions without error code and ``` extern "x86-interrupt" fn page_fault_handler(stack_frame: &ExceptionStackFrame, error_code: u64) {…} ``` for exceptions that push an error code (e.g., page faults or general protection faults). The programmer must ensure that the correct version is used for each interrupt. For more details see the [LLVM PR][1] and the corresponding [proposal][2]. [1]: https://reviews.llvm.org/D15567 [2]: http://lists.llvm.org/pipermail/cfe-dev/2015-September/045171.html
ba66d92
to
b448058
Compare
Rebased |
@bors: r=nagisa |
📌 Commit b448058 has been approved by |
…ention, r=nagisa Add support for the x86-interrupt calling convention This calling convention can be used for definining interrupt handlers on 32-bit and 64-bit x86 targets. The compiler then uses `iret` instead of `ret` for returning and ensures that all registers are restored to their original values. Usage: ```rust extern "x86-interrupt" fn handler(stack_frame: &ExceptionStackFrame) {…} ``` for interrupts and exceptions without error code and ```rust extern "x86-interrupt" fn handler_with_err_code(stack_frame: &ExceptionStackFrame, error_code: u64) {…} ``` for exceptions that push an error code (e.g., page faults or general protection faults). The programmer must ensure that the correct version is used for each interrupt. For more details see the [LLVM PR][1] and the corresponding [proposal][2]. [1]: https://reviews.llvm.org/D15567 [2]: http://lists.llvm.org/pipermail/cfe-dev/2015-September/045171.html It is also possible to implement interrupt handlers on x86 through [naked functions](https://github.com/rust-lang/rfcs/blob/master/text/1201-naked-fns.md). In fact, almost all existing Rust OS projects for x86 use naked functions for this, including [Redox](https://github.com/redox-os/kernel/blob/b9793deb59c7650f0805dea96adb6b773ad99336/arch/x86_64/src/lib.rs#L109-L147), [IntermezzOS](https://github.com/intermezzOS/kernel/blob/f959cc18c78b1ba153f3ff7039d9ecc07f397628/interrupts/src/lib.rs#L28-L72), and [blog_os](https://github.com/phil-opp/blog_os/blob/844d739379ffdea6a7ede88365ec6e21a725bbf5/src/interrupts/mod.rs#L49-L64). So support for the `x86-interrupt` calling convention isn't absolutely needed. However, it has a number of benefits to naked functions: - **No inline assembly needed**: [Inline assembly](https://doc.rust-lang.org/book/inline-assembly.html) is highly unstable and dangerous. It's pretty easy to mess things up. Also, it uses an arcane syntax and requires that the programmer knows x86 assembly. - **Higher performance**: A naked wrapper function always saves _all_ registers before calling the Rust function. This isn't needed for a compiler supported calling convention, since the compiler knows which registers are clobbered by the interrupt handler. Thus, only these registers need to be saved and restored. - **Safer interfaces**: We can write a `set_handler` function that takes a `extern "x86-interrupt" fn(&ExceptionStackFrame)` and the compiler ensures that we always use the right function type for all handler functions. This isn't possible with the `#[naked]` attribute. - **More convenient**: Instead of writing [tons of assembly boilerplate](https://github.com/redox-os/kernel/blob/b9793deb59c7650f0805dea96adb6b773ad99336/arch/x86_64/src/lib.rs#L109-L147) and desperately trying to improve things [through macros](https://github.com/phil-opp/blog_os/blob/844d739379ffdea6a7ede88365ec6e21a725bbf5/src/interrupts/mod.rs#L17-L92), we can just write [code like this](https://github.com/phil-opp/blog_os/blob/e6a61f9507a4c4fef6fb4e3474bc596391bc97d2/src/interrupts/mod.rs#L85-L89). - **Naked functions are unreliable**: It is allowed to use Rust code inside a naked function, which sometimes works and sometimes not. For example, [calling a function](https://github.com/redox-os/kernel/blob/b9793deb59c7650f0805dea96adb6b773ad99336/arch/x86_64/src/lib.rs#L132) through Rust code seems to work fine without function prologue, but [code declaring a variable](https://is.gd/NQYXqE) silently adds a prologue even though the function is naked (look at the generated assembly, there is a `movl` instruction before the `nop`). **Edit**: See the [tracking issue](rust-lang#40180) for an updated list of issues. Unfortunately, the implementation of the `x86-interrupt` calling convention in LLVM has some issues that make it unsuitable for 64-bit kernels at the moment: - LLVM always tries to backup the `xmm` registers on 64-bit platforms even if the target doesn't support SSE. This leads to invalid opcode exceptions whenever an interrupt handler is invoked. I submitted a fix to LLVM in [D29959](https://reviews.llvm.org/D29959). The fix is really small (<10 lines), so maybe we could backport it to [Rust's LLVM fork](https://github.com/rust-lang/llvm)?. **Edit**: The fix was merged to LLVM trunk in [rL295347](https://reviews.llvm.org/rL295347). Backported in rust-lang/llvm#63. - On targets with SSE support, LLVM uses the `movaps` instruction for saving the `xmm` registers, which requires an alignment of 16. For handlers with error codes, however, the stack alignment is only 8, so a alignment exception occurs. This issue is tracked in [bug 26413](https://bugs.llvm.org/show_bug.cgi?id=26413). ~~Unfortunately, I don't know enough about LLVM to fix this.~~ **Edit**: Fix submitted in [D30049](https://reviews.llvm.org/D30049). This PR adds experimental support for this calling convention under the `abi_x86_interrupt` feature gate. The implementation is very similar to rust-lang#38465 and was surprisingly simple :). There is no accepted RFC for this change. In fact, the [RFC for interrupt calling convention](rust-lang/rfcs#1275) from 2015 was closed in favor of naked functions. However, the reactions to the recent [PR](rust-lang#38465) for a MSP430 interrupt calling convention were [in favor of experimental interrupt ABIs](rust-lang#38465 (comment)). - [x] Add compile-fail tests for the feature gate. - [x] Create tracking issue for the `abi_x86_interrupt` feature (and link it in code). **Edit**: Tracking issue: rust-lang#40180 - [x] Backport [rL295347](https://reviews.llvm.org/rL295347) to Rust's LLVM fork. **Edit**: Done in rust-lang/llvm#63 @tari @steveklabnik @jackpot51 @ticki @hawkw @thepowersgang, you might be interested in this.
…ention, r=nagisa Add support for the x86-interrupt calling convention This calling convention can be used for definining interrupt handlers on 32-bit and 64-bit x86 targets. The compiler then uses `iret` instead of `ret` for returning and ensures that all registers are restored to their original values. Usage: ```rust extern "x86-interrupt" fn handler(stack_frame: &ExceptionStackFrame) {…} ``` for interrupts and exceptions without error code and ```rust extern "x86-interrupt" fn handler_with_err_code(stack_frame: &ExceptionStackFrame, error_code: u64) {…} ``` for exceptions that push an error code (e.g., page faults or general protection faults). The programmer must ensure that the correct version is used for each interrupt. For more details see the [LLVM PR][1] and the corresponding [proposal][2]. [1]: https://reviews.llvm.org/D15567 [2]: http://lists.llvm.org/pipermail/cfe-dev/2015-September/045171.html It is also possible to implement interrupt handlers on x86 through [naked functions](https://github.com/rust-lang/rfcs/blob/master/text/1201-naked-fns.md). In fact, almost all existing Rust OS projects for x86 use naked functions for this, including [Redox](https://github.com/redox-os/kernel/blob/b9793deb59c7650f0805dea96adb6b773ad99336/arch/x86_64/src/lib.rs#L109-L147), [IntermezzOS](https://github.com/intermezzOS/kernel/blob/f959cc18c78b1ba153f3ff7039d9ecc07f397628/interrupts/src/lib.rs#L28-L72), and [blog_os](https://github.com/phil-opp/blog_os/blob/844d739379ffdea6a7ede88365ec6e21a725bbf5/src/interrupts/mod.rs#L49-L64). So support for the `x86-interrupt` calling convention isn't absolutely needed. However, it has a number of benefits to naked functions: - **No inline assembly needed**: [Inline assembly](https://doc.rust-lang.org/book/inline-assembly.html) is highly unstable and dangerous. It's pretty easy to mess things up. Also, it uses an arcane syntax and requires that the programmer knows x86 assembly. - **Higher performance**: A naked wrapper function always saves _all_ registers before calling the Rust function. This isn't needed for a compiler supported calling convention, since the compiler knows which registers are clobbered by the interrupt handler. Thus, only these registers need to be saved and restored. - **Safer interfaces**: We can write a `set_handler` function that takes a `extern "x86-interrupt" fn(&ExceptionStackFrame)` and the compiler ensures that we always use the right function type for all handler functions. This isn't possible with the `#[naked]` attribute. - **More convenient**: Instead of writing [tons of assembly boilerplate](https://github.com/redox-os/kernel/blob/b9793deb59c7650f0805dea96adb6b773ad99336/arch/x86_64/src/lib.rs#L109-L147) and desperately trying to improve things [through macros](https://github.com/phil-opp/blog_os/blob/844d739379ffdea6a7ede88365ec6e21a725bbf5/src/interrupts/mod.rs#L17-L92), we can just write [code like this](https://github.com/phil-opp/blog_os/blob/e6a61f9507a4c4fef6fb4e3474bc596391bc97d2/src/interrupts/mod.rs#L85-L89). - **Naked functions are unreliable**: It is allowed to use Rust code inside a naked function, which sometimes works and sometimes not. For example, [calling a function](https://github.com/redox-os/kernel/blob/b9793deb59c7650f0805dea96adb6b773ad99336/arch/x86_64/src/lib.rs#L132) through Rust code seems to work fine without function prologue, but [code declaring a variable](https://is.gd/NQYXqE) silently adds a prologue even though the function is naked (look at the generated assembly, there is a `movl` instruction before the `nop`). **Edit**: See the [tracking issue](rust-lang#40180) for an updated list of issues. Unfortunately, the implementation of the `x86-interrupt` calling convention in LLVM has some issues that make it unsuitable for 64-bit kernels at the moment: - LLVM always tries to backup the `xmm` registers on 64-bit platforms even if the target doesn't support SSE. This leads to invalid opcode exceptions whenever an interrupt handler is invoked. I submitted a fix to LLVM in [D29959](https://reviews.llvm.org/D29959). The fix is really small (<10 lines), so maybe we could backport it to [Rust's LLVM fork](https://github.com/rust-lang/llvm)?. **Edit**: The fix was merged to LLVM trunk in [rL295347](https://reviews.llvm.org/rL295347). Backported in rust-lang/llvm#63. - On targets with SSE support, LLVM uses the `movaps` instruction for saving the `xmm` registers, which requires an alignment of 16. For handlers with error codes, however, the stack alignment is only 8, so a alignment exception occurs. This issue is tracked in [bug 26413](https://bugs.llvm.org/show_bug.cgi?id=26413). ~~Unfortunately, I don't know enough about LLVM to fix this.~~ **Edit**: Fix submitted in [D30049](https://reviews.llvm.org/D30049). This PR adds experimental support for this calling convention under the `abi_x86_interrupt` feature gate. The implementation is very similar to rust-lang#38465 and was surprisingly simple :). There is no accepted RFC for this change. In fact, the [RFC for interrupt calling convention](rust-lang/rfcs#1275) from 2015 was closed in favor of naked functions. However, the reactions to the recent [PR](rust-lang#38465) for a MSP430 interrupt calling convention were [in favor of experimental interrupt ABIs](rust-lang#38465 (comment)). - [x] Add compile-fail tests for the feature gate. - [x] Create tracking issue for the `abi_x86_interrupt` feature (and link it in code). **Edit**: Tracking issue: rust-lang#40180 - [x] Backport [rL295347](https://reviews.llvm.org/rL295347) to Rust's LLVM fork. **Edit**: Done in rust-lang/llvm#63 @tari @steveklabnik @jackpot51 @ticki @hawkw @thepowersgang, you might be interested in this.
Don't other architectures also have specialized ABIs for interrupts? Wouldn't it be more reasonable to have a generalized extern "interrupt" that works on whatever architecture you're compiling for? Even if we don't implement them all for all of the architectures Rust supports, it seems like a better practice to encourage. |
The RFC touches on that, but as I see it the most important points:
|
Some platforms have more than one C ABIs and yet we still have |
Hey,
Sometimes, this has nothing to do with the registers actually used by the interrupt handler. Is there any way to force saving all registers using this convention, and access them in the interrupt handler (ie. extended ExceptionStackFrame) ? If not, then the use of naked function will be the only option ... |
Each of those sounds to me like distinct calling conventions. To use the system call example, handling the x86 It might be reasonable to define a different calling convention that acts like save-all but makes the machine's entry state available, but I'm not convinced it would be worthwhile since exposing a struct would inherently require spilling all registers (which you can easily do yourself) and making each register a unique parameter is both cumbersome (you end up with a large number of parameters, many of which may go unused) and requires user code be aware of what resources exist (which is what you need to do when writing a custom calling convention with naked functions). Naked functions are the ultimate escape hatch for these situations, so the argument to be made is that there is an efficient way to expose the machine state on function entry while restoring it on exit which is sufficiently general to be worth the maintenance burden. |
Overview
This calling convention can be used for definining interrupt handlers on 32-bit and 64-bit x86 targets. The compiler then uses
iret
instead ofret
for returning and ensures that all registers are restored to theiroriginal values.
Usage:
for interrupts and exceptions without error code and
for exceptions that push an error code (e.g., page faults or general protection faults). The programmer must ensure that the correct version is used for each interrupt.
For more details see the LLVM PR and the corresponding proposal.
Compared to naked functions
It is also possible to implement interrupt handlers on x86 through naked functions. In fact, almost all existing Rust OS projects for x86 use naked functions for this, including Redox, IntermezzOS, and blog_os. So support for the
x86-interrupt
calling convention isn't absolutely needed.However, it has a number of benefits to naked functions:
set_handler
function that takes aextern "x86-interrupt" fn(&ExceptionStackFrame)
and the compiler ensures that we always use the right function type for all handler functions. This isn't possible with the#[naked]
attribute.movl
instruction before thenop
).Known issues
Edit: See the tracking issue for an updated list of issues.
Unfortunately, the implementation of the
x86-interrupt
calling convention in LLVM has some issues that make it unsuitable for 64-bit kernels at the moment:LLVM always tries to backup the
xmm
registers on 64-bit platforms even if the target doesn't support SSE. This leads to invalid opcode exceptions whenever an interrupt handler is invoked. I submitted a fix to LLVM in D29959. The fix is really small (<10 lines), so maybe we could backport it to Rust's LLVM fork?. Edit: The fix was merged to LLVM trunk in rL295347. Backported in x86 interrupt calling convention: only save xmm registers if the target supports SSE llvm#63.On targets with SSE support, LLVM uses the
movaps
instruction for saving thexmm
registers, which requires an alignment of 16. For handlers with error codes, however, the stack alignment is only 8, so a alignment exception occurs. This issue is tracked in bug 26413.Unfortunately, I don't know enough about LLVM to fix this.Edit: Fix submitted in D30049.About this PR
This PR adds experimental support for this calling convention under the
abi_x86_interrupt
feature gate. The implementation is very similar to #38465 and was surprisingly simple :).There is no accepted RFC for this change. In fact, the RFC for interrupt calling convention from 2015 was closed in favor of naked functions. However, the reactions to the recent PR for a MSP430 interrupt calling convention were in favor of experimental interrupt ABIs.
Todo
abi_x86_interrupt
feature (and link it in code). Edit: Tracking issue: Tracking issue for thex86-interrupt
calling convention #40180CC
@tari @steveklabnik @jackpot51 @ticki @hawkw @thepowersgang, you might be interested in this.