x86-64: properly implement ISA IOAPIC redirections

lib/device-api/src/interrupt.rs

@@ -27,12 +27,6 @@ pub enum IpiDeliveryTarget {
     OtherCpus,
 }
 
-// #[derive(Clone, Copy, PartialEq, Eq, Debug)]
-// pub enum IrqNumber {
-//     Private(u32),
-//     Shared(u32),
-// }
-
 #[derive(Default, Clone, Copy, Debug)]
 pub struct IrqOptions {
     pub level: IrqLevel,
@@ -90,6 +84,24 @@ pub struct FixedInterruptTable<const SIZE: usize> {
     entries: [Option<&'static dyn InterruptHandler>; SIZE],
 }
 
+impl IrqLevel {
+    pub fn override_default(self, value: IrqLevel) -> Self {
+        match self {
+            Self::Default => value,
+            _ => self,
+        }
+    }
+}
+
+impl IrqTrigger {
+    pub fn override_default(self, value: IrqTrigger) -> Self {
+        match self {
+            Self::Default => value,
+            _ => self,
+        }
+    }
+}
+
 impl<const SIZE: usize> FixedInterruptTable<SIZE> {
     pub const fn new() -> Self {
         Self {

src/arch/x86_64/apic/ioapic.rs

@@ -30,18 +30,12 @@ const ENTRY_LOW_DESTINATION_LOGICAL: u32 = 1 << 11;
 
 const ENTRY_HIGH_APIC_ID_SHIFT: u32 = 24;
 
-#[derive(Clone, Copy, PartialEq)]
-enum ExternalTriggerMode {
-    Edge,
-    Level,
-}
-
 #[allow(dead_code)]
 #[derive(Clone, Copy)]
 struct IsaRedirection {
     gsi_index: u32,
-    polarity: bool,
-    trigger: ExternalTriggerMode,
+    level: IrqLevel,
+    trigger: IrqTrigger,
 }
 
 struct Regs {
@@ -99,6 +93,8 @@ impl Inner {
         low |= vector;
         // Destination - physical
         low &= !ENTRY_LOW_DESTINATION_LOGICAL;
+        // Clear delivery mode
+        low &= !(0x7 << 8);
 
         // Destination APIC ID
         high &= !(0xFF << ENTRY_HIGH_APIC_ID_SHIFT);
@@ -110,12 +106,12 @@ impl Inner {
         Ok(())
     }
 
-    fn configure_gsi(&mut self, gsi: u32, options: IrqOptions) {
+    fn configure_gsi(&mut self, gsi: u32, level: IrqLevel, trigger: IrqTrigger) {
         assert!(gsi < self.max_gsi);
 
         let mut low = self.regs.read(REG_REDIRECTION_BASE + gsi * 2);
 
-        match options.level {
+        match level {
             IrqLevel::Default => (),
             IrqLevel::ActiveLow => {
                 low |= ENTRY_LOW_POLARITY_LOW;
@@ -125,7 +121,7 @@ impl Inner {
             }
         }
 
-        match options.trigger {
+        match trigger {
             IrqTrigger::Default => (),
             IrqTrigger::Level => {
                 low |= ENTRY_LOW_TRIGGER_LEVEL;
@@ -186,8 +182,24 @@ impl ExternalInterruptController for IoApic {
             table_vector
         );
 
-        let gsi = self.translate_irq(irq);
-        inner.configure_gsi(gsi, options);
+        let (gsi, level, trigger) = match irq {
+            IrqNumber::Isa(irq) => {
+                if let Some(redir) = self.isa_redirections[irq as usize].as_ref() {
+                    // Mapped to a (possibly different) GSI, but also with possibly different options
+                    (
+                        redir.gsi_index,
+                        options.level.override_default(redir.level),
+                        options.trigger.override_default(redir.trigger),
+                    )
+                } else {
+                    // Directly mapped to a GSI
+                    (irq as u32, options.level, options.trigger)
+                }
+            }
+            IrqNumber::Gsi(irq) => (irq as u32, options.level, options.trigger),
+        };
+
+        inner.configure_gsi(gsi, level, trigger);
         inner.map_gsi(gsi, gsi_target_vector, bsp_apic)?;
 
         Ok(())
@@ -214,6 +226,9 @@ impl ExternalInterruptController for IoApic {
 impl IoApic {
     /// Creates an I/O APIC instance from its ACPI definition
     pub fn from_acpi(info: &AcpiApic) -> Result<Self, Error> {
+        if info.io_apics.len() != 1 {
+            todo!();
+        }
         let ioapic = info.io_apics.first().unwrap();
 
         infoln!("I/O APIC at {:#x}", ioapic.address);
@@ -222,14 +237,23 @@ impl IoApic {
 
         for redir in info.interrupt_source_overrides.iter() {
             let index = redir.isa_source as usize;
-            let polarity = match redir.polarity {
-                Polarity::ActiveLow => false,
-                // TODO this may not be correct
-                Polarity::ActiveHigh | Polarity::SameAsBus => true,
-            };
             let trigger = match redir.trigger_mode {
-                TriggerMode::Edge | TriggerMode::SameAsBus => ExternalTriggerMode::Edge,
-                TriggerMode::Level => ExternalTriggerMode::Level,
+                // ISA IRQs are edge-triggered by default
+                TriggerMode::SameAsBus => IrqTrigger::Edge,
+                TriggerMode::Edge => IrqTrigger::Edge,
+                TriggerMode::Level => IrqTrigger::Level,
+            };
+            let level = match redir.polarity {
+                // Level-triggered ISA IRQs are ActiveLow by default
+                Polarity::SameAsBus => {
+                    if trigger == IrqTrigger::Level {
+                        IrqLevel::ActiveLow
+                    } else {
+                        IrqLevel::ActiveHigh
+                    }
+                }
+                Polarity::ActiveLow => IrqLevel::ActiveLow,
+                Polarity::ActiveHigh => IrqLevel::ActiveHigh,
             };
 
             debugln!(
@@ -239,7 +263,7 @@ impl IoApic {
             );
             isa_redirections[index].replace(IsaRedirection {
                 gsi_index: redir.global_system_interrupt,
-                polarity,
+                level,
                 trigger,
             });
         }