From d93479bb6ab4f6a8e7251b67b2e31c1d37838c47 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 10:44:04 -0800 Subject: [PATCH 01/10] bad fix for -machine accel=kvm hang --- hal-x86_64/src/interrupt.rs | 42 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index 5bb7a521..d70d551e 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -169,7 +169,7 @@ impl Controller { pics.set_irq_address(Idt::PIC_BIG_START as u8, Idt::PIC_LITTLE_START as u8); } - let model = match acpi { + let controller = match acpi { Some(acpi::InterruptModel::Apic(apic_info)) => { tracing::info!("detected APIC interrupt model"); @@ -198,21 +198,31 @@ impl Controller { // redirection entries. io.map_isa_irqs(Idt::IOAPIC_START as u8); - // unmask the PIT timer vector --- we'll need this for calibrating - // the local APIC timer... - io.set_masked(IoApic::PIT_TIMER_IRQ, false); - - // unmask the PS/2 keyboard interrupt as well. - io.set_masked(IoApic::PS2_KEYBOARD_IRQ, false); - // enable the local APIC let local = LocalApic::new(&mut pagectrl, frame_alloc); local.enable(Idt::LOCAL_APIC_SPURIOUS as u8); - InterruptModel::Apic { + let model = InterruptModel::Apic { local, io: Mutex::new_with_raw_mutex(io, Spinlock::new()), - } + }; + + tracing::trace!(interrupt_model = ?model); + let controller = INTERRUPT_CONTROLLER.init(Self { model }); + + // GOD THIS SUCKS SO BAD + let InterruptModel::Apic { ref io, .. } = controller.model else { + unreachable!("lol") + }; + let mut io = io.lock(); + + // unmask the PIT timer vector --- we'll need this for calibrating + // the local APIC timer... + io.set_masked(IoApic::PIT_TIMER_IRQ, false); + + // unmask the PS/2 keyboard interrupt as well. + io.set_masked(IoApic::PS2_KEYBOARD_IRQ, false); + controller } model => { if model.is_none() { @@ -229,17 +239,13 @@ impl Controller { // clear for you, the reader, that at this point they are definitely intentionally enabled. pics.enable(); } - InterruptModel::Pic(Mutex::new_with_raw_mutex(pics, Spinlock::new())) + INTERRUPT_CONTROLLER.init(Self { + model: InterruptModel::Pic(Mutex::new_with_raw_mutex(pics, Spinlock::new())), + }) } }; - tracing::trace!(interrupt_model = ?model); - - let controller = INTERRUPT_CONTROLLER.init(Self { model }); - // `sti` may not be called until the interrupt controller static is - // fully initialized, as an interrupt that occurs before it is - // initialized may attempt to access the static to finish the interrupt! - core::sync::atomic::fence(core::sync::atomic::Ordering::SeqCst); + tracing::trace!("sti"); unsafe { crate::cpu::intrinsics::sti(); } From 9aa109f0e9230208531bff082b3750e1976e231d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 13:15:41 -0800 Subject: [PATCH 02/10] make I/O APIC ISA interrupt mapping not dumb --- hal-x86_64/src/interrupt.rs | 163 ++++++++++++++-------- hal-x86_64/src/interrupt/apic.rs | 2 +- hal-x86_64/src/interrupt/apic/io.rs | 205 +++++++++++++++++++++++++--- hal-x86_64/src/interrupt/idt.rs | 61 ++------- hal-x86_64/src/interrupt/pic.rs | 47 ++++++- 5 files changed, 351 insertions(+), 127 deletions(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index d70d551e..e99fd028 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -1,4 +1,4 @@ -use crate::{cpu, mm, segment, time, PAddr, VAddr}; +use crate::{cpu, mm, segment, time, VAddr}; use core::{arch::asm, marker::PhantomData, time::Duration}; use hal_core::interrupt::Control; use hal_core::interrupt::{ctx, Handlers}; @@ -15,7 +15,7 @@ pub mod apic; pub mod idt; pub mod pic; -use self::apic::{IoApic, LocalApic}; +use self::apic::{IoApicSet, LocalApic}; pub use idt::Idt; pub use pic::CascadedPic; @@ -69,10 +69,7 @@ enum InterruptModel { /// [apics]: apic Apic { local: apic::LocalApic, - // TODO(eliza): allow further configuration of the I/O APIC (e.g. - // masking/unmasking stuff...) - #[allow(dead_code)] - io: Mutex, + io: apic::IoApicSet, }, } @@ -134,6 +131,67 @@ pub struct Registers { static IDT: Mutex = Mutex::new_with_raw_mutex(idt::Idt::new(), Spinlock::new()); static INTERRUPT_CONTROLLER: InitOnce = InitOnce::uninitialized(); +pub enum MaskError { + NotHwIrq, +} + +/// ISA interrupt vectors +/// +/// See: https://wiki.osdev.org/Interrupts#General_IBM-PC_Compatible_Interrupt_Information +#[derive(Copy, Clone, Debug)] +#[repr(u8)] +pub enum IsaInterrupt { + /// Programmable Interval Timer (PIT) timer interrupt. + PitTimer = 0, + /// PS/2 keyboard controller interrupt. + Ps2Keyboard = 1, + // IRQ 2 is reserved for the PIC cascade interrupt and isn't user accessible! + /// COM2 / COM4 serial port interrupt. + Com2 = 3, + /// COM1 / COM3 serial port interrupt. + Com1 = 4, + /// LPT2 parallel port interrupt. + Lpt2 = 5, + /// Floppy disk + Floppy = 6, + /// LPT1 parallel port interrupt, or spurious. + Lpt1 = 7, + /// CMOS real-time clock. + CmosRtc = 8, + /// Free for peripherals/SCSI/NIC + Periph1 = 9, + Periph2 = 10, + Periph3 = 11, + /// PS/2 Mouse + Ps2Mouse = 12, + /// FPU + Fpu = 13, + /// Primary ATA hard disk + AtaPrimary = 14, + /// Secondary ATA hard disk + AtaSecondary = 15, +} + +impl IsaInterrupt { + pub const ALL: [IsaInterrupt; 15] = [ + IsaInterrupt::PitTimer, + IsaInterrupt::Ps2Keyboard, + IsaInterrupt::Com2, + IsaInterrupt::Com1, + IsaInterrupt::Lpt2, + IsaInterrupt::Floppy, + IsaInterrupt::Lpt1, + IsaInterrupt::CmosRtc, + IsaInterrupt::Periph1, + IsaInterrupt::Periph2, + IsaInterrupt::Periph3, + IsaInterrupt::Ps2Mouse, + IsaInterrupt::Fpu, + IsaInterrupt::AtaPrimary, + IsaInterrupt::AtaSecondary, + ]; +} + impl Controller { // const DEFAULT_IOAPIC_BASE_PADDR: u64 = 0xFEC00000; @@ -152,6 +210,31 @@ impl Controller { } } + pub fn mask_isa_irq(&self, irq: IsaInterrupt) { + match self.model { + InterruptModel::Pic(ref pics) => pics.lock().mask(irq), + InterruptModel::Apic { ref io, .. } => io.set_isa_masked(irq, true), + } + } + + pub fn unmask_isa_irq(&self, irq: IsaInterrupt) { + match self.model { + InterruptModel::Pic(ref pics) => pics.lock().unmask(irq), + InterruptModel::Apic { ref io, .. } => io.set_isa_masked(irq, false), + } + } + + /// # Safety + /// + /// Calling this when there isn't actually an ISA interrupt pending can do + /// arbitrary bad things (which I think is basically just faulting the CPU). + pub unsafe fn end_isa_irq(&self, irq: IsaInterrupt) { + match self.model { + InterruptModel::Pic(ref pics) => pics.lock().end_interrupt(irq), + InterruptModel::Apic { ref local, .. } => local.end_interrupt(), + } + } + pub fn enable_hardware_interrupts( acpi: Option<&acpi::InterruptModel>, frame_alloc: &impl hal_core::mem::page::Alloc, @@ -181,48 +264,17 @@ impl Controller { } tracing::info!("disabled 8259 PICs"); - // configure the I/O APIC - let mut io = { - // TODO(eliza): consider actually using other I/O APICs? do - // we need them for anything?? - tracing::trace!(?apic_info.io_apics, "found {} IO APICs", apic_info.io_apics.len()); - - let io_apic = &apic_info.io_apics[0]; - let addr = PAddr::from_u64(io_apic.address as u64); - - tracing::debug!(ioapic.paddr = ?addr, "IOAPIC"); - IoApic::new(addr, &mut pagectrl, frame_alloc) - }; - - // map the standard ISA hardware interrupts to I/O APIC - // redirection entries. - io.map_isa_irqs(Idt::IOAPIC_START as u8); + // configure the I/O APIC(s) + let io = IoApicSet::new(apic_info, frame_alloc, &mut pagectrl, Idt::ISA_BASE as u8); // enable the local APIC let local = LocalApic::new(&mut pagectrl, frame_alloc); local.enable(Idt::LOCAL_APIC_SPURIOUS as u8); - let model = InterruptModel::Apic { - local, - io: Mutex::new_with_raw_mutex(io, Spinlock::new()), - }; + let model = InterruptModel::Apic { local, io }; tracing::trace!(interrupt_model = ?model); - let controller = INTERRUPT_CONTROLLER.init(Self { model }); - - // GOD THIS SUCKS SO BAD - let InterruptModel::Apic { ref io, .. } = controller.model else { - unreachable!("lol") - }; - let mut io = io.lock(); - - // unmask the PIT timer vector --- we'll need this for calibrating - // the local APIC timer... - io.set_masked(IoApic::PIT_TIMER_IRQ, false); - - // unmask the PS/2 keyboard interrupt as well. - io.set_masked(IoApic::PS2_KEYBOARD_IRQ, false); - controller + INTERRUPT_CONTROLLER.init(Self { model }) } model => { if model.is_none() { @@ -250,6 +302,8 @@ impl Controller { crate::cpu::intrinsics::sti(); } + controller.unmask_isa_irq(IsaInterrupt::PitTimer); + controller.unmask_isa_irq(IsaInterrupt::Ps2Keyboard); controller } @@ -411,12 +465,9 @@ impl hal_core::interrupt::Control for Idt { tracing::trace!("PIT sleep completed"); } unsafe { - match INTERRUPT_CONTROLLER.get_unchecked().model { - InterruptModel::Pic(ref pics) => { - pics.lock().end_interrupt(Idt::PIC_PIT_TIMER as u8) - } - InterruptModel::Apic { ref local, .. } => local.end_interrupt(), - } + INTERRUPT_CONTROLLER + .get_unchecked() + .end_isa_irq(IsaInterrupt::PitTimer); } } @@ -438,12 +489,9 @@ impl hal_core::interrupt::Control for Idt { let scancode = unsafe { PORT.readb() }; H::ps2_keyboard(scancode); unsafe { - match INTERRUPT_CONTROLLER.get_unchecked().model { - InterruptModel::Pic(ref pics) => { - pics.lock().end_interrupt(Idt::PIC_PS2_KEYBOARD as u8) - } - InterruptModel::Apic { ref local, .. } => local.end_interrupt(), - } + INTERRUPT_CONTROLLER + .get_unchecked() + .end_isa_irq(IsaInterrupt::Ps2Keyboard); } } @@ -597,11 +645,10 @@ impl hal_core::interrupt::Control for Idt { // === hardware interrupts === // ISA standard hardware interrupts mapped on both the PICs and IO APIC // interrupt models. - self.register_isr(Self::PIC_PIT_TIMER, pit_timer_isr:: as *const ()); - self.register_isr(Self::IOAPIC_PIT_TIMER, pit_timer_isr:: as *const ()); - self.register_isr(Self::PIC_PS2_KEYBOARD, keyboard_isr:: as *const ()); - self.register_isr(Self::IOAPIC_PS2_KEYBOARD, keyboard_isr:: as *const ()); - // local APIC specific hardware itnerrupts + self.register_isa_isr(IsaInterrupt::PitTimer, pit_timer_isr:: as *const ()); + self.register_isa_isr(IsaInterrupt::Ps2Keyboard, keyboard_isr:: 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 ()); diff --git a/hal-x86_64/src/interrupt/apic.rs b/hal-x86_64/src/interrupt/apic.rs index 54687809..3c15b998 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 local; -pub use io::IoApic; +pub use io::{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/io.rs index 3c1b28b9..4a947c32 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -1,12 +1,22 @@ use super::{PinPolarity, TriggerMode}; use crate::{ cpu::FeatureNotSupported, + interrupt::IsaInterrupt, mm::{self, page, size::Size4Kb, PhysPage, VirtPage}, }; use hal_core::PAddr; -use mycelium_util::bits::{bitfield, enum_from_bits}; +use mycelium_util::{ + bits::{bitfield, enum_from_bits}, + sync::blocking::Mutex, +}; use volatile::Volatile; +#[derive(Debug)] +pub struct IoApicSet { + ioapics: alloc::vec::Vec>, + isa_map: [IsaOverride; 16], +} + #[derive(Debug)] #[must_use] pub struct IoApic { @@ -81,11 +91,184 @@ struct MmioRegisters { data: u32, } +#[derive(Copy, Clone, Debug)] +struct IsaOverride { + apic: u8, + vec: u8, +} + +// === impl IoApicSet === + +impl IoApicSet { + pub fn new( + madt: &acpi::platform::interrupt::Apic, + frame_alloc: &impl hal_core::mem::page::Alloc, + pagectrl: &mut crate::mm::PageCtrl, + isa_base: u8, + ) -> Self { + // The ACPI Multiple APIC Descriptor Table (MADT) tells us where to find + // the I/O APICs, as well as information about how the ISA standard + // interrupts are routed to I/O APIC inputs on this system. + // + // See: https://wiki.osdev.org/MADT + + // There may be multiple I/O APICs in the system, so we'll collect them + // all into a `Vec`. We're also going to build a table of how the ISA + // standard interrupts are mapped across those I/O APICs, so we can look + // up which one a particular interrupt lives on. + let n_ioapics = madt.io_apics.len(); + tracing::trace!(?madt.io_apics, "found {n_ioapics} IO APICs", ); + assert_ne!( + n_ioapics, 0, + "why would a computer have the APIC interrupt model but not \ + have any IO APICs???" + ); + let mut this = IoApicSet { + ioapics: alloc::vec::Vec::with_capacity(n_ioapics), + isa_map: [IsaOverride { apic: 0, vec: 0 }; 16], + }; + + for (n, ioapic) in madt.io_apics.iter().enumerate() { + let addr = PAddr::from_u64(ioapic.address as u64); + tracing::debug!(ioapic.paddr = ?addr, "IOAPIC {n}"); + this.ioapics + .push(Mutex::new(IoApic::new(addr, pagectrl, frame_alloc))); + } + + // Okay, so here's where it gets ~*weird*~. + // + // On the Platonic Ideal Normal Computer, the ISA PC interrupts would be + // mapped to I/O APIC input pins by number, so ISA IRQ 0 would go to pin + // 0 on I/O APIC 0, and so on. + // + // However, motherboard manufacturers can do whatever they like when + // routing these interrupts, so they could go to different pins, + // potentially on different I/O APICs (if the system has more than one). + // Also, some of these interrupts might have different pin polarity or + // edge/level-triggered-iness than what we might expect. + // + // Fortunately, ACPI is here to help (statements dreamed up by the + // utterly deranged). + // + // The MADT includes a list of "interrupt source overrides" that + // describe any ISA interrupts that are not mapped to I/O APIC pins in + // numeric order. Each entry in the interrupt source overrides list will + // contain: + // - an ISA IRQ number (naturally), + // - the "global system interrupt", which is NOT the IDT vector for that + // interrupt but instead the I/O APIC input pin number that the IRQ is + // routed to, + // - the polarity and trigger mode of the interrupt pin, if these are + // different from the default bus trigger mode. + // We can walk this + for irq in IsaInterrupt::ALL { + // Assume the IRQ is mapped to the I/O APIC pin corresponding to + // that ISA IRQ number, and is active-high and edge-triggered. + let mut global_system_interrupt = irq as u8; + let mut polarity = PinPolarity::High; + let mut trigger = TriggerMode::Edge; + // Is there an override for this IRQ? If there is, clobber the + // assumed defaults with "whatever the override says". + if let Some(src_override) = madt + .interrupt_source_overrides + .iter() + .find(|o| o.isa_source == irq as u8) + { + use acpi::platform::interrupt::{ + Polarity as AcpiPolarity, TriggerMode as AcpiTriggerMode, + }; + tracing::debug!( + ?irq, + ?src_override.global_system_interrupt, + ?src_override.polarity, + ?src_override.trigger_mode, + "ISA interrupt {irq:?} is overridden by MADT" + ); + match src_override.polarity { + AcpiPolarity::ActiveHigh => polarity = PinPolarity::High, + AcpiPolarity::ActiveLow => polarity = PinPolarity::Low, + AcpiPolarity::SameAsBus => {} + } + match src_override.trigger_mode { + AcpiTriggerMode::Edge => trigger = TriggerMode::Edge, + AcpiTriggerMode::Level => trigger = TriggerMode::Level, + _ => {} + } + global_system_interrupt = src_override.global_system_interrupt.try_into().expect( + "if this exceeds u8::MAX, what the fuck! \ + that's bigger than the entire IDT...", + ); + } + // Now, scan to find which I/OAPIC this IRQ corresponds to. if the + // system only has one I/O APIC, this will always be 0, but we gotta + // handle systems with more than one. So, we'll this by traversing + // the list of I/O APICs, seeing if the GSI number is less than the + // max number of IRQs handled by that I/O APIC, and if it is, we'll + // stick it in there. If not, keep searching for the next one, + // subtracting the max number of interrupts handled by the I/O APIC + // we just looked at. + let mut entry_idx = global_system_interrupt; + let mut got_him = false; + 'apic_scan: for (apic_idx, apic) in this.ioapics.iter_mut().enumerate() { + let apic = apic.get_mut(); + let max_entries = apic.max_entries(); + if entry_idx > max_entries { + entry_idx -= max_entries; + continue; + } + + // Ladies and gentlemen...we got him! + got_him = true; + tracing::debug!( + ?irq, + ?global_system_interrupt, + ?apic_idx, + ?entry_idx, + "found IOAPIC for ISA interrupt" + ); + let entry = RedirectionEntry::new() + .with(RedirectionEntry::DELIVERY, DeliveryMode::Normal) + .with(RedirectionEntry::POLARITY, polarity) + .with(RedirectionEntry::REMOTE_IRR, false) + .with(RedirectionEntry::TRIGGER, trigger) + .with(RedirectionEntry::MASKED, true) + .with(RedirectionEntry::DESTINATION, 0xff) + .with(RedirectionEntry::VECTOR, isa_base + irq as u8); + apic.set_entry(entry_idx, entry); + this.isa_map[irq as usize] = IsaOverride { + apic: apic_idx as u8, + vec: entry_idx, + }; + break 'apic_scan; + } + + assert!( + got_him, + "somehow, we didn't find an I/O APIC for MADT global system \ + interrupt {global_system_interrupt} (ISA IRQ {irq:?})!\n \ + this probably means the MADT is corrupted somehow, or maybe \ + your motherboard is just super weird? i have no idea what to \ + do in this situation, so i guess i'll die." + ); + } + + this + } + + fn for_isa_irq(&self, irq: IsaInterrupt) -> (&Mutex, u8) { + let isa_override = self.isa_map[irq as usize]; + (&self.ioapics[isa_override.apic as usize], isa_override.vec) + } + + pub fn set_isa_masked(&self, irq: IsaInterrupt, masked: bool) { + let (ioapic, vec) = self.for_isa_irq(irq); + ioapic.with_lock(|ioapic| ioapic.set_masked(vec, masked)); + } +} + // === impl IoApic === impl IoApic { - pub(crate) const PS2_KEYBOARD_IRQ: u8 = 0x1; - pub(crate) const PIT_TIMER_IRQ: u8 = 0x2; const REDIRECTION_ENTRY_BASE: u32 = 0x10; /// Try to construct an `IoApic`. @@ -150,22 +333,6 @@ impl IoApic { Self::try_new(addr, pagectrl, frame_alloc).unwrap() } - /// Map all ISA interrupts starting at `base`. - #[tracing::instrument(level = tracing::Level::DEBUG, skip(self))] - pub fn map_isa_irqs(&mut self, base: u8) { - let flags = RedirectionEntry::new() - .with(RedirectionEntry::DELIVERY, DeliveryMode::Normal) - .with(RedirectionEntry::POLARITY, PinPolarity::High) - .with(RedirectionEntry::REMOTE_IRR, false) - .with(RedirectionEntry::TRIGGER, TriggerMode::Edge) - .with(RedirectionEntry::MASKED, true) - .with(RedirectionEntry::DESTINATION, 0xff); - for irq in 0..16 { - let entry = flags.with(RedirectionEntry::VECTOR, base + irq); - self.set_entry(irq, entry); - } - } - /// Returns the IO APIC's ID. #[must_use] pub fn id(&mut self) -> u8 { diff --git a/hal-x86_64/src/interrupt/idt.rs b/hal-x86_64/src/interrupt/idt.rs index 116a1646..73acdf92 100644 --- a/hal-x86_64/src/interrupt/idt.rs +++ b/hal-x86_64/src/interrupt/idt.rs @@ -1,4 +1,4 @@ -use super::apic::IoApic; +use super::IsaInterrupt; use crate::{cpu, segment}; use mycelium_util::{bits, fmt}; @@ -155,23 +155,10 @@ impl Idt { /// Chosen by fair die roll, guaranteed to be random. pub const DOUBLE_FAULT_IST_OFFSET: usize = 4; - /// PIC Programmable Interval Timer (PIT) interrupt vector mapped by - /// [`Controller::enable_hardware_interrupts`]. - /// - /// Systems which do not use that function to initialize the PICs may - /// map this interrupt to a different IDT vector. - /// - /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const PIC_PIT_TIMER: usize = Self::PIC_BIG_START; + pub const MAX_CPU_EXCEPTION: usize = Self::SECURITY_EXCEPTION; - /// PIC PS/2 interrupt vector mapped by - /// [`Controller::enable_hardware_interrupts`]. - /// - /// Systems which do not use that function to initialize the PICs may - /// map this interrupt to a different IDT vector. - /// - /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const PIC_PS2_KEYBOARD: usize = Self::PIC_BIG_START + 1; + /// Base offset for ISA hardware interrupts. + pub const ISA_BASE: usize = 0x20; /// Local APIC timer interrupt vector mapped by /// [`Controller::enable_hardware_interrupts`]. @@ -198,7 +185,7 @@ impl Idt { /// map this interrupt to a different IDT vector. /// /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const PIC_BIG_START: usize = 0x20; + pub const PIC_BIG_START: usize = Self::ISA_BASE; /// Base of the secondary PIC's interrupt vector region mapped by /// [`Controller::enable_hardware_interrupts`]. @@ -207,48 +194,28 @@ impl Idt { /// map this interrupt to a different IDT vector. /// /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const PIC_LITTLE_START: usize = 0x28; + pub const PIC_LITTLE_START: usize = Self::ISA_BASE + 8; // put the IOAPIC right after the PICs - /// Base of the IOAPIC's interrupt vector region mapped by - /// [`Controller::enable_hardware_interrupts`]. - /// - /// Systems which do not use that function to initialize the IOAPIC may - /// map this interrupt to a different IDT vector. - /// - /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const IOAPIC_START: usize = 0x30; - - /// IOAPIC Programmable Interval Timer (PIT) interrupt vector region mapped - /// by [`Controller::enable_hardware_interrupts`]. - /// - /// Systems which do not use that function to initialize the IOAPIC may - /// map this interrupt to a different IDT vector. - /// - /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const IOAPIC_PIT_TIMER: usize = Self::IOAPIC_START + IoApic::PIT_TIMER_IRQ as usize; - - /// IOAPIC PS/2 keyboard interrupt vector mapped by - /// [`Controller::enable_hardware_interrupts`]. - /// - /// Systems which do not use that function to initialize the IOAPIC may - /// map this interrupt to a different IDT vector. - /// - /// [`Controller::enable_hardware_interrupts`]: super::Controller::enable_hardware_interrupts - pub const IOAPIC_PS2_KEYBOARD: usize = Self::IOAPIC_START + IoApic::PS2_KEYBOARD_IRQ as usize; - pub const fn new() -> Self { Self { descriptors: [Descriptor::null(); Self::NUM_VECTORS], } } + /// Register an interrupt service routine (ISR) for the given ISA standard + /// PC interrupt. + pub fn register_isa_isr(&mut self, irq: IsaInterrupt, isr: *const ()) { + let vector = irq as usize + Self::ISA_BASE; + self.register_isr(vector, isr); + } + pub fn register_isr(&mut self, vector: usize, isr: *const ()) { let descr = self.descriptors[vector].set_handler(isr); if vector == Self::DOUBLE_FAULT { descr.set_ist_offset(Self::DOUBLE_FAULT_IST_OFFSET as u8); } - tracing::debug!(vector, ?isr, ?descr, "set isr"); + tracing::debug!(vector, ?isr, ?descr, "set ISR"); } pub fn load(&'static self) { diff --git a/hal-x86_64/src/interrupt/pic.rs b/hal-x86_64/src/interrupt/pic.rs index da37c42f..00d8349e 100644 --- a/hal-x86_64/src/interrupt/pic.rs +++ b/hal-x86_64/src/interrupt/pic.rs @@ -1,3 +1,4 @@ +use super::IsaInterrupt; use crate::cpu; use hal_core::interrupt::{Handlers, RegistrationError}; @@ -16,6 +17,24 @@ impl Pic { data: cpu::Port::at(data), } } + + /// Mask the provided interrupt number. + unsafe fn mask(&mut self, num: u8) { + debug_assert!(num < 8); + // read the current value of the Interrupt Mask Register (IMR). + let imr = self.data.readb(); + // set the bit corresponding to the interrupt number to 1. + self.data.writeb(imr | (1 << num)); + } + + /// Unmask the provided interrupt number. + unsafe fn unmask(&mut self, num: u8) { + debug_assert!(num < 8); + // read the current value of the Interrupt Mask Register (IMR). + let imr = self.data.readb(); + // clear the bit corresponding to the interrupt number to 1. + self.data.writeb(imr & !(1 << num)); + } } #[derive(Debug)] @@ -50,9 +69,33 @@ impl CascadedPic { } } - pub(crate) fn end_interrupt(&mut self, num: u8) { + pub(crate) fn mask(&mut self, irq: IsaInterrupt) { + let (pic, num) = self.pic_for_irq(irq); + unsafe { + pic.mask(num); + } + } + + pub(crate) fn unmask(&mut self, irq: IsaInterrupt) { + let (pic, num) = self.pic_for_irq(irq); + unsafe { + pic.unmask(num); + } + } + + fn pic_for_irq(&mut self, irq: IsaInterrupt) -> (&mut Pic, u8) { + let num = irq as u8; + if num >= 8 { + (&mut self.sisters.little, num - 8) + } else { + (&mut self.sisters.big, num) + } + } + + pub(crate) fn end_interrupt(&mut self, irq: IsaInterrupt) { const END_INTERRUPT: u8 = 0x20; // from osdev wiki - if num >= self.sisters.little.address && num < self.sisters.little.address + 8 { + let num = irq as u8; + if num >= 8 { unsafe { self.sisters.little.command.writeb(END_INTERRUPT); } From 924a3d2f902792abe6c8eb9de52e89a32c39a6e1 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 13:27:38 -0800 Subject: [PATCH 03/10] white space --- hal-x86_64/src/interrupt/apic/io.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/io.rs index 4a947c32..e0bb98cc 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -150,7 +150,7 @@ impl IoApicSet { // Fortunately, ACPI is here to help (statements dreamed up by the // utterly deranged). // - // The MADT includes a list of "interrupt source overrides" that + // The MADT includes a list of "interrupt source overrides" that // describe any ISA interrupts that are not mapped to I/O APIC pins in // numeric order. Each entry in the interrupt source overrides list will // contain: From 3b720729e396b9b150cdfaa3f1c8d36e7f00de5b Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 13:52:18 -0800 Subject: [PATCH 04/10] commentary & cleanup --- hal-x86_64/src/interrupt.rs | 20 +++++++++++++++++++- hal-x86_64/src/interrupt/apic/io.rs | 23 ++++++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index e99fd028..d2869542 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -297,11 +297,29 @@ impl Controller { } }; - tracing::trace!("sti"); unsafe { crate::cpu::intrinsics::sti(); } + // There's a weird behavior in QEMU where, apparently, when we unmask + // the PIT interrupt, it might fire spuriously *as soon as its + // unmasked*, even if we haven't actually done an `sti`. I don't get + // this and it seems wrong, but it seems to happen occasionally with the + // default QEMU acceleration, and pretty consistently with `-machine + // accel=kvm`, so *maybe* it can also happen on a real computer? + // + // Anyway, because of this, we can't unmask the PIT interrupt until + // after we've actually initialized the interrupt controller static. + // Otherwise, if we unmask it before initializing the static (like we + // used to), the interrupt gets raised immediately, and when the ISR + // tries to actually send an EOI to ack it, it dereferences + // uninitialized memory and the kernel just hangs. So, we wait to do it + // until here. + // + // The fact that this behavior exists at all makes me really, profoundly + // uncomfortable: shouldn't `cli` like, actually do what it's supposed + // to? But, we can just choose to unmask the PIT later and it's fine, I + // guess... controller.unmask_isa_irq(IsaInterrupt::PitTimer); controller.unmask_isa_irq(IsaInterrupt::Ps2Keyboard); controller diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/io.rs index e0bb98cc..f1de4a84 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -160,7 +160,28 @@ impl IoApicSet { // routed to, // - the polarity and trigger mode of the interrupt pin, if these are // different from the default bus trigger mode. - // We can walk this + // + // For each of the 16 ISA interrupt vectors, we'll configure the + // redirection entry in the appropriate I/O APIC and record which I/O + // APIC (and which pin on that I/O APIC) the interrupt is routed to. We + // do this by first checking if the MADT contains an override matching + // that ISA interrupt, using that if so, and if not, falling back to the + // ISA interrupt number. + // + // Yes, this is a big pile of nested loops. But, consider the following: + // + // - the MADT override entries can probably come in any order, if the + // motherboard firmware chooses to be maximally perverse, so we have + // to scan the whole list of them to find out if each ISA interrupt is + // overridden. + // - there are only ever 16 ISA interrupts, so the outer loop iterates + // exactly 16 times; and there can't be *more* overrides than there + // are ISA interrupts (and there's generally substantially fewer). + // similarly, if there's more than 1-8 I/O APICs, you probably have + // some kind of really weird computer and should tell me about it + // because i bet it's awesome. + // so, neither inner loop actually loops that many times. + // - finally, we only do this once on boot, so who cares? for irq in IsaInterrupt::ALL { // Assume the IRQ is mapped to the I/O APIC pin corresponding to // that ISA IRQ number, and is active-high and edge-triggered. From 844336fb2a250d2f0aa42a2ad32783482d12ef0d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 14:09:03 -0800 Subject: [PATCH 05/10] Update io.rs Co-authored-by: iximeow --- hal-x86_64/src/interrupt/apic/io.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/io.rs index f1de4a84..a30ed996 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -220,7 +220,7 @@ impl IoApicSet { that's bigger than the entire IDT...", ); } - // Now, scan to find which I/OAPIC this IRQ corresponds to. if the + // Now, scan to find which I/O APIC this IRQ corresponds to. if the // system only has one I/O APIC, this will always be 0, but we gotta // handle systems with more than one. So, we'll this by traversing // the list of I/O APICs, seeing if the GSI number is less than the From 0d5e8aa944e1721c0c7b7bab5a912f390e4cda0a Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 14:09:53 -0800 Subject: [PATCH 06/10] Update pic.rs Co-authored-by: iximeow --- hal-x86_64/src/interrupt/pic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal-x86_64/src/interrupt/pic.rs b/hal-x86_64/src/interrupt/pic.rs index 00d8349e..e61150a0 100644 --- a/hal-x86_64/src/interrupt/pic.rs +++ b/hal-x86_64/src/interrupt/pic.rs @@ -32,7 +32,7 @@ impl Pic { debug_assert!(num < 8); // read the current value of the Interrupt Mask Register (IMR). let imr = self.data.readb(); - // clear the bit corresponding to the interrupt number to 1. + // clear the bit corresponding to the interrupt number. self.data.writeb(imr & !(1 << num)); } } From f2d5045c08ea3755990253787df1944e2a0fce88 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 20:24:01 -0800 Subject: [PATCH 07/10] TODO: what we're doing is bad & i should feel bad --- hal-x86_64/src/interrupt/apic/io.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/io.rs index a30ed996..06111ffa 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -208,12 +208,22 @@ impl IoApicSet { match src_override.polarity { AcpiPolarity::ActiveHigh => polarity = PinPolarity::High, AcpiPolarity::ActiveLow => polarity = PinPolarity::Low, + // TODO(eliza): if the MADT override entry says that the pin + // polarity is "same as bus", we should probably actually + // make it be the same as the bus, instead of just assuming + // that it's active high. But...just assuming that "same as + // bus" means active high seems to basically work so far... AcpiPolarity::SameAsBus => {} } match src_override.trigger_mode { AcpiTriggerMode::Edge => trigger = TriggerMode::Edge, AcpiTriggerMode::Level => trigger = TriggerMode::Level, - _ => {} + // TODO(eliza): As above, when the MADT says this is "same + // as bus", we should make it be the same as the bus instead + // of going "ITS EDGE TRIGGERED LOL LMAO" which is what + // we're currently doing. But, just like above, this Seems + // To Work? + AcpiTriggerMode::SameAsBus => {} } global_system_interrupt = src_override.global_system_interrupt.try_into().expect( "if this exceeds u8::MAX, what the fuck! \ From e1be6bbd06bcd421cb6dd03a0738d7f8bba14009 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 20:26:40 -0800 Subject: [PATCH 08/10] factor out base redirection entry --- hal-x86_64/src/interrupt/apic/io.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/io.rs index 06111ffa..7dc3e94a 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -182,6 +182,12 @@ impl IoApicSet { // because i bet it's awesome. // so, neither inner loop actually loops that many times. // - finally, we only do this once on boot, so who cares? + + let base_entry = RedirectionEntry::new() + .with(RedirectionEntry::DELIVERY, DeliveryMode::Normal) + .with(RedirectionEntry::REMOTE_IRR, false) + .with(RedirectionEntry::MASKED, true) + .with(RedirectionEntry::DESTINATION, 0xff); for irq in IsaInterrupt::ALL { // Assume the IRQ is mapped to the I/O APIC pin corresponding to // that ISA IRQ number, and is active-high and edge-triggered. @@ -257,13 +263,9 @@ impl IoApicSet { ?entry_idx, "found IOAPIC for ISA interrupt" ); - let entry = RedirectionEntry::new() - .with(RedirectionEntry::DELIVERY, DeliveryMode::Normal) + let entry = base_entry .with(RedirectionEntry::POLARITY, polarity) - .with(RedirectionEntry::REMOTE_IRR, false) .with(RedirectionEntry::TRIGGER, trigger) - .with(RedirectionEntry::MASKED, true) - .with(RedirectionEntry::DESTINATION, 0xff) .with(RedirectionEntry::VECTOR, isa_base + irq as u8); apic.set_entry(entry_idx, entry); this.isa_map[irq as usize] = IsaOverride { From 87b9cdd8c8bcf5597b40a6f88dedc471ac5d273f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 20:27:42 -0800 Subject: [PATCH 09/10] add @iximeow's comment that made me giggle --- hal-x86_64/src/interrupt/apic/io.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hal-x86_64/src/interrupt/apic/io.rs b/hal-x86_64/src/interrupt/apic/io.rs index 7dc3e94a..0a0426df 100644 --- a/hal-x86_64/src/interrupt/apic/io.rs +++ b/hal-x86_64/src/interrupt/apic/io.rs @@ -201,6 +201,8 @@ impl IoApicSet { .iter() .find(|o| o.isa_source == irq as u8) { + // put the defaults through an extruder that maybe messes with + // them. use acpi::platform::interrupt::{ Polarity as AcpiPolarity, TriggerMode as AcpiTriggerMode, }; From 60678fdb61c3c489f440f2e795afe84ad79c74f0 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 28 Dec 2024 20:31:22 -0800 Subject: [PATCH 10/10] blegh rustdoc --- hal-x86_64/src/interrupt.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal-x86_64/src/interrupt.rs b/hal-x86_64/src/interrupt.rs index d2869542..17200169 100644 --- a/hal-x86_64/src/interrupt.rs +++ b/hal-x86_64/src/interrupt.rs @@ -137,7 +137,7 @@ pub enum MaskError { /// ISA interrupt vectors /// -/// See: https://wiki.osdev.org/Interrupts#General_IBM-PC_Compatible_Interrupt_Information +/// See: [the other wiki](https://wiki.osdev.org/Interrupts#General_IBM-PC_Compatible_Interrupt_Information) #[derive(Copy, Clone, Debug)] #[repr(u8)] pub enum IsaInterrupt {