Better distinction between GSI and interrupt vectors

ALSchwalm · ALSchwalm · commit 7b921f6873aa · 2020-12-29T20:09:31.000-06:00
diff --git a/mythril/src/kmain.rs b/mythril/src/kmain.rs
@@ -65,18 +65,17 @@ fn build_vm(
 
     let mut madt = acpi::madt::MADTBuilder::<[_; 8]>::new();
     madt.set_ica(vm::GUEST_LOCAL_APIC_ADDR.as_u64() as u32);
-    madt.add_ics(acpi::madt::Ics::LocalApic {
-        apic_id: 0,
-        apic_uid: 0,
-        flags: acpi::madt::LocalApicFlags::ENABLED,
-    })
-    .expect("Failed to add APIC to MADT");
-    madt.add_ics(acpi::madt::Ics::LocalApic {
-        apic_id: 1,
-        apic_uid: 0,
-        flags: acpi::madt::LocalApicFlags::ENABLED,
-    })
-    .expect("Failed to add APIC to MADT");
+
+    for core in cfg.cpus.iter() {
+        madt.add_ics(acpi::madt::Ics::LocalApic {
+            // TODO(alschwalm): we should assign an actual APIC id here,
+            // instead of using the core id
+            apic_id: core.raw as u8,
+            apic_uid: 0,
+            flags: acpi::madt::LocalApicFlags::ENABLED,
+        })
+        .expect("Failed to add APIC to MADT");
+    }
     madt.add_ics(acpi::madt::Ics::IoApic {
         ioapic_id: 0,
         ioapic_addr: 0xfec00000 as *mut u8,
diff --git a/mythril/src/time.rs b/mythril/src/time.rs
@@ -57,6 +57,27 @@ fn frequency() -> u64 {
     TIME_SRC.frequency()
 }
 
+/// An interrupt to be delivered by a timer
+#[derive(Clone)]
+pub enum TimerInterruptType {
+    /// An interrupt to be delivered to the core that is running the timer.
+    /// For example, when virtualizing the guest local apic timer, the
+    /// generated interrupt is _not_ a guest GSI, but a directly delivered
+    /// interrupt.
+    Direct {
+        /// The interrupt vector to be delivered by the timer
+        vector: u8,
+
+        /// The kind of interrupt to be delivered by the timer
+        kind: vcpu::InjectedInterruptType,
+    },
+
+    /// An interrupt to be delivered to the guest via a GSI.
+    /// For example, any hardware timer external to the core will generate
+    /// a GSI and be routed to a vector through the guest IO APIC
+    GSI(u32),
+}
+
 /// A point in time on the system in terms of the global system `TimeSource`
 ///
 /// An `Instant` can be added/subtracted with a `Duration` to produce an
@@ -123,38 +144,33 @@ enum TimerMode {
 pub struct ReadyTimer {
     duration: Duration,
     mode: TimerMode,
-
-    // The interrupt vector to deliver to the guest when this timer
-    // expires.
-    // TODO: Not all timers represent interrupts to deliver to the guest,
-    // so we will need to make this more abstract.
-    vector: u8,
+    kind: TimerInterruptType,
 }
 
 /// A started one-shot or periodic timer
 pub struct RunningTimer {
     duration: Duration,
     mode: TimerMode,
     started: Instant,
-    vector: u8,
+    kind: TimerInterruptType,
 }
 
 impl ReadyTimer {
     /// Create a new one-shot timer.
-    pub fn one_shot(duration: Duration, vector: u8) -> Self {
+    pub fn one_shot(duration: Duration, kind: TimerInterruptType) -> Self {
         Self {
-            duration: duration,
+            duration,
             mode: TimerMode::OneShot,
-            vector: vector,
+            kind,
         }
     }
 
     /// Create a new periodic timer.
-    pub fn periodic(period: Duration, vector: u8) -> Self {
+    pub fn periodic(period: Duration, kind: TimerInterruptType) -> Self {
         Self {
             duration: period,
             mode: TimerMode::Periodic,
-            vector: vector,
+            kind,
         }
     }
 
@@ -164,7 +180,7 @@ impl ReadyTimer {
             duration: self.duration,
             mode: self.mode,
             started: now(),
-            vector: self.vector,
+            kind: self.kind,
         }
     }
 
@@ -183,7 +199,7 @@ impl RunningTimer {
         ReadyTimer {
             duration: self.duration,
             mode: self.mode,
-            vector: self.vector,
+            kind: self.kind,
         }
     }
 
@@ -273,7 +289,7 @@ impl TimerWheel {
     /// expired and will reset any periodic timers.
     pub fn expire_elapsed_timers(
         &mut self,
-    ) -> Result<vec::Vec<(u8, vcpu::InjectedInterruptType)>> {
+    ) -> Result<vec::Vec<TimerInterruptType>> {
         let mut interrupts = vec![];
         let elapsed_oneshots = self
             .timers
@@ -288,10 +304,7 @@ impl TimerWheel {
             .collect::<vec::Vec<_>>();
 
         for id in elapsed_oneshots {
-            interrupts.push((
-                self.timers[&id].vector,
-                vcpu::InjectedInterruptType::ExternalInterrupt,
-            ));
+            interrupts.push(self.timers[&id].kind.clone());
             self.timers.remove(&id);
         }
 
@@ -300,10 +313,7 @@ impl TimerWheel {
             .iter_mut()
             .filter(|(_, timer)| timer.elapsed() && timer.is_periodic())
         {
-            interrupts.push((
-                timer.vector,
-                vcpu::InjectedInterruptType::ExternalInterrupt,
-            ));
+            interrupts.push(timer.kind.clone());
             timer.reset();
         }
 
@@ -315,8 +325,8 @@ impl TimerWheel {
         let soonest = self
             .timers
             .values()
-            .map(|timer| (timer.elapses_at(), timer.vector))
-            .min();
+            .map(|timer| (timer.elapses_at(), &timer.kind))
+            .min_by(|(time1, _), (time2, _)| time1.cmp(time2));
 
         // TODO: we should only actually reset this if the new time
         // is sooner than the last time we set
@@ -407,19 +417,19 @@ pub fn cancel_timer(id: &TimerId) -> Result<()> {
 /// Set a one shot timer on this core
 pub fn set_oneshot_timer(
     duration: core::time::Duration,
-    vector: u8,
+    kind: TimerInterruptType,
 ) -> TimerId {
     let wheel = unsafe { get_timer_wheel_mut() };
-    let timer = ReadyTimer::one_shot(duration, vector);
+    let timer = ReadyTimer::one_shot(duration, kind);
     wheel.register_timer(timer)
 }
 
 /// Set a periodic timer on this core
 pub fn set_periodic_timer(
     interval: core::time::Duration,
-    vector: u8,
+    kind: TimerInterruptType,
 ) -> TimerId {
     let wheel = unsafe { get_timer_wheel_mut() };
-    let timer = ReadyTimer::periodic(interval, vector);
+    let timer = ReadyTimer::periodic(interval, kind);
     wheel.register_timer(timer)
 }
diff --git a/mythril/src/vcpu.rs b/mythril/src/vcpu.rs
@@ -425,6 +425,21 @@ impl VCpu {
         Ok(())
     }
 
+    pub fn route_interrupt(&mut self, gsi: u32) -> Result<()> {
+        let (destination, vector, kind) =
+            self.vm.read().gsi_destination(gsi)?;
+        if destination == percore::read_core_id() {
+            self.inject_interrupt(vector, kind);
+            Ok(())
+        } else {
+            vm::send_vm_msg_core(
+                vm::VirtualMachineMsg::GuestInterrupt { vector, kind },
+                destination,
+                true,
+            )
+        }
+    }
+
     /// Handle an arbitrary guest VMEXIT.
     ///
     /// This is the rust 'entry' point when a guest exists.
@@ -443,10 +458,19 @@ impl VCpu {
 
         // Always check for expired timers
         unsafe {
-            for (vec, kind) in
+            for timer_event in
                 time::get_timer_wheel_mut().expire_elapsed_timers()?
             {
-                self.inject_interrupt(vec, kind);
+                match timer_event {
+                    time::TimerInterruptType::Direct {
+                        vector, kind, ..
+                    } => {
+                        self.inject_interrupt(vector, kind);
+                    }
+                    time::TimerInterruptType::GSI(gsi) => {
+                        self.route_interrupt(gsi)?;
+                    }
+                }
             }
         }
 
@@ -528,8 +552,6 @@ impl VCpu {
             .map(|serial| (serial.read(), serial.base_port()));
         drop(vm);
 
-        // info!("Received UART keypress on core: {}", crate::percore::read_core_id());
-
         let mut vm = self.vm.write();
         if let Some((key, port)) = serial_info {
             vm.dispatch_event(
@@ -647,12 +669,8 @@ impl VCpu {
 
         for response in responses {
             match response {
-                virtdev::DeviceEventResponse::Interrupt((vector, kind)) => {
-                    // FIXME(alschwalm): for now, only deliver the UART interrupts to
-                    // the BSP core.
-                    if vector != 52 || crate::percore::read_core_id().raw == 0 {
-                        self.inject_interrupt(vector, kind);
-                    }
+                virtdev::DeviceEventResponse::GSI(gsi) => {
+                    self.route_interrupt(gsi)?;
                 }
                 virtdev::DeviceEventResponse::NextConsole => {
                     info!("Received Ctrl-a three times. Switching console to next VM");
diff --git a/mythril/src/virtdev/com.rs b/mythril/src/virtdev/com.rs
@@ -61,14 +61,7 @@ impl EmulatedDevice for Uart8250 {
     fn on_event(&mut self, event: Event) -> Result<()> {
         match event.kind {
             DeviceEvent::HostUartReceived(key) => {
-                // info!("Processing UART keypress on core: {}", crate::percore::read_core_id());
-                event.responses.push(
-                    // IRQ4
-                    DeviceEventResponse::Interrupt((
-                        52,
-                        vcpu::InjectedInterruptType::ExternalInterrupt,
-                    )),
-                );
+                event.responses.push(DeviceEventResponse::GSI(4));
                 if key == 0x01 {
                     // ctrl+a
                     self.ctrl_a_count += 1;
@@ -133,18 +126,11 @@ impl EmulatedDevice for Uart8250 {
                             DeviceEventResponse::GuestUartTransmitted(val),
                         );
 
-                        //FIXME(alschwalm): this should actually either directly
-                        // respond with an interrupt device event _or_ send the
-                        // GuestInterrupt VirtualMachineMsg to the appropriate
-                        // core (according to either the PIC or IOAPIC)
                         if self
                             .interrupt_enable_register
                             .contains(IerFlags::THR_EMPTY_INTERRUPT)
                         {
-                            event.responses.push(
-                                // IRQ4
-                                DeviceEventResponse::Interrupt((52, vcpu::InjectedInterruptType::ExternalInterrupt))
-                            );
+                            event.responses.push(DeviceEventResponse::GSI(4));
                         }
                         self.interrupt_identification_register = 0b10;
                     }
diff --git a/mythril/src/virtdev/mod.rs b/mythril/src/virtdev/mod.rs
@@ -45,7 +45,7 @@ pub enum DeviceEvent<'a> {
 pub enum DeviceEventResponse {
     GuestUartTransmitted(u8),
     NextConsole,
-    Interrupt((u8, vcpu::InjectedInterruptType)),
+    GSI(u32),
 }
 
 pub struct Event<'a> {
diff --git a/mythril/src/virtdev/pit.rs b/mythril/src/virtdev/pit.rs
@@ -225,10 +225,10 @@ impl Pit8254 {
 
                         // Only channel 0 produces timer interrupts
                         if port == PIT_COUNTER_0 {
-                            //FIXME: this value should be determined by the virtual
-                            // PIC/APIC. Currently use the vector linux has for IRQ0
-                            *timer =
-                                Some(time::set_oneshot_timer(duration, 48));
+                            *timer = Some(time::set_oneshot_timer(
+                                duration,
+                                time::TimerInterruptType::GSI(0),
+                            ));
                         }
                     }
 
@@ -241,10 +241,10 @@ impl Pit8254 {
                         *start_time = Some(time::now());
 
                         if port == PIT_COUNTER_0 {
-                            //FIXME: this value should be determined by the virtual
-                            // PIC/APIC. Currently use the vector linux has for IRQ0
-                            *timer =
-                                Some(time::set_periodic_timer(duration, 48));
+                            *timer = Some(time::set_periodic_timer(
+                                duration,
+                                time::TimerInterruptType::GSI(0),
+                            ));
                         }
                     }
                 };
diff --git a/mythril/src/vm.rs b/mythril/src/vm.rs
@@ -426,6 +426,30 @@ impl VirtualMachine {
         dev.write().on_event(event)
     }
 
+    pub fn gsi_destination(
+        &self,
+        gsi: u32,
+    ) -> Result<(percore::CoreId, u8, vcpu::InjectedInterruptType)> {
+        //TODO(alschwalm): For now just route the UART interrupts to the BSP,
+        // but this should ulimately do actual interrupt routing based on the
+        // guest IO APICs. For now just blindly translate GSI to vector based
+        // on this basic formula.
+        let vector = (gsi + 48) as u8;
+        if gsi == 4 {
+            Ok((
+                self.config.bsp_id(),
+                vector,
+                vcpu::InjectedInterruptType::ExternalInterrupt,
+            ))
+        } else {
+            Ok((
+                percore::read_core_id(),
+                vector,
+                vcpu::InjectedInterruptType::ExternalInterrupt,
+            ))
+        }
+    }
+
     fn map_data(
         image: &[u8],
         addr: &GuestPhysAddr,

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ pub enum DeviceEvent<'a> {`
`45`	`45`	`pub enum DeviceEventResponse {`
`46`	`46`	`GuestUartTransmitted(u8),`
`47`	`47`	`NextConsole,`
`48`		`- Interrupt((u8, vcpu::InjectedInterruptType)),`
	`48`	`+ GSI(u32),`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`pub struct Event<'a> {`