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

Fix the exception handlers. #27

Merged
merged 7 commits into from
Apr 10, 2025
Merged

Fix the exception handlers. #27

merged 7 commits into from
Apr 10, 2025

Conversation

jonathanpallant
Copy link
Contributor

@jonathanpallant jonathanpallant commented Apr 9, 2025

It was looking at CPSR's Thumb bit, which tells you if the handler is in thumb mode, not the code that threw the fault.

I changed the undef-exception test to validate the address of the failing function, to verify that we've got this right. I also split the prefetch-exception test into separate T32 and A32 versions, to check that we return back into the correct state. I didn't bother splitting the data abort test.

I also fixed the issue of _asm_default_undefined_handler damaging r4. You have to save all the state first, then you can touch registers.

@jonathanpallant
Copy link
Contributor Author

cc @robamu

@jonathanpallant jonathanpallant changed the title Fix the undef handler. Draft: Fix the exception handlers. Apr 9, 2025
@jonathanpallant jonathanpallant changed the title Draft: Fix the exception handlers. Fix the exception handlers. Apr 9, 2025
@jonathanpallant
Copy link
Contributor Author

OK, I think this is good to go. I learned a lot about exception handling today 🙃

@jonathanpallant jonathanpallant changed the title Fix the exception handlers. Draft: Fix the exception handlers. Apr 9, 2025
@jonathanpallant
Copy link
Contributor Author

No, it's still wrong 😭

The undefined exception handler is supposed to also return to the faulting instruction, and we currently don't. This is because we push LR before we work out which mode we're in, and then pop the original LR of the stack with RFEFD. We need to save our state manually.

Comment on lines +40 to +51
core::arch::global_asm!(
r#"
// fn bkpt_from_a32();
.arm
.global bkpt_from_a32
.type bkpt_from_a32, %function
bkpt_from_a32:
bkpt #0
bx lr
.size bkpt_from_a32, . - bkpt_from_a32
"#
);

Choose a reason for hiding this comment

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

unrelated, but this looks like it could fit cortex-a::asm/cortex-r::asm. Perhaps as an unsafe function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it doesn't help here because I need to know the precise address of the instruction that does the breakpoint.

It was looking at CPSR's Thumb bit, which tells you if the *handler* is in thumb mode, not the code that threw the fault.

Change the test to validate the address of the failing function, to verify that we've got this right.

Also fixes the issue of _asm_default_undefined_handler damaging r4. You have to save all the state first, then you can touch registers.
One for T32 and one for A32. We have to do this because the exception is non-recoverable (we return to the faulting instruction, which faults again).
I had to move the DFSR read to after the alignment check was disabled, otherwise it double-faulted.

Also it turns out the versatileab tests were running with the Cortex-A linker script not the Cortex-R linker script, and the Cortex-R linker script had a bug in it that we previously missed (it wasn't calling the new exception handlers).
@jonathanpallant
Copy link
Contributor Author

😭 😭 😭 😭

OK, I think I now work out which mode (A32 or T32) tripped the undefined abort, without damaging any registers. This took a lot of work to debug, but it seems to work now?

@jonathanpallant jonathanpallant changed the title Draft: Fix the exception handlers. Fix the exception handlers. Apr 9, 2025
@jonathanpallant
Copy link
Contributor Author

Tentatively claiming that I am finished here.

thejpster and others added 2 commits April 9, 2025 22:08
- The SVC asm trampoline can now be over-ridden
- The Undefined, Prefetch and Abort handlers can either return never, or can return a new address to continue executing from when the handler is over
- I tried to ensure the all the handlers are listed in the same order as the vector table, for consistency
- Put every function in its own section in the hope that unused ones will be GC'd by the linker
More exception handling updates.
@jonathanpallant
Copy link
Contributor Author

jonathanpallant commented Apr 9, 2025

Reader, I was not finished here.

But now I am. I swear.

@@ -1,22 +1,25 @@
//! # Run-time support for Arm Cortex-A
Copy link
Contributor

Choose a reason for hiding this comment

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

Cortex-A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦

@robamu
Copy link
Contributor

robamu commented Apr 10, 2025

really nice work!

Also clarify that both only apply to AArch32 systems. The terms 'Cortex-R'
and 'Cortex-A' are particular brands of processor made by Arm, and there
are processors under each of those brands that run in (or only run in)
AArch64 mode.
@jonathanpallant
Copy link
Contributor Author

Let's go! Before I fix something else.

@jonathanpallant jonathanpallant merged commit b493320 into main Apr 10, 2025
57 checks passed
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.

4 participants