From 05e5106b9b12a2b6a486289eef12df130bcfe45e Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Mon, 18 Jul 2022 15:47:51 +0000 Subject: [PATCH] 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 --- Cargo.lock | 1 + arch/src/x86_64/interrupts.rs | 67 ++----------------------------- hypervisor/Cargo.toml | 1 + hypervisor/src/arch/x86/mod.rs | 65 ++++++++++++++++++++++++++++++ hypervisor/src/cpu.rs | 4 +- hypervisor/src/kvm/mod.rs | 13 +++--- hypervisor/src/kvm/x86_64/mod.rs | 23 +++++++++-- hypervisor/src/mshv/mod.rs | 11 +++-- hypervisor/src/mshv/x86_64/mod.rs | 22 +++++++++- vmm/src/cpu.rs | 11 +++-- 10 files changed, 132 insertions(+), 86 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 811f8f90c..4c4ea33df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -342,6 +342,7 @@ name = "hypervisor" version = "0.1.0" dependencies = [ "anyhow", + "byteorder", "env_logger", "epoll", "iced-x86", diff --git a/arch/src/x86_64/interrupts.rs b/arch/src/x86_64/interrupts.rs index 0ada843d0..1ca322ec7 100644 --- a/arch/src/x86_64/interrupts.rs +++ b/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::() - .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::(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) -> Result<()> { let mut klapic = vcpu.get_lapic()?; - let lvt_lint0 = get_klapic_reg(&klapic, APIC_LVT0); - set_klapic_reg( - &mut klapic, + let lvt_lint0 = klapic.get_klapic_reg(APIC_LVT0); + klapic.set_klapic_reg( APIC_LVT0, set_apic_delivery_mode(lvt_lint0, APIC_MODE_EXTINT), ); - let lvt_lint1 = get_klapic_reg(&klapic, APIC_LVT1); - set_klapic_reg( - &mut klapic, - APIC_LVT1, - set_apic_delivery_mode(lvt_lint1, APIC_MODE_NMI), - ); + let lvt_lint1 = klapic.get_klapic_reg(APIC_LVT1); + klapic.set_klapic_reg(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); - } -} diff --git a/hypervisor/Cargo.toml b/hypervisor/Cargo.toml index 611630478..dd4ed1381 100644 --- a/hypervisor/Cargo.toml +++ b/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" diff --git a/hypervisor/src/arch/x86/mod.rs b/hypervisor/src/arch/x86/mod.rs index 32751715f..e866cac5e 100644 --- a/hypervisor/src/arch/x86/mod.rs +++ b/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::() + .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::(value) + .expect("Failed to write klapic register") + } +} diff --git a/hypervisor/src/cpu.rs b/hypervisor/src/cpu.rs index dca8d271e..ad51aeb51 100644 --- a/hypervisor/src/cpu.rs +++ b/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")] diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index 0c1581f20..4fe12f0f8 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/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 { - 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")] diff --git a/hypervisor/src/kvm/x86_64/mod.rs b/hypervisor/src/kvm/x86_64/mod.rs index 12df38771..fd7d0bd15 100644 --- a/hypervisor/src/kvm/x86_64/mod.rs +++ b/hypervisor/src/kvm/x86_64/mod.rs @@ -9,8 +9,8 @@ // use crate::arch::x86::{ - CpuIdEntry, DescriptorTable, FpuState, SegmentRegister, SpecialRegisters, StandardRegisters, - CPUID_FLAG_VALID_INDEX, + CpuIdEntry, DescriptorTable, FpuState, LapicState, SegmentRegister, SpecialRegisters, + 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 for kvm_fpu { } } } + +impl From 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 for LapicState { + fn from(s: kvm_lapic_state) -> Self { + LapicState::Kvm(s) + } +} diff --git a/hypervisor/src/mshv/mod.rs b/hypervisor/src/mshv/mod.rs index c6dfbb116..39fe4eabf 100644 --- a/hypervisor/src/mshv/mod.rs +++ b/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 { - 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())) } /// diff --git a/hypervisor/src/mshv/x86_64/mod.rs b/hypervisor/src/mshv/x86_64/mod.rs index d1cc9b0ba..a0259cb9e 100644 --- a/hypervisor/src/mshv/x86_64/mod.rs +++ b/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 for FloatingPointUnit { } } } + +impl From 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 for LapicState { + fn from(s: MshvLapicState) -> Self { + LapicState::Mshv(s) + } +} diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index a80a36c57..786aa907a 100644 --- a/vmm/src/cpu.rs +++ b/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::x86_64::LapicState; + use hypervisor::arch::x86::{FpuState, LapicState, StandardRegisters}; #[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 lint1 = get_klapic_reg(&klapic_before, APIC_LVT1); + let lint0 = klapic_before.get_klapic_reg(APIC_LVT0); + 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 lint1_mode_actual = get_klapic_reg(&klapic_actual, APIC_LVT1); + let lint0_mode_actual = klapic_actual.get_klapic_reg(APIC_LVT0); + 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); }