From b0d028fbf13f8c746e1af8ce2b1eeddadfec9945 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 30 Dec 2024 10:02:24 -0800 Subject: [PATCH] refac(hal_x86_64): define ISRs at the top level (#497) Currently, all the interrupt service routines provided by the HAL are defined inside of the `Idt::register_handlers` method. I did this initially because they're only referenced inside that method. However, this has the unfortunate side effect of making the *debug symbols* for ISRs quite unpleasant, because `register_handlers` is generic. In GDB, the symbol name for the PIT timer ISR is ```rust hal_x86_64::interrupt::::register_handlers::pit_timer_isr ```` For an example, consider [this stack trace][1], which is horrible. So, if you want to set a breakpoint on that ISR, you have to type all of that, and also remember that this is the symbol name for the ISR, and not something normal. This is annoying. Therefore, this commit moves the ISR definitions out of this method and into a top-level `hal_x86_64::interrupt::isr` module. Now, the names of these symbols should be more like this: ```rust hal_x86_64::interrupt::isr::pit_timer ``` Which seems a *lot* less unpleasant. There should be no actual functional change here besides getting nicer debug symbols. [1]: https://github.com/hawkw/mycelium/commit/c4183db5e6dcf55b3474790d298edfd842204788#commitcomment-150799709 --- hal-x86_64/src/interrupt.rs | 469 +++++++++++++++++++----------------- 1 file changed, 243 insertions(+), 226 deletions(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index 23350ed8..a4eaf012 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -423,255 +423,54 @@ impl hal_core::interrupt::Control for Idt { where H: Handlers, { - macro_rules! gen_code_faults { - ($self:ident, $h:ty, $($vector:path => fn $name:ident($($rest:tt)+),)+) => { - $( - gen_code_faults! {@ $name($($rest)+); } - $self.register_isr($vector, $name::<$h> as *const ()); - )+ - }; - (@ $name:ident($kind:literal);) => { - extern "x86-interrupt" fn $name>(mut registers: Registers) { - let code = CodeFault { - error_code: None, - kind: $kind, - }; - H::code_fault(Context { registers: &mut registers, code }); - } - }; - (@ $name:ident($kind:literal, code);) => { - extern "x86-interrupt" fn $name>( - mut registers: Registers, - code: u64, - ) { - let code = CodeFault { - error_code: Some(&code), - kind: $kind, - }; - H::code_fault(Context { registers: &mut registers, code }); - } - }; - } - let span = tracing::debug_span!("Idt::register_handlers"); let _enter = span.enter(); - extern "x86-interrupt" fn page_fault_isr>( - mut registers: Registers, - code: PageFaultCode, - ) { - H::page_fault(Context { - registers: &mut registers, - code, - }); - } - - extern "x86-interrupt" fn double_fault_isr>( - mut registers: Registers, - code: u64, - ) { - H::double_fault(Context { - registers: &mut registers, - code, - }); - } - - extern "x86-interrupt" fn pit_timer_isr>(_regs: Registers) { - if crate::time::Pit::handle_interrupt() { - H::timer_tick() - } - unsafe { - INTERRUPT_CONTROLLER - .get_unchecked() - .end_isa_irq(IsaInterrupt::PitTimer); - } - } - - extern "x86-interrupt" fn apic_timer_isr>(_regs: Registers) { - H::timer_tick(); - unsafe { - match INTERRUPT_CONTROLLER.get_unchecked().model { - InterruptModel::Pic(_) => unreachable!(), - InterruptModel::Apic { ref local, .. } => local.end_interrupt(), - } - } - } - - extern "x86-interrupt" fn keyboard_isr>(_regs: Registers) { - // 0x60 is a magic PC/AT number. - static PORT: cpu::Port = cpu::Port::at(0x60); - // load-bearing read - if we don't read from the keyboard controller it won't - // send another interrupt on later keystrokes. - let scancode = unsafe { PORT.readb() }; - H::ps2_keyboard(scancode); - unsafe { - INTERRUPT_CONTROLLER - .get_unchecked() - .end_isa_irq(IsaInterrupt::Ps2Keyboard); - } - } - - extern "x86-interrupt" fn test_isr>(mut registers: Registers) { - H::test_interrupt(Context { - registers: &mut registers, - code: (), - }); - } - - extern "x86-interrupt" fn invalid_tss_isr>( - mut registers: Registers, - code: u64, - ) { - unsafe { - // Safety: An invalid TSS exception is always an oops. Since we're - // not coming back from this, it's okay to forcefully unlock the - // tracing outputs. - force_unlock_tracing(); - } - let selector = SelectorErrorCode(code as u16); - tracing::error!(?selector, "invalid task-state segment!"); - - let msg = selector.named("task-state segment (TSS)"); - let code = CodeFault { - error_code: Some(&msg), - kind: "Invalid TSS (0xA)", - }; - H::code_fault(Context { - registers: &mut registers, - code, - }); - } - - extern "x86-interrupt" fn segment_not_present_isr>( - mut registers: Registers, - code: u64, - ) { - unsafe { - // Safety: An segment not present exception is always an oops. - // Since we're not coming back from this, it's okay to - // forcefully unlock the tracing outputs. - force_unlock_tracing(); - } - let selector = SelectorErrorCode(code as u16); - tracing::error!(?selector, "a segment was not present!"); - - let msg = selector.named("stack segment"); - let code = CodeFault { - error_code: Some(&msg), - kind: "Segment Not Present (0xB)", - }; - H::code_fault(Context { - registers: &mut registers, - code, - }); - } - - extern "x86-interrupt" fn stack_segment_isr>( - mut registers: Registers, - code: u64, - ) { - unsafe { - // Safety: An stack-segment fault exeption is always an oops. - // Since we're not coming back from this, it's okay to - // forcefully unlock the tracing outputs. - force_unlock_tracing(); - } - let selector = SelectorErrorCode(code as u16); - tracing::error!(?selector, "a stack-segment fault is happening"); - - let msg = selector.named("stack segment"); - let code = CodeFault { - error_code: Some(&msg), - kind: "Stack-Segment Fault (0xC)", - }; - H::code_fault(Context { - registers: &mut registers, - code, - }); - } - - extern "x86-interrupt" fn gpf_isr>( - mut registers: Registers, - code: u64, - ) { - unsafe { - // Safety: A general protection fault is (currently) always an - // oops. Since we're not coming back from this, it's okay to - // forcefully unlock the tracing outputs. - // - // TODO(eliza): in the future, if we allow the kernel to - // recover from general protection faults in user mode programs, - // rather than treating them as invariably fatal, we should - // probably not always do this. Instead, we should just handle - // the user-mode GPF non-fatally, and only unlock the tracing - // stuff if we know we're going to do a kernel oops... - force_unlock_tracing(); - } - - let segment = if code > 0 { - Some(SelectorErrorCode(code as u16)) - } else { - None - }; - - tracing::error!(?segment, "lmao, a general protection fault is happening"); - let error_code = segment.map(|seg| seg.named("selector")); - let code = CodeFault { - error_code: error_code.as_ref().map(|code| code as &dyn fmt::Display), - kind: "General Protection Fault (0xD)", - }; - H::code_fault(Context { - registers: &mut registers, - code, - }); - } - - extern "x86-interrupt" fn spurious_isr() { - // TODO(eliza): do we need to actually do something here? - } - // === exceptions === // these exceptions are mapped to the HAL `Handlers` trait's "code // fault" handler, and indicate that the code that was executing did a // Bad Thing - gen_code_faults! { - self, H, - Self::DIVIDE_BY_ZERO => fn div_0_isr("Divide-By-Zero (0x0)"), - Self::OVERFLOW => fn overflow_isr("Overflow (0x4)"), - Self::BOUND_RANGE_EXCEEDED => fn br_isr("Bound Range Exceeded (0x5)"), - Self::INVALID_OPCODE => fn ud_isr("Invalid Opcode (0x6)"), - Self::DEVICE_NOT_AVAILABLE => fn no_fpu_isr("Device (FPU) Not Available (0x7)"), - Self::ALIGNMENT_CHECK => fn alignment_check_isr("Alignment Check (0x11)", code), - Self::SIMD_FLOATING_POINT => fn simd_fp_exn_isr("SIMD Floating-Point Exception (0x13)"), - Self::X87_FPU_EXCEPTION => fn x87_exn_isr("x87 Floating-Point Exception (0x10)"), - } + self.register_isr(Self::DIVIDE_BY_ZERO, isr::div_0:: as *const ()); + self.register_isr(Self::OVERFLOW, isr::overflow:: as *const ()); + self.register_isr(Self::BOUND_RANGE_EXCEEDED, isr::br:: as *const ()); + self.register_isr(Self::INVALID_OPCODE, isr::ud:: as *const ()); + self.register_isr(Self::DEVICE_NOT_AVAILABLE, isr::no_fpu:: as *const ()); + self.register_isr( + Self::ALIGNMENT_CHECK, + isr::alignment_check:: as *const (), + ); + self.register_isr( + Self::SIMD_FLOATING_POINT, + isr::simd_fp_exn:: as *const (), + ); + self.register_isr(Self::X87_FPU_EXCEPTION, isr::x87_exn:: as *const ()); // other exceptions, not mapped to the "code fault" handler - self.register_isr(Self::PAGE_FAULT, page_fault_isr:: as *const ()); - self.register_isr(Self::INVALID_TSS, invalid_tss_isr:: as *const ()); + self.register_isr(Self::PAGE_FAULT, isr::page_fault:: as *const ()); + self.register_isr(Self::INVALID_TSS, isr::invalid_tss:: as *const ()); self.register_isr( Self::SEGMENT_NOT_PRESENT, - segment_not_present_isr:: as *const (), + isr::segment_not_present:: as *const (), ); self.register_isr( Self::STACK_SEGMENT_FAULT, - stack_segment_isr:: as *const (), + isr::stack_segment:: as *const (), ); - self.register_isr(Self::GENERAL_PROTECTION_FAULT, gpf_isr:: as *const ()); - self.register_isr(Self::DOUBLE_FAULT, double_fault_isr:: as *const ()); + self.register_isr(Self::GENERAL_PROTECTION_FAULT, isr::gpf:: as *const ()); + self.register_isr(Self::DOUBLE_FAULT, isr::double_fault:: as *const ()); // === hardware interrupts === // ISA standard hardware interrupts mapped on both the PICs and IO APIC // interrupt models. - self.register_isa_isr(IsaInterrupt::PitTimer, pit_timer_isr:: as *const ()); - self.register_isa_isr(IsaInterrupt::Ps2Keyboard, keyboard_isr:: as *const ()); + self.register_isa_isr(IsaInterrupt::PitTimer, isr::pit_timer:: as *const ()); + self.register_isa_isr(IsaInterrupt::Ps2Keyboard, isr::keyboard:: as *const ()); // local APIC specific hardware interrupts - self.register_isr(Self::LOCAL_APIC_SPURIOUS, spurious_isr as *const ()); - self.register_isr(Self::LOCAL_APIC_TIMER, apic_timer_isr:: as *const ()); + self.register_isr(Self::LOCAL_APIC_SPURIOUS, isr::spurious as *const ()); + self.register_isr(Self::LOCAL_APIC_TIMER, isr::apic_timer:: as *const ()); // vector 69 (nice) is reserved by the HAL for testing the IDT. - self.register_isr(69, test_isr:: as *const ()); + self.register_isr(69, isr::test:: as *const ()); Ok(()) } @@ -772,6 +571,224 @@ impl fmt::Display for NamedSelectorErrorCode { } } +mod isr { + use super::*; + + macro_rules! gen_code_faults { + ($(fn $name:ident($($rest:tt)+),)+) => { + $( + gen_code_faults! {@ $name($($rest)+); } + )+ + }; + (@ $name:ident($kind:literal);) => { + pub(super) extern "x86-interrupt" fn $name>(mut registers: Registers) { + let code = CodeFault { + error_code: None, + kind: $kind, + }; + H::code_fault(Context { registers: &mut registers, code }); + } + }; + (@ $name:ident($kind:literal, code);) => { + pub(super) extern "x86-interrupt" fn $name>( + mut registers: Registers, + code: u64, + ) { + let code = CodeFault { + error_code: Some(&code), + kind: $kind, + }; + H::code_fault(Context { registers: &mut registers, code }); + } + }; + } + + gen_code_faults! { + fn div_0("Divide-By-Zero (0x0)"), + fn overflow("Overflow (0x4)"), + fn br("Bound Range Exceeded (0x5)"), + fn ud("Invalid Opcode (0x6)"), + fn no_fpu("Device (FPU) Not Available (0x7)"), + fn alignment_check("Alignment Check (0x11)", code), + fn simd_fp_exn("SIMD Floating-Point Exception (0x13)"), + fn x87_exn("x87 Floating-Point Exception (0x10)"), + } + + pub(super) extern "x86-interrupt" fn page_fault>( + mut registers: Registers, + code: PageFaultCode, + ) { + H::page_fault(Context { + registers: &mut registers, + code, + }); + } + + pub(super) extern "x86-interrupt" fn double_fault>( + mut registers: Registers, + code: u64, + ) { + H::double_fault(Context { + registers: &mut registers, + code, + }); + } + + pub(super) extern "x86-interrupt" fn pit_timer>(_regs: Registers) { + if crate::time::Pit::handle_interrupt() { + H::timer_tick() + } + unsafe { + INTERRUPT_CONTROLLER + .get_unchecked() + .end_isa_irq(IsaInterrupt::PitTimer); + } + } + + pub(super) extern "x86-interrupt" fn apic_timer>(_regs: Registers) { + H::timer_tick(); + unsafe { + match INTERRUPT_CONTROLLER.get_unchecked().model { + InterruptModel::Pic(_) => unreachable!(), + InterruptModel::Apic { ref local, .. } => local.end_interrupt(), + } + } + } + + pub(super) extern "x86-interrupt" fn keyboard>(_regs: Registers) { + // 0x60 is a magic PC/AT number. + static PORT: cpu::Port = cpu::Port::at(0x60); + // load-bearing read - if we don't read from the keyboard controller it won't + // send another interrupt on later keystrokes. + let scancode = unsafe { PORT.readb() }; + H::ps2_keyboard(scancode); + unsafe { + INTERRUPT_CONTROLLER + .get_unchecked() + .end_isa_irq(IsaInterrupt::Ps2Keyboard); + } + } + + pub(super) extern "x86-interrupt" fn test>(mut registers: Registers) { + H::test_interrupt(Context { + registers: &mut registers, + code: (), + }); + } + + pub(super) extern "x86-interrupt" fn invalid_tss>( + mut registers: Registers, + code: u64, + ) { + unsafe { + // Safety: An invalid TSS exception is always an oops. Since we're + // not coming back from this, it's okay to forcefully unlock the + // tracing outputs. + force_unlock_tracing(); + } + let selector = SelectorErrorCode(code as u16); + tracing::error!(?selector, "invalid task-state segment!"); + + let msg = selector.named("task-state segment (TSS)"); + let code = CodeFault { + error_code: Some(&msg), + kind: "Invalid TSS (0xA)", + }; + H::code_fault(Context { + registers: &mut registers, + code, + }); + } + + pub(super) extern "x86-interrupt" fn segment_not_present>( + mut registers: Registers, + code: u64, + ) { + unsafe { + // Safety: An segment not present exception is always an oops. + // Since we're not coming back from this, it's okay to + // forcefully unlock the tracing outputs. + force_unlock_tracing(); + } + let selector = SelectorErrorCode(code as u16); + tracing::error!(?selector, "a segment was not present!"); + + let msg = selector.named("stack segment"); + let code = CodeFault { + error_code: Some(&msg), + kind: "Segment Not Present (0xB)", + }; + H::code_fault(Context { + registers: &mut registers, + code, + }); + } + + pub(super) extern "x86-interrupt" fn stack_segment>( + mut registers: Registers, + code: u64, + ) { + unsafe { + // Safety: An stack-segment fault exeption is always an oops. + // Since we're not coming back from this, it's okay to + // forcefully unlock the tracing outputs. + force_unlock_tracing(); + } + let selector = SelectorErrorCode(code as u16); + tracing::error!(?selector, "a stack-segment fault is happening"); + + let msg = selector.named("stack segment"); + let code = CodeFault { + error_code: Some(&msg), + kind: "Stack-Segment Fault (0xC)", + }; + H::code_fault(Context { + registers: &mut registers, + code, + }); + } + + pub(super) extern "x86-interrupt" fn gpf>( + mut registers: Registers, + code: u64, + ) { + unsafe { + // Safety: A general protection fault is (currently) always an + // oops. Since we're not coming back from this, it's okay to + // forcefully unlock the tracing outputs. + // + // TODO(eliza): in the future, if we allow the kernel to + // recover from general protection faults in user mode programs, + // rather than treating them as invariably fatal, we should + // probably not always do this. Instead, we should just handle + // the user-mode GPF non-fatally, and only unlock the tracing + // stuff if we know we're going to do a kernel oops... + force_unlock_tracing(); + } + + let segment = if code > 0 { + Some(SelectorErrorCode(code as u16)) + } else { + None + }; + + tracing::error!(?segment, "lmao, a general protection fault is happening"); + let error_code = segment.map(|seg| seg.named("selector")); + let code = CodeFault { + error_code: error_code.as_ref().map(|code| code as &dyn fmt::Display), + kind: "General Protection Fault (0xD)", + }; + H::code_fault(Context { + registers: &mut registers, + code, + }); + } + + pub(super) extern "x86-interrupt" fn spurious() { + // TODO(eliza): do we need to actually do something here? + } +} + #[cfg(test)] mod tests { use super::*;