From b32d3025f3e7f493695cc1f9e7b5759fd8188943 Mon Sep 17 00:00:00 2001 From: Michael Zhao Date: Mon, 25 May 2020 16:27:08 +0800 Subject: [PATCH] devices: Refactor IOAPIC to cover other architectures IOAPIC, a X86 specific interrupt controller, is referenced by device manager and CPU manager. To work with more architectures, a common type for all architectures is needed. This commit introduces trait InterruptController to provide architecture agnostic functions. Device manager and CPU manager can use it without caring what the underlying device is. Signed-off-by: Michael Zhao --- devices/src/interrupt_controller.rs | 58 ++++++++ devices/src/ioapic.rs | 204 +++++++++++++--------------- devices/src/lib.rs | 1 + vmm/src/cpu.rs | 25 ++-- vmm/src/device_manager.rs | 51 ++++--- vmm/src/interrupt.rs | 10 +- 6 files changed, 202 insertions(+), 147 deletions(-) create mode 100644 devices/src/interrupt_controller.rs diff --git a/devices/src/interrupt_controller.rs b/devices/src/interrupt_controller.rs new file mode 100644 index 000000000..ad31a0f23 --- /dev/null +++ b/devices/src/interrupt_controller.rs @@ -0,0 +1,58 @@ +// Copyright 2020, ARM Limited. +// +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause + +use std::io; +use std::result; + +#[derive(Debug)] +pub enum Error { + /// Invalid destination mode. + InvalidDestinationMode, + /// Invalid trigger mode. + InvalidTriggerMode, + /// Invalid delivery mode. + InvalidDeliveryMode, + /// Failed creating the interrupt source group. + CreateInterruptSourceGroup(io::Error), + /// Failed triggering the interrupt. + TriggerInterrupt(io::Error), + /// Failed masking the interrupt. + MaskInterrupt(io::Error), + /// Failed unmasking the interrupt. + UnmaskInterrupt(io::Error), + /// Failed updating the interrupt. + UpdateInterrupt(io::Error), + /// Failed enabling the interrupt. + EnableInterrupt(io::Error), +} + +type Result = result::Result; + +pub struct MsiMessage { + // Message Address Register + // 31-20: Base address. Fixed value (0x0FEE) + // 19-12: Destination ID + // 11-4: Reserved + // 3: Redirection Hint indication + // 2: Destination Mode + // 1-0: Reserved + pub addr: u32, + // Message Data Register + // 32-16: Reserved + // 15: Trigger Mode. 0 = Edge, 1 = Level + // 14: Level. 0 = Deassert, 1 = Assert + // 13-11: Reserved + // 10-8: Delivery Mode + // 7-0: Vector + pub data: u32, +} + +// Introduce trait InterruptController to uniform the interrupt +// service provided for devices. +// Device manager uses this trait without caring whether it is a +// IOAPIC (X86) or GIC (Arm). +pub trait InterruptController: Send { + fn service_irq(&mut self, irq: usize) -> Result<()>; + fn end_of_interrupt(&mut self, vec: u8); +} diff --git a/devices/src/ioapic.rs b/devices/src/ioapic.rs index a4bf065d3..565937f7a 100644 --- a/devices/src/ioapic.rs +++ b/devices/src/ioapic.rs @@ -9,10 +9,10 @@ // Implementation of an intel 82093AA Input/Output Advanced Programmable Interrupt Controller // See https://pdos.csail.mit.edu/6.828/2016/readings/ia32/ioapic.pdf for a specification. +use super::interrupt_controller::{Error, InterruptController}; use crate::BusDevice; use anyhow::anyhow; use byteorder::{ByteOrder, LittleEndian}; -use std::io; use std::result; use std::sync::Arc; use vm_device::interrupt::{ @@ -29,28 +29,6 @@ use vm_migration::{ #[serde(remote = "GuestAddress")] pub struct GuestAddressDef(pub u64); -#[derive(Debug)] -pub enum Error { - /// Invalid destination mode. - InvalidDestinationMode, - /// Invalid trigger mode. - InvalidTriggerMode, - /// Invalid delivery mode. - InvalidDeliveryMode, - /// Failed creating the interrupt source group. - CreateInterruptSourceGroup(io::Error), - /// Failed triggering the interrupt. - TriggerInterrupt(io::Error), - /// Failed masking the interrupt. - MaskInterrupt(io::Error), - /// Failed unmasking the interrupt. - UnmaskInterrupt(io::Error), - /// Failed updating the interrupt. - UpdateInterrupt(io::Error), - /// Failed enabling the interrupt. - EnableInterrupt(io::Error), -} - type Result = result::Result; // I/O REDIRECTION TABLE REGISTER @@ -238,35 +216,77 @@ impl Ioapic { }) } - // The ioapic must be informed about EOIs in order to deassert interrupts - // already sent. - pub fn end_of_interrupt(&mut self, vec: u8) { - for i in 0..NUM_IOAPIC_PINS { - let entry = &mut self.reg_entries[i]; - // Clear Remote IRR bit - if vector(*entry) == vec && trigger_mode(*entry) == 1 { - set_remote_irr(entry, 0); + fn ioapic_write(&mut self, val: u32) { + debug!("IOAPIC_W reg 0x{:x}, val 0x{:x}", self.reg_sel, val); + + match self.reg_sel as u8 { + IOAPIC_REG_ID => self.id_reg = (val >> 24) & 0xf, + IOWIN_OFF..=REG_MAX_OFFSET => { + let (index, is_high_bits) = decode_irq_from_selector(self.reg_sel as u8); + if is_high_bits { + self.reg_entries[index] &= 0xffff_ffff; + self.reg_entries[index] |= u64::from(val) << 32; + } else { + // Ensure not to override read-only bits: + // - Delivery Status (bit 12) + // - Remote IRR (bit 14) + self.reg_entries[index] &= 0xffff_ffff_0000_5000; + self.reg_entries[index] |= u64::from(val) & 0xffff_afff; + } + // The entry must be updated through the interrupt source + // group. + if let Err(e) = self.update_entry(index) { + error!("Failed updating IOAPIC entry: {:?}", e); + } + // Store the information this IRQ is now being used. + self.used_entries[index] = true; + } + _ => error!("IOAPIC: invalid write to register offset"), + } + } + + fn ioapic_read(&self) -> u32 { + debug!("IOAPIC_R reg 0x{:x}", self.reg_sel); + + match self.reg_sel as u8 { + IOAPIC_REG_VERSION => IOAPIC_VERSION_ID, + IOAPIC_REG_ID | IOAPIC_REG_ARBITRATION_ID => (self.id_reg & 0xf) << 24, + IOWIN_OFF..=REG_MAX_OFFSET => { + let (index, is_high_bits) = decode_irq_from_selector(self.reg_sel as u8); + if is_high_bits { + (self.reg_entries[index] >> 32) as u32 + } else { + (self.reg_entries[index] & 0xffff_ffff) as u32 + } + } + _ => { + error!("IOAPIC: invalid read from register offset"); + 0 } } } - // This should be called anytime an interrupt needs to be injected into the - // running guest. - pub fn service_irq(&mut self, irq: usize) -> Result<()> { - let entry = &mut self.reg_entries[irq]; - - self.interrupt_source_group - .trigger(irq as InterruptIndex) - .map_err(Error::TriggerInterrupt)?; - debug!("Interrupt successfully delivered"); - - // If trigger mode is level sensitive, set the Remote IRR bit. - // It will be cleared when the EOI is received. - if trigger_mode(*entry) == 1 { - set_remote_irr(entry, 1); + fn state(&self) -> IoapicState { + IoapicState { + id_reg: self.id_reg, + reg_sel: self.reg_sel, + reg_entries: self.reg_entries, + used_entries: self.used_entries, + apic_address: self.apic_address, + } + } + + fn set_state(&mut self, state: &IoapicState) -> Result<()> { + self.id_reg = state.id_reg; + self.reg_sel = state.reg_sel; + self.reg_entries = state.reg_entries; + self.used_entries = state.used_entries; + self.apic_address = state.apic_address; + for (irq, entry) in self.used_entries.iter().enumerate() { + if *entry { + self.update_entry(irq)?; + } } - // Clear the Delivery Status bit - set_delivery_status(entry, 0); Ok(()) } @@ -342,78 +362,38 @@ impl Ioapic { Ok(()) } +} - fn ioapic_write(&mut self, val: u32) { - debug!("IOAPIC_W reg 0x{:x}, val 0x{:x}", self.reg_sel, val); - - match self.reg_sel as u8 { - IOAPIC_REG_ID => self.id_reg = (val >> 24) & 0xf, - IOWIN_OFF..=REG_MAX_OFFSET => { - let (index, is_high_bits) = decode_irq_from_selector(self.reg_sel as u8); - if is_high_bits { - self.reg_entries[index] &= 0xffff_ffff; - self.reg_entries[index] |= u64::from(val) << 32; - } else { - // Ensure not to override read-only bits: - // - Delivery Status (bit 12) - // - Remote IRR (bit 14) - self.reg_entries[index] &= 0xffff_ffff_0000_5000; - self.reg_entries[index] |= u64::from(val) & 0xffff_afff; - } - // The entry must be updated through the interrupt source - // group. - if let Err(e) = self.update_entry(index) { - error!("Failed updating IOAPIC entry: {:?}", e); - } - // Store the information this IRQ is now being used. - self.used_entries[index] = true; - } - _ => error!("IOAPIC: invalid write to register offset"), - } - } - - fn ioapic_read(&self) -> u32 { - debug!("IOAPIC_R reg 0x{:x}", self.reg_sel); - - match self.reg_sel as u8 { - IOAPIC_REG_VERSION => IOAPIC_VERSION_ID, - IOAPIC_REG_ID | IOAPIC_REG_ARBITRATION_ID => (self.id_reg & 0xf) << 24, - IOWIN_OFF..=REG_MAX_OFFSET => { - let (index, is_high_bits) = decode_irq_from_selector(self.reg_sel as u8); - if is_high_bits { - (self.reg_entries[index] >> 32) as u32 - } else { - (self.reg_entries[index] & 0xffff_ffff) as u32 - } - } - _ => { - error!("IOAPIC: invalid read from register offset"); - 0 +impl InterruptController for Ioapic { + // The ioapic must be informed about EOIs in order to deassert interrupts + // already sent. + fn end_of_interrupt(&mut self, vec: u8) { + for i in 0..NUM_IOAPIC_PINS { + let entry = &mut self.reg_entries[i]; + // Clear Remote IRR bit + if vector(*entry) == vec && trigger_mode(*entry) == 1 { + set_remote_irr(entry, 0); } } } - fn state(&self) -> IoapicState { - IoapicState { - id_reg: self.id_reg, - reg_sel: self.reg_sel, - reg_entries: self.reg_entries, - used_entries: self.used_entries, - apic_address: self.apic_address, - } - } + // This should be called anytime an interrupt needs to be injected into the + // running guest. + fn service_irq(&mut self, irq: usize) -> Result<()> { + let entry = &mut self.reg_entries[irq]; - fn set_state(&mut self, state: &IoapicState) -> Result<()> { - self.id_reg = state.id_reg; - self.reg_sel = state.reg_sel; - self.reg_entries = state.reg_entries; - self.used_entries = state.used_entries; - self.apic_address = state.apic_address; - for (irq, entry) in self.used_entries.iter().enumerate() { - if *entry { - self.update_entry(irq)?; - } + self.interrupt_source_group + .trigger(irq as InterruptIndex) + .map_err(Error::TriggerInterrupt)?; + debug!("Interrupt successfully delivered"); + + // If trigger mode is level sensitive, set the Remote IRR bit. + // It will be cleared when the EOI is received. + if trigger_mode(*entry) == 1 { + set_remote_irr(entry, 1); } + // Clear the Delivery Status bit + set_delivery_status(entry, 0); Ok(()) } diff --git a/devices/src/lib.rs b/devices/src/lib.rs index ba553d6b5..d936f0fd0 100644 --- a/devices/src/lib.rs +++ b/devices/src/lib.rs @@ -31,6 +31,7 @@ use std::io; #[cfg(feature = "acpi")] mod acpi; mod bus; +pub mod interrupt_controller; pub mod ioapic; pub mod legacy; diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index 23f720c10..47d12efdf 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -20,7 +20,7 @@ use anyhow::anyhow; #[cfg(feature = "acpi")] use arch::layout; use arch::EntryPoint; -use devices::{ioapic, BusDevice}; +use devices::{interrupt_controller::InterruptController, BusDevice}; #[cfg(target_arch = "x86_64")] use kvm_bindings::{ kvm_fpu, kvm_lapic_state, kvm_mp_state, kvm_regs, kvm_sregs, kvm_vcpu_events, kvm_xcrs, @@ -329,7 +329,7 @@ pub struct Vcpu { io_bus: Arc, mmio_bus: Arc, #[cfg_attr(target_arch = "aarch64", allow(dead_code))] - ioapic: Option>>, + interrupt_controller: Option>>, #[cfg_attr(target_arch = "aarch64", allow(dead_code))] vm_ts: std::time::Instant, } @@ -364,7 +364,7 @@ impl Vcpu { fd: &Arc, io_bus: Arc, mmio_bus: Arc, - ioapic: Option>>, + interrupt_controller: Option>>, creation_ts: std::time::Instant, ) -> Result>> { let kvm_vcpu = fd.create_vcpu(id).map_err(Error::VcpuFd)?; @@ -374,7 +374,7 @@ impl Vcpu { id, io_bus, mmio_bus, - ioapic, + interrupt_controller, vm_ts: creation_ts, }))) } @@ -452,8 +452,11 @@ impl Vcpu { } #[cfg(target_arch = "x86_64")] VcpuExit::IoapicEoi(vector) => { - if let Some(ioapic) = &self.ioapic { - ioapic.lock().unwrap().end_of_interrupt(vector); + if let Some(interrupt_controller) = &self.interrupt_controller { + interrupt_controller + .lock() + .unwrap() + .end_of_interrupt(vector); } Ok(true) } @@ -618,7 +621,7 @@ pub struct CpuManager { #[cfg_attr(target_arch = "aarch64", allow(dead_code))] mmio_bus: Arc, #[cfg_attr(target_arch = "aarch64", allow(dead_code))] - ioapic: Option>>, + interrupt_controller: Option>>, #[cfg_attr(target_arch = "aarch64", allow(dead_code))] vm_memory: GuestMemoryAtomic, #[cfg(target_arch = "x86_64")] @@ -769,7 +772,7 @@ impl CpuManager { max_vcpus: config.max_vcpus, io_bus: device_manager.io_bus().clone(), mmio_bus: device_manager.mmio_bus().clone(), - ioapic: device_manager.ioapic().clone(), + interrupt_controller: device_manager.interrupt_controller().clone(), vm_memory: guest_memory, #[cfg(target_arch = "x86_64")] cpuid, @@ -858,8 +861,8 @@ impl CpuManager { inserting: bool, snapshot: Option, ) -> Result<()> { - let ioapic = if let Some(ioapic) = &self.ioapic { - Some(ioapic.clone()) + let interrupt_controller = if let Some(interrupt_controller) = &self.interrupt_controller { + Some(interrupt_controller.clone()) } else { None }; @@ -869,7 +872,7 @@ impl CpuManager { &self.fd, self.io_bus.clone(), self.mmio_bus.clone(), - ioapic, + interrupt_controller, creation_ts, )?; diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index bee084977..cc2d81181 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -26,7 +26,10 @@ use anyhow::anyhow; use arch::layout; #[cfg(target_arch = "x86_64")] use arch::layout::{APIC_START, IOAPIC_SIZE, IOAPIC_START}; -use devices::{ioapic, BusDevice, HotPlugNotificationFlags}; +use devices::{ + interrupt_controller, interrupt_controller::InterruptController, ioapic, BusDevice, + HotPlugNotificationFlags, +}; use kvm_ioctls::*; use libc::TIOCGWINSZ; use libc::{MAP_NORESERVE, MAP_PRIVATE, MAP_SHARED, O_TMPFILE, PROT_READ, PROT_WRITE}; @@ -235,8 +238,8 @@ pub enum DeviceManagerError { /// Failed to update interrupt source group. UpdateInterruptGroup(io::Error), - /// Failed creating IOAPIC. - CreateIoapic(ioapic::Error), + /// Failed creating interrupt controller. + CreateInterruptController(interrupt_controller::Error), /// Failed creating a new MmapRegion instance. NewMmapRegion(vm_memory::mmap::MmapRegionError), @@ -626,8 +629,8 @@ pub struct DeviceManager { // Console abstraction console: Arc, - // IOAPIC - ioapic: Option>>, + // Interrupt controller + interrupt_controller: Option>>, // Things to be added to the commandline (i.e. for virtio-mmio) cmdline_additions: Vec, @@ -746,7 +749,7 @@ impl DeviceManager { let device_manager = DeviceManager { address_manager: Arc::clone(&address_manager), console: Arc::new(Console::default()), - ioapic: None, + interrupt_controller: None, cmdline_additions: Vec::new(), #[cfg(feature = "acpi")] ged_notification_device: None, @@ -804,13 +807,15 @@ impl DeviceManager { pub fn create_devices(&mut self) -> DeviceManagerResult<()> { let mut virtio_devices: Vec<(VirtioDeviceArc, bool, String)> = Vec::new(); - let ioapic = self.add_ioapic()?; + let interrupt_controller = self.add_interrupt_controller()?; // Now we can create the legacy interrupt manager, which needs the freshly // formed IOAPIC device. let legacy_interrupt_manager: Arc< dyn InterruptManager, - > = Arc::new(KvmLegacyUserspaceInterruptManager::new(ioapic)); + > = Arc::new(KvmLegacyUserspaceInterruptManager::new(Arc::clone( + &interrupt_controller, + ))); #[cfg(feature = "acpi")] self.address_manager @@ -1003,33 +1008,37 @@ impl DeviceManager { } #[cfg(target_arch = "aarch64")] - fn add_ioapic(&mut self) -> DeviceManagerResult>> { + fn add_interrupt_controller( + &mut self, + ) -> DeviceManagerResult>> { unimplemented!(); } #[cfg(target_arch = "x86_64")] - fn add_ioapic(&mut self) -> DeviceManagerResult>> { + fn add_interrupt_controller( + &mut self, + ) -> DeviceManagerResult>> { let id = String::from(IOAPIC_DEVICE_NAME); // Create IOAPIC - let ioapic = Arc::new(Mutex::new( + let interrupt_controller = Arc::new(Mutex::new( ioapic::Ioapic::new( id.clone(), APIC_START, Arc::clone(&self.msi_interrupt_manager), ) - .map_err(DeviceManagerError::CreateIoapic)?, + .map_err(DeviceManagerError::CreateInterruptController)?, )); - self.ioapic = Some(ioapic.clone()); + self.interrupt_controller = Some(interrupt_controller.clone()); self.address_manager .mmio_bus - .insert(ioapic.clone(), IOAPIC_START.0, IOAPIC_SIZE) + .insert(interrupt_controller.clone(), IOAPIC_START.0, IOAPIC_SIZE) .map_err(DeviceManagerError::BusError)?; self.bus_devices - .push(Arc::clone(&ioapic) as Arc>); + .push(Arc::clone(&interrupt_controller) as Arc>); // Fill the device tree with a new node. In case of restore, we // know there is nothing to do, so we can simply override the @@ -1037,9 +1046,9 @@ impl DeviceManager { self.device_tree .lock() .unwrap() - .insert(id.clone(), device_node!(id, ioapic)); + .insert(id.clone(), device_node!(id, interrupt_controller)); - Ok(ioapic) + Ok(interrupt_controller) } #[cfg(feature = "acpi")] @@ -2550,8 +2559,12 @@ impl DeviceManager { &self.address_manager.allocator } - pub fn ioapic(&self) -> &Option>> { - &self.ioapic + pub fn interrupt_controller(&self) -> Option>> { + if let Some(interrupt_controller) = &self.interrupt_controller { + Some(interrupt_controller.clone() as Arc>) + } else { + None + } } pub fn console(&self) -> &Arc { diff --git a/vmm/src/interrupt.rs b/vmm/src/interrupt.rs index fad2ea691..f3934e33d 100644 --- a/vmm/src/interrupt.rs +++ b/vmm/src/interrupt.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause // -use devices::ioapic; +use devices::interrupt_controller::InterruptController; use kvm_bindings::{kvm_irq_routing, kvm_irq_routing_entry, KVM_IRQ_ROUTING_MSI}; use kvm_ioctls::VmFd; use std::collections::HashMap; @@ -281,12 +281,12 @@ impl InterruptSourceGroup for MsiInterruptGroup { } pub struct LegacyUserspaceInterruptGroup { - ioapic: Arc>, + ioapic: Arc>, irq: u32, } impl LegacyUserspaceInterruptGroup { - fn new(ioapic: Arc>, irq: u32) -> Self { + fn new(ioapic: Arc>, irq: u32) -> Self { LegacyUserspaceInterruptGroup { ioapic, irq } } } @@ -311,7 +311,7 @@ impl InterruptSourceGroup for LegacyUserspaceInterruptGroup { } pub struct KvmLegacyUserspaceInterruptManager { - ioapic: Arc>, + ioapic: Arc>, } pub struct KvmMsiInterruptManager { @@ -321,7 +321,7 @@ pub struct KvmMsiInterruptManager { } impl KvmLegacyUserspaceInterruptManager { - pub fn new(ioapic: Arc>) -> Self { + pub fn new(ioapic: Arc>) -> Self { KvmLegacyUserspaceInterruptManager { ioapic } } }