From c8b655490e300e28c957d0fd188cadfaa59ec8f9 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Wed, 25 Nov 2020 20:07:45 +0000 Subject: [PATCH] hypervisor: x86: reference PlatformEmulator in Emulator The observation here is PlatformEmulator can be seen as the context for emulation to take place. It should be rather easy to construct a context that satisfies the lifetime constraints for instruction emulation. The thread doing the emulation will have full ownership over the context, so this removes the need to wrap PlatformEmulator in Arc and Mutex, as well as the need for the context to be either Clone or Copy. Signed-off-by: Wei Liu --- .../src/arch/x86/emulator/instructions/mod.rs | 3 +- .../src/arch/x86/emulator/instructions/mov.rs | 74 ++++--------------- hypervisor/src/arch/x86/emulator/mod.rs | 18 ++--- 3 files changed, 23 insertions(+), 72 deletions(-) diff --git a/hypervisor/src/arch/x86/emulator/instructions/mod.rs b/hypervisor/src/arch/x86/emulator/instructions/mod.rs index ca79cf496..b7051270f 100644 --- a/hypervisor/src/arch/x86/emulator/instructions/mod.rs +++ b/hypervisor/src/arch/x86/emulator/instructions/mod.rs @@ -11,7 +11,6 @@ use crate::arch::x86::emulator::CpuStateManager; use crate::arch::x86::Exception; use iced_x86::*; use std::collections::HashMap; -use std::sync::{Arc, Mutex}; pub mod mov; @@ -48,7 +47,7 @@ pub trait InstructionHandler { &self, insn: &Instruction, state: &mut T, - platform: Arc>>, + platform: &mut dyn PlatformEmulator, ) -> Result<(), EmulationError>; } diff --git a/hypervisor/src/arch/x86/emulator/instructions/mov.rs b/hypervisor/src/arch/x86/emulator/instructions/mov.rs index 273cb7f0e..02a3b4a45 100644 --- a/hypervisor/src/arch/x86/emulator/instructions/mov.rs +++ b/hypervisor/src/arch/x86/emulator/instructions/mov.rs @@ -18,7 +18,6 @@ use crate::arch::emulator::{EmulationError, PlatformEmulator}; use crate::arch::x86::emulator::instructions::*; use crate::arch::x86::Exception; use std::mem; -use std::sync::{Arc, Mutex}; macro_rules! mov_rm_r { ($bound:ty) => { @@ -26,7 +25,7 @@ macro_rules! mov_rm_r { &self, insn: &Instruction, state: &mut T, - platform: Arc>>, + platform: &mut dyn PlatformEmulator, ) -> Result<(), EmulationError> { let src_reg_value = state .read_reg(insn.op1_register()) @@ -42,8 +41,6 @@ macro_rules! mov_rm_r { OpKind::Memory => { let src_reg_value_type: $bound = src_reg_value as $bound; platform - .lock() - .unwrap() .write_memory(addr, &src_reg_value_type.to_le_bytes()) .map_err(EmulationError::PlatformEmulationError)? } @@ -64,7 +61,7 @@ macro_rules! mov_rm_imm { &self, insn: &Instruction, state: &mut T, - platform: Arc>>, + platform: &mut dyn PlatformEmulator, ) -> Result<(), EmulationError> { let imm = imm_op!($type, insn); let addr = memory_operand_address(insn, state) @@ -75,8 +72,6 @@ macro_rules! mov_rm_imm { .write_reg(insn.op0_register(), imm as u64) .map_err(EmulationError::PlatformEmulationError)?, OpKind::Memory => platform - .lock() - .unwrap() .write_memory(addr, &imm.to_le_bytes()) .map_err(EmulationError::PlatformEmulationError)?, k => return Err(EmulationError::InvalidOperand(anyhow!("{:?}", k))), @@ -95,7 +90,7 @@ macro_rules! mov_r_rm { &self, insn: &Instruction, state: &mut T, - platform: Arc>>, + platform: &mut dyn PlatformEmulator, ) -> Result<(), EmulationError> { let src_value: $bound = match insn.op1_kind() { OpKind::Register => state @@ -107,8 +102,6 @@ macro_rules! mov_r_rm { .map_err(EmulationError::PlatformEmulationError)?; let mut memory: [u8; mem::size_of::<$bound>()] = [0; mem::size_of::<$bound>()]; platform - .lock() - .unwrap() .read_memory(target_address, &mut memory) .map_err(EmulationError::PlatformEmulationError)?; <$bound>::from_le_bytes(memory) @@ -134,7 +127,7 @@ macro_rules! mov_r_imm { &self, insn: &Instruction, state: &mut T, - _platform: Arc>>, + _platform: &mut dyn PlatformEmulator, ) -> Result<(), EmulationError> { state .write_reg(insn.op0_register(), imm_op!($type, insn) as u64) @@ -261,6 +254,7 @@ mod tests { use crate::arch::x86::gdt::{gdt_entry, segment_from_gdt}; use crate::arch::x86::Exception; use std::collections::HashMap; + use std::sync::{Arc, Mutex}; #[derive(Debug, Clone)] struct MockVMM { @@ -340,7 +334,7 @@ mod tests { regs: HashMap, memory: Option<(u64, &[u8])>, num_insn: Option, - ) -> Arc> { + ) -> MockVMM { let _ = env_logger::try_init(); let cs_reg = segment_from_gdt(gdt_entry(0xc09b, 0, 0xffffffff), 1); let ds_reg = segment_from_gdt(gdt_entry(0xc093, 0, 0xffffffff), 2); @@ -352,11 +346,11 @@ mod tests { initial_state.write_reg(reg, value).unwrap(); } - let vmm = Arc::new(Mutex::new(MockVMM::new(initial_state))); + let mut vmm = MockVMM::new(initial_state); if let Some(mem) = memory { - vmm.lock().unwrap().write_memory(mem.0, &mem.1).unwrap(); + vmm.write_memory(mem.0, &mem.1).unwrap(); } - let mut emulator = Emulator::new(vmm.clone()); + let mut emulator = Emulator::new(&mut vmm); let new_state = emulator .emulate_insn_stream(cpu_id, &insn, num_insn) @@ -365,10 +359,7 @@ mod tests { assert_eq!(ip + insn.len() as u64, new_state.ip()); } - vmm.lock() - .unwrap() - .set_cpu_state(cpu_id, new_state) - .unwrap(); + vmm.set_cpu_state(cpu_id, new_state).unwrap(); vmm } @@ -379,7 +370,7 @@ mod tests { insn: &[u8], regs: HashMap, memory: Option<(u64, &[u8])>, - ) -> Arc> { + ) -> MockVMM { _init_and_run(cpu_id, ip, insn, regs, memory, None) } @@ -393,8 +384,6 @@ mod tests { let vmm = init_and_run(cpu_id, ip, &insn, hashmap![Register::RBX => rbx], None); let rax: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RAX) @@ -414,8 +403,6 @@ mod tests { let vmm = init_and_run(cpu_id, ip, &insn, hashmap![], None); let rax: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RAX) @@ -443,8 +430,6 @@ mod tests { ); rax = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RAX) @@ -464,8 +449,6 @@ mod tests { let vmm = init_and_run(cpu_id, ip, &insn, hashmap![], None); let al = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::AL) @@ -485,8 +468,6 @@ mod tests { let vmm = init_and_run(cpu_id, ip, &insn, hashmap![], None); let eax = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::EAX) @@ -506,8 +487,6 @@ mod tests { let vmm = init_and_run(cpu_id, ip, &insn, hashmap![], None); let rax: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RAX) @@ -534,7 +513,7 @@ mod tests { ); let mut memory: [u8; 1] = [0; 1]; - vmm.lock().unwrap().read_memory(rax, &mut memory).unwrap(); + vmm.read_memory(rax, &mut memory).unwrap(); assert_eq!(u8::from_le_bytes(memory), dh); @@ -558,7 +537,7 @@ mod tests { ); let mut memory: [u8; 4] = [0; 4]; - vmm.lock().unwrap().read_memory(rax, &mut memory).unwrap(); + vmm.read_memory(rax, &mut memory).unwrap(); assert_eq!(u32::from_le_bytes(memory), esi); @@ -583,10 +562,7 @@ mod tests { ); let mut memory: [u8; 4] = [0; 4]; - vmm.lock() - .unwrap() - .read_memory(rax + displacement, &mut memory) - .unwrap(); + vmm.read_memory(rax + displacement, &mut memory).unwrap(); assert_eq!(u32::from_le_bytes(memory), edi); @@ -612,8 +588,6 @@ mod tests { ); let new_eax = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::EAX) @@ -642,8 +616,6 @@ mod tests { ); let new_al = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::AL) @@ -677,8 +649,6 @@ mod tests { ); let rbx: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RBX) @@ -713,14 +683,9 @@ mod tests { Some(1), ); - assert_eq!( - ip + 7 as u64, - vmm.lock().unwrap().cpu_state(cpu_id).unwrap().ip() - ); + assert_eq!(ip + 7 as u64, vmm.cpu_state(cpu_id).unwrap().ip()); let new_rax: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RAX) @@ -757,14 +722,9 @@ mod tests { Some(2), ); - assert_eq!( - ip + 7 + 4 as u64, - vmm.lock().unwrap().cpu_state(cpu_id).unwrap().ip() - ); + assert_eq!(ip + 7 + 4 as u64, vmm.cpu_state(cpu_id).unwrap().ip()); let rbx: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RBX) @@ -773,8 +733,6 @@ mod tests { // Check that rax is still at 0x100 let new_rax: u64 = vmm - .lock() - .unwrap() .cpu_state(cpu_id) .unwrap() .read_reg(Register::RAX) diff --git a/hypervisor/src/arch/x86/emulator/mod.rs b/hypervisor/src/arch/x86/emulator/mod.rs index 33e1bbece..8d544edbf 100644 --- a/hypervisor/src/arch/x86/emulator/mod.rs +++ b/hypervisor/src/arch/x86/emulator/mod.rs @@ -11,7 +11,6 @@ use crate::arch::x86::emulator::instructions::*; use crate::arch::x86::Exception; use crate::x86_64::{SegmentRegister, SpecialRegisters, StandardRegisters}; use iced_x86::*; -use std::sync::{Arc, Mutex}; #[macro_use] mod instructions; @@ -349,13 +348,13 @@ impl CpuStateManager for EmulatorCpuState { } } -pub struct Emulator { - platform: Arc>>, +pub struct Emulator<'a, T: CpuStateManager> { + platform: &'a mut dyn PlatformEmulator, insn_map: InstructionMap, } -impl Emulator { - pub fn new(platform: Arc>>) -> Emulator { +impl<'a, T: CpuStateManager> Emulator<'a, T> { + pub fn new(platform: &mut dyn PlatformEmulator) -> Emulator { let mut insn_map = InstructionMap::::new(); // MOV @@ -376,10 +375,7 @@ impl Emulator { insn_add!(insn_map, mov, Mov_rm64_imm32); insn_add!(insn_map, mov, Mov_rm64_r64); - Emulator { - platform: Arc::clone(&platform), - insn_map, - } + Emulator { platform, insn_map } } fn emulate_insn_stream( @@ -390,8 +386,6 @@ impl Emulator { ) -> EmulationResult { let mut state = self .platform - .lock() - .unwrap() .cpu_state(cpu_id) .map_err(EmulationError::PlatformEmulationError)?; let mut decoder = Decoder::new(64, insn_stream, DecoderOptions::NONE); @@ -404,7 +398,7 @@ impl Emulator { .ok_or_else(|| { EmulationError::UnsupportedInstruction(anyhow!("{:?}", insn.mnemonic())) })? - .emulate(&insn, &mut state, Arc::clone(&self.platform))?; + .emulate(&insn, &mut state, self.platform)?; if let Some(num_insn) = num_insn { if index + 1 >= num_insn {