-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add aarch64 user support #1
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
base: main
Are you sure you want to change the base?
Conversation
when lower exception come, UserContext.run can return a reason.
pub use page_table_entry::MappingFlags as PageFaultFlags; | ||
|
||
pub use crate::TrapFrame; | ||
pub use linkme::distributed_slice as def_trap_handler; |
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.
Don't need to change these imports
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.
These are modified by rustfmt as well 🤣
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.
Then maybe you can enable the CICD to check the fmt
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.
https://github.com/Starry-OS/arceos also has this problem. Since we are working on a fork and the ultimate goal is to port it back upstream, if we make a coding style change here, the diff will be a disaster.
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 we keep the coding style consistent with the upstream by enabling the CI and use the same fmt confiig?
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 will not work as expected. The upstream repository does not configure rustfmt, which results in a very relaxed configuration, meaning that code formatted with the strict configuration will definitely pass checks, as is the case in this PR.
use aarch64_cpu::registers::{FAR_EL1, Readable}; | ||
use page_table_entry::MappingFlags; | ||
|
||
fn handle_instruction_abort_lower(tf: &TrapFrame, iss: u64, is_user: bool) -> ReturnReason { |
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 function only check whether it is a PageFault and return a ReturnReason
. Maybe we can merge the check logic with
Lines 66 to 91 in f046e84
fn handle_data_abort(tf: &TrapFrame, iss: u64) { | |
let wnr = (iss & (1 << 6)) != 0; // WnR: Write not Read | |
let cm = (iss & (1 << 8)) != 0; // CM: Cache maintenance | |
let access_flags = if wnr & !cm { | |
PageFaultFlags::WRITE | |
} else { | |
PageFaultFlags::READ | |
}; | |
let vaddr = va!(FAR_EL1.get() as usize); | |
// TODO: fixup_exception | |
// Only handle Translation fault and Permission fault | |
if !matches!(iss & 0b111100, 0b0100 | 0b1100) // IFSC or DFSC bits | |
|| !handle_trap!(PAGE_FAULT, vaddr, access_flags) | |
{ | |
panic!( | |
"Unhandled EL1 Data Abort @ {:#x}, fault_vaddr={:#x}, ESR={:#x} ({:?}):\n{:#x?}\n{}", | |
tf.elr, | |
vaddr, | |
ESR_EL1.get(), | |
access_flags, | |
tf, | |
tf.backtrace() | |
); | |
} | |
} |
- For user mode, it returns a error reason.
- For kernel mode, it call
handle_trap
directly to handle it.
@@ -1,5 +1,4 @@ | |||
.macro SAVE_REGS | |||
sub sp, sp, {trapframe_size} |
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.
Keep the sub
code here because the RESTORE_REGS
calls add
code so you need to keep them consistent
ldp x6, x7, [sp, 6 * 8] | ||
ldp x4, x5, [sp, 4 * 8] | ||
ldp x2, x3, [sp, 2 * 8] | ||
ldp x0, x1, [sp] |
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 we call RESTORE_REGS
macro here to restore user context?
Then you can call SAVE_REGS
in Task_EXIT
macro.
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.
I suggest moving _enter_user
and _user_trap_entry
into trap.S
.
Irq = 1, | ||
Fiq = 2, | ||
SError = 3, | ||
Irq = 1, |
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.
Why add the padding here?
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.
rustfmt
pub const fn sysno(&self) -> usize { | ||
self.r[8] as usize | ||
} | ||
|
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.
And also add set_sysno
.Ldeadloop: | ||
b .Ldeadloop |
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.
Is this used at all? I suspect it's there for debugging...?
src/aarch64/uspace.rs
Outdated
pub esr: u64, | ||
pub stval: usize, | ||
} | ||
|
||
/// Creates a new context from the given [`TrapFrame`]. | ||
pub const fn from(trap_frame: &TrapFrame) -> Self { | ||
Self(*trap_frame) | ||
impl ExceptionInfo { | ||
pub fn kind(&self) -> ExceptionKind { | ||
let esr: tock_registers::LocalRegisterCopy<u64, ESR_EL1::Register> = | ||
tock_registers::LocalRegisterCopy::new(self.esr); |
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.
Maybe you can store a LocalRegisterCopy
in ExceptionInfo
directly. It's Debug + Clone + Copy
.
panic!( | ||
"Unhandled {} Instruction Abort @ {:#x}, fault_vaddr={:#x}, ESR={:#x} \ | ||
({:?}):\n{:#x?}\n{}", | ||
if is_user { "EL0" } else { "EL1" }, | ||
tf.elr, | ||
vaddr, | ||
ESR_EL1.get(), | ||
access_flags, | ||
tf, | ||
tf.backtrace() | ||
); |
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.
Never panic for user mode exceptions. Instruct the caller that this kind of exception can not be handled.
src/aarch64/uspace.rs
Outdated
} | ||
} | ||
|
||
pub fn new(entry: usize, ustack_top: VirtAddr, _arg0: usize) -> Self { |
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.
Correctly set arg0
src/aarch64/uspace.rs
Outdated
impl UserContext { | ||
pub fn run(&mut self) -> ReturnReason { | ||
let tp_kind = unsafe { _enter_user(self) }; | ||
trace!("Returned from user space with TrapKind: {:?}", tp_kind); |
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.
Same here.
#[derive(Debug, Clone)] | ||
pub struct UserContext { | ||
tf: TrapFrame, | ||
sp_el1: u64, |
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.
I'm not quite familiar with AArch64, but I believe that there is no need to save SP_EL1
since SP_EL0
should be used instead in user mode? Ignore this if I'm wrong.
ldp x6, x7, [sp, 6 * 8] | ||
ldp x4, x5, [sp, 4 * 8] | ||
ldp x2, x3, [sp, 2 * 8] | ||
ldp x0, x1, [sp] |
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.
I suggest moving _enter_user
and _user_trap_entry
into trap.S
.
// AArch64 user space safe memory copy | ||
// Based on RISC-V implementation and optimized for AArch64 |
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.
Sorry, but where did this code come from? I can't find any reference, and it doesn't look like it was implemented by porting axcpu
's user_copy
implementation on RISC-V to AArch64...
It should account for possible page faults and set up the exception table.
Co-authored-by: Asakura Mizu <[email protected]>
No description provided.