hypervisor x86: provide a generic LapicState structure

This requires making get/set_lapic_reg part of the type.

For the moment we cannot provide a default variant for the new type,
because picking one will be wrong for the other hypervisor, so I just
drop the test cases that requires LapicState::default().

Signed-off-by: Wei Liu <liuwe@microsoft.com>

Cargo.lock

@@ -342,6 +342,7 @@ name = "hypervisor"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "byteorder",
  "env_logger",
  "epoll",
  "iced-x86",

arch/src/x86_64/interrupts.rs

@@ -5,10 +5,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE-BSD-3-Clause file.
 
-use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
-use hypervisor::x86_64::LapicState;
-use std::io::Cursor;
-use std::mem;
 use std::result;
 use std::sync::Arc;
 
@@ -20,32 +16,6 @@ pub const APIC_LVT1: usize = 0x360;
 pub const APIC_MODE_NMI: u32 = 0x4;
 pub const APIC_MODE_EXTINT: u32 = 0x7;
 
-pub fn get_klapic_reg(klapic: &LapicState, reg_offset: usize) -> u32 {
-    let sliceu8 = unsafe {
-        // This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
-        // Cursors are only readable on arrays of u8, not i8(c_char).
-        mem::transmute::<&[i8], &[u8]>(&klapic.regs[reg_offset..])
-    };
-    let mut reader = Cursor::new(sliceu8);
-    // Following call can't fail if the offsets defined above are correct.
-    reader
-        .read_u32::<LittleEndian>()
-        .expect("Failed to read klapic register")
-}
-
-pub fn set_klapic_reg(klapic: &mut LapicState, reg_offset: usize, value: u32) {
-    let sliceu8 = unsafe {
-        // This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
-        // Cursors are only readable on arrays of u8, not i8(c_char).
-        mem::transmute::<&mut [i8], &mut [u8]>(&mut klapic.regs[reg_offset..])
-    };
-    let mut writer = Cursor::new(sliceu8);
-    // Following call can't fail if the offsets defined above are correct.
-    writer
-        .write_u32::<LittleEndian>(value)
-        .expect("Failed to write klapic register")
-}
-
 pub fn set_apic_delivery_mode(reg: u32, mode: u32) -> u32 {
     ((reg) & !0x700) | ((mode) << 8)
 }
@@ -57,42 +27,13 @@ pub fn set_apic_delivery_mode(reg: u32, mode: u32) -> u32 {
 pub fn set_lint(vcpu: &Arc<dyn hypervisor::Vcpu>) -> Result<()> {
     let mut klapic = vcpu.get_lapic()?;
 
-    let lvt_lint0 = get_klapic_reg(&klapic, APIC_LVT0);
+    let lvt_lint0 = klapic.get_klapic_reg(APIC_LVT0);
-    set_klapic_reg(
+    klapic.set_klapic_reg(
-        &mut klapic,
         APIC_LVT0,
         set_apic_delivery_mode(lvt_lint0, APIC_MODE_EXTINT),
     );
-    let lvt_lint1 = get_klapic_reg(&klapic, APIC_LVT1);
+    let lvt_lint1 = klapic.get_klapic_reg(APIC_LVT1);
-    set_klapic_reg(
+    klapic.set_klapic_reg(APIC_LVT1, set_apic_delivery_mode(lvt_lint1, APIC_MODE_NMI));
-        &mut klapic,
-        APIC_LVT1,
-        set_apic_delivery_mode(lvt_lint1, APIC_MODE_NMI),
-    );
 
     vcpu.set_lapic(&klapic)
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    const KVM_APIC_REG_SIZE: usize = 0x400;
-
-    #[test]
-    fn test_set_and_get_klapic_reg() {
-        let reg_offset = 0x340;
-        let mut klapic = LapicState::default();
-        set_klapic_reg(&mut klapic, reg_offset, 3);
-        let value = get_klapic_reg(&klapic, reg_offset);
-        assert_eq!(value, 3);
-    }
-
-    #[test]
-    #[should_panic]
-    fn test_set_and_get_klapic_out_of_bounds() {
-        let reg_offset = KVM_APIC_REG_SIZE + 10;
-        let mut klapic = LapicState::default();
-        set_klapic_reg(&mut klapic, reg_offset, 3);
-    }
-}

hypervisor/Cargo.toml

@@ -12,6 +12,7 @@ tdx = []
 
 [dependencies]
 anyhow = "1.0.58"
+byteorder = "1.4.3"
 epoll = "4.3.1"
 thiserror = "1.0.31"
 libc = "0.2.126"

hypervisor/src/arch/x86/mod.rs

@@ -254,3 +254,68 @@ pub struct FpuState {
     pub xmm: [[u8; 16usize]; 16usize],
     pub mxcsr: u32,
 }
+
+#[derive(Debug, Clone, serde::Deserialize, serde::Serialize)]
+pub enum LapicState {
+    #[cfg(feature = "kvm")]
+    Kvm(kvm_bindings::kvm_lapic_state),
+    #[cfg(feature = "mshv")]
+    Mshv(mshv_bindings::LapicState),
+}
+
+#[cfg(any(feature = "kvm", feature = "mshv"))]
+impl LapicState {
+    pub fn get_klapic_reg(&self, reg_offset: usize) -> u32 {
+        use byteorder::{LittleEndian, ReadBytesExt};
+        use std::io::Cursor;
+        use std::mem;
+
+        let sliceu8 = match self {
+            #[cfg(feature = "kvm")]
+            LapicState::Kvm(s) => unsafe {
+                // This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
+                // Cursors are only readable on arrays of u8, not i8(c_char).
+                mem::transmute::<&[i8], &[u8]>(&s.regs[reg_offset..])
+            },
+            #[cfg(feature = "mshv")]
+            LapicState::Mshv(s) => unsafe {
+                // This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
+                // Cursors are only readable on arrays of u8, not i8(c_char).
+                mem::transmute::<&[i8], &[u8]>(&s.regs[reg_offset..])
+            },
+        };
+
+        let mut reader = Cursor::new(sliceu8);
+        // Following call can't fail if the offsets defined above are correct.
+        reader
+            .read_u32::<LittleEndian>()
+            .expect("Failed to read klapic register")
+    }
+
+    pub fn set_klapic_reg(&mut self, reg_offset: usize, value: u32) {
+        use byteorder::{LittleEndian, WriteBytesExt};
+        use std::io::Cursor;
+        use std::mem;
+
+        let sliceu8 = match self {
+            #[cfg(feature = "kvm")]
+            LapicState::Kvm(s) => unsafe {
+                // This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
+                // Cursors are only readable on arrays of u8, not i8(c_char).
+                mem::transmute::<&mut [i8], &mut [u8]>(&mut s.regs[reg_offset..])
+            },
+            #[cfg(feature = "mshv")]
+            LapicState::Mshv(s) => unsafe {
+                // This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
+                // Cursors are only readable on arrays of u8, not i8(c_char).
+                mem::transmute::<&mut [i8], &mut [u8]>(&mut s.regs[reg_offset..])
+            },
+        };
+
+        let mut writer = Cursor::new(sliceu8);
+        // Following call can't fail if the offsets defined above are correct.
+        writer
+            .write_u32::<LittleEndian>(value)
+            .expect("Failed to write klapic register")
+    }
+}

hypervisor/src/cpu.rs

@@ -13,12 +13,10 @@ use crate::aarch64::VcpuInit;
 #[cfg(target_arch = "aarch64")]
 use crate::aarch64::{RegList, Register, StandardRegisters};
 #[cfg(target_arch = "x86_64")]
-use crate::arch::x86::{CpuIdEntry, FpuState, SpecialRegisters, StandardRegisters};
+use crate::arch::x86::{CpuIdEntry, FpuState, LapicState, SpecialRegisters, StandardRegisters};
 #[cfg(feature = "tdx")]
 use crate::kvm::{TdxExitDetails, TdxExitStatus};
 #[cfg(target_arch = "x86_64")]
-use crate::x86_64::LapicState;
-#[cfg(target_arch = "x86_64")]
 use crate::x86_64::MsrEntries;
 use crate::CpuState;
 #[cfg(target_arch = "aarch64")]

hypervisor/src/kvm/mod.rs

@@ -47,7 +47,7 @@ use vmm_sys_util::eventfd::EventFd;
 pub mod x86_64;
 #[cfg(target_arch = "x86_64")]
 use crate::arch::x86::{
-    CpuIdEntry, FpuState, SpecialRegisters, StandardRegisters, NUM_IOAPIC_PINS,
+    CpuIdEntry, FpuState, LapicState, SpecialRegisters, StandardRegisters, NUM_IOAPIC_PINS,
 };
 #[cfg(target_arch = "x86_64")]
 use crate::ClockData;
@@ -65,7 +65,7 @@ use kvm_bindings::{
 #[cfg(target_arch = "x86_64")]
 use x86_64::check_required_kvm_extensions;
 #[cfg(target_arch = "x86_64")]
-pub use x86_64::{CpuId, ExtendedControlRegisters, LapicState, MsrEntries, VcpuKvmState, Xsave};
+pub use x86_64::{CpuId, ExtendedControlRegisters, MsrEntries, VcpuKvmState, Xsave};
 // aarch64 dependencies
 #[cfg(target_arch = "aarch64")]
 pub mod aarch64;
@@ -1374,17 +1374,20 @@ impl cpu::Vcpu for KvmVcpu {
     /// Returns the state of the LAPIC (Local Advanced Programmable Interrupt Controller).
     ///
     fn get_lapic(&self) -> cpu::Result<LapicState> {
-        self.fd
+        Ok(self
+            .fd
             .get_lapic()
-            .map_err(|e| cpu::HypervisorCpuError::GetlapicState(e.into()))
+            .map_err(|e| cpu::HypervisorCpuError::GetlapicState(e.into()))?
+            .into())
     }
     #[cfg(target_arch = "x86_64")]
     ///
     /// Sets the state of the LAPIC (Local Advanced Programmable Interrupt Controller).
     ///
     fn set_lapic(&self, klapic: &LapicState) -> cpu::Result<()> {
+        let klapic: kvm_bindings::kvm_lapic_state = (*klapic).clone().into();
         self.fd
-            .set_lapic(klapic)
+            .set_lapic(&klapic)
             .map_err(|e| cpu::HypervisorCpuError::SetLapicState(e.into()))
     }
     #[cfg(target_arch = "x86_64")]

hypervisor/src/kvm/x86_64/mod.rs

@@ -9,8 +9,8 @@
 //
 
 use crate::arch::x86::{
-    CpuIdEntry, DescriptorTable, FpuState, SegmentRegister, SpecialRegisters, StandardRegisters,
+    CpuIdEntry, DescriptorTable, FpuState, LapicState, SegmentRegister, SpecialRegisters,
-    CPUID_FLAG_VALID_INDEX,
+    StandardRegisters, CPUID_FLAG_VALID_INDEX,
 };
 use crate::kvm::{Cap, Kvm, KvmError, KvmResult};
 use serde::{Deserialize, Serialize};
@@ -20,7 +20,7 @@ use serde::{Deserialize, Serialize};
 ///
 pub use {
     kvm_bindings::kvm_cpuid_entry2, kvm_bindings::kvm_dtable, kvm_bindings::kvm_fpu,
-    kvm_bindings::kvm_lapic_state as LapicState, kvm_bindings::kvm_mp_state as MpState,
+    kvm_bindings::kvm_lapic_state, kvm_bindings::kvm_mp_state as MpState,
     kvm_bindings::kvm_msr_entry as MsrEntry, kvm_bindings::kvm_regs, kvm_bindings::kvm_segment,
     kvm_bindings::kvm_sregs, kvm_bindings::kvm_vcpu_events as VcpuEvents,
     kvm_bindings::kvm_xcrs as ExtendedControlRegisters, kvm_bindings::kvm_xsave as Xsave,
@@ -295,3 +295,20 @@ impl From<FpuState> for kvm_fpu {
         }
     }
 }
+
+impl From<LapicState> for kvm_lapic_state {
+    fn from(s: LapicState) -> Self {
+        match s {
+            LapicState::Kvm(s) => s,
+            /* Needed in case other hypervisors are enabled */
+            #[allow(unreachable_patterns)]
+            _ => panic!("LapicState is not valid"),
+        }
+    }
+}
+
+impl From<kvm_lapic_state> for LapicState {
+    fn from(s: kvm_lapic_state) -> Self {
+        LapicState::Kvm(s)
+    }
+}

hypervisor/src/mshv/mod.rs

@@ -37,7 +37,7 @@ use std::fs::File;
 use std::os::unix::io::AsRawFd;
 
 #[cfg(target_arch = "x86_64")]
-use crate::arch::x86::{CpuIdEntry, FpuState, SpecialRegisters, StandardRegisters};
+use crate::arch::x86::{CpuIdEntry, FpuState, LapicState, SpecialRegisters, StandardRegisters};
 
 const DIRTY_BITMAP_CLEAR_DIRTY: u64 = 0x4;
 const DIRTY_BITMAP_SET_DIRTY: u64 = 0x8;
@@ -543,17 +543,20 @@ impl cpu::Vcpu for MshvVcpu {
     /// Returns the state of the LAPIC (Local Advanced Programmable Interrupt Controller).
     ///
     fn get_lapic(&self) -> cpu::Result<LapicState> {
-        self.fd
+        Ok(self
+            .fd
             .get_lapic()
-            .map_err(|e| cpu::HypervisorCpuError::GetlapicState(e.into()))
+            .map_err(|e| cpu::HypervisorCpuError::GetlapicState(e.into()))?
+            .into())
     }
     #[cfg(target_arch = "x86_64")]
     ///
     /// Sets the state of the LAPIC (Local Advanced Programmable Interrupt Controller).
     ///
     fn set_lapic(&self, lapic: &LapicState) -> cpu::Result<()> {
+        let lapic: mshv_bindings::LapicState = (*lapic).clone().into();
         self.fd
-            .set_lapic(lapic)
+            .set_lapic(&lapic)
             .map_err(|e| cpu::HypervisorCpuError::SetLapicState(e.into()))
     }
     ///

hypervisor/src/mshv/x86_64/mod.rs

@@ -8,7 +8,8 @@
 //
 //
 use crate::arch::x86::{
-    CpuIdEntry, DescriptorTable, FpuState, SegmentRegister, SpecialRegisters, StandardRegisters,
+    CpuIdEntry, DescriptorTable, FpuState, LapicState, SegmentRegister, SpecialRegisters,
+    StandardRegisters,
 };
 use serde::{Deserialize, Serialize};
 use std::fmt;
@@ -19,7 +20,7 @@ use std::fmt;
 pub use {
     mshv_bindings::hv_cpuid_entry, mshv_bindings::mshv_user_mem_region as MemoryRegion,
     mshv_bindings::msr_entry as MsrEntry, mshv_bindings::CpuId, mshv_bindings::DebugRegisters,
-    mshv_bindings::FloatingPointUnit, mshv_bindings::LapicState,
+    mshv_bindings::FloatingPointUnit, mshv_bindings::LapicState as MshvLapicState,
     mshv_bindings::MiscRegs as MiscRegisters, mshv_bindings::MsrList,
     mshv_bindings::Msrs as MsrEntries, mshv_bindings::Msrs,
     mshv_bindings::SegmentRegister as MshvSegmentRegister,
@@ -285,3 +286,20 @@ impl From<FpuState> for FloatingPointUnit {
         }
     }
 }
+
+impl From<LapicState> for MshvLapicState {
+    fn from(s: LapicState) -> Self {
+        match s {
+            LapicState::Mshv(s) => s,
+            /* Needed in case other hypervisors are enabled */
+            #[allow(unreachable_patterns)]
+            _ => panic!("LapicState is not valid"),
+        }
+    }
+}
+
+impl From<MshvLapicState> for LapicState {
+    fn from(s: MshvLapicState) -> Self {
+        LapicState::Mshv(s)
+    }
+}

vmm/src/cpu.rs

@@ -2336,8 +2336,7 @@ impl CpuElf64Writable for CpuManager {
 mod tests {
     use arch::x86_64::interrupts::*;
     use arch::x86_64::regs::*;
-    use hypervisor::arch::x86::{FpuState, StandardRegisters};
+    use hypervisor::arch::x86::{FpuState, LapicState, StandardRegisters};
-    use hypervisor::x86_64::LapicState;
 
     #[test]
     fn test_setlint() {
@@ -2350,8 +2349,8 @@ mod tests {
         let klapic_before: LapicState = vcpu.get_lapic().unwrap();
 
         // Compute the value that is expected to represent LVT0 and LVT1.
-        let lint0 = get_klapic_reg(&klapic_before, APIC_LVT0);
+        let lint0 = klapic_before.get_klapic_reg(APIC_LVT0);
-        let lint1 = get_klapic_reg(&klapic_before, APIC_LVT1);
+        let lint1 = klapic_before.get_klapic_reg(APIC_LVT1);
         let lint0_mode_expected = set_apic_delivery_mode(lint0, APIC_MODE_EXTINT);
         let lint1_mode_expected = set_apic_delivery_mode(lint1, APIC_MODE_NMI);
 
@@ -2359,8 +2358,8 @@ mod tests {
 
         // Compute the value that represents LVT0 and LVT1 after set_lint.
         let klapic_actual: LapicState = vcpu.get_lapic().unwrap();
-        let lint0_mode_actual = get_klapic_reg(&klapic_actual, APIC_LVT0);
+        let lint0_mode_actual = klapic_actual.get_klapic_reg(APIC_LVT0);
-        let lint1_mode_actual = get_klapic_reg(&klapic_actual, APIC_LVT1);
+        let lint1_mode_actual = klapic_actual.get_klapic_reg(APIC_LVT1);
         assert_eq!(lint0_mode_expected, lint0_mode_actual);
         assert_eq!(lint1_mode_expected, lint1_mode_actual);
     }