From 7fc77ad01879a83d3bb9d6a3d284fbe1fdc3d7fb Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 30 Dec 2024 09:43:48 -0800 Subject: [PATCH 1/2] refac(hal_x86_64): rename `apic::io` to `ioapic` (#497) I don't like that the module is just called "io", that feels weird. --- hal-x86_64/src/interrupt/apic.rs | 4 ++-- hal-x86_64/src/interrupt/apic/{io.rs => ioapic.rs} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename hal-x86_64/src/interrupt/apic/{io.rs => ioapic.rs} (100%) diff --git a/hal-x86_64/src/interrupt/apic.rs b/hal-x86_64/src/interrupt/apic.rs index 3c15b998..846a5613 100644 --- a/hal-x86_64/src/interrupt/apic.rs +++ b/hal-x86_64/src/interrupt/apic.rs @@ -1,7 +1,7 @@ //! Advanced Programmable Interrupt Controller (APIC). -pub mod io; +pub mod ioapic; pub mod local; -pub use io::{IoApic, IoApicSet}; +pub use ioapic::{IoApic, IoApicSet}; pub use local::LocalApic; use mycelium_util::bits::enum_from_bits; diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/ioapic.rs similarity index 100% rename from hal-x86_64/src/interrupt/apic/io.rs rename to hal-x86_64/src/interrupt/apic/ioapic.rs From 2fb5d216aa99b044623f6111d26278b422d1f677 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 30 Dec 2024 10:02:24 -0800 Subject: [PATCH 2/2] 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::*;