From 80af84ad625891146c9ed003c5b73bd8981f45bf Mon Sep 17 00:00:00 2001 From: Adam Dunlap Date: Thu, 12 Oct 2023 15:21:13 -0700 Subject: [PATCH] cpu/vc Add #VC handler tests Adds tests that make sure a #VC exception is raised while executing various instructions and that the #VC handler properly handles them via the ghcb. Adds tests for cpuid, msrs, port IO, MMIO, wbinvd, rdtsc, dr7, and vmmcall. Relies on being able to run tests inside the guest VM, e.g. with PR #120. The #VC handler is not yet implemented, so these tests are all marked with #[should_panic]. See PR #55 for progress on its implementation. TODO: Remove added dependencies Move the tests to vc.rs or a new file Clean up comments, etc. Move assembly wrapper helper functions to more sensible places Implement page state change (PSC) test Signed-off-by: Adam Dunlap --- Cargo.lock | 37 +++++ Cargo.toml | 2 + src/cpu/msr.rs | 23 ++++ src/sev/ghcb.rs | 352 ++++++++++++++++++++++++++++++++++++++++++++++- src/sev/utils.rs | 26 ++++ 5 files changed, 438 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ce56a400..a67e411f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "bit_field" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc827186963e592360843fb5ba4b973e145841266c1357f7180c43526f2e5b61" + [[package]] name = "bitflags" version = "1.3.2" @@ -127,6 +133,24 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "raw-cpuid" +version = "10.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c297679cb867470fa8c9f67dbba74a78d78e3e98d7cf2b08d6d71540f797332" +dependencies = [ + "bitflags 1.3.2", +] + +[[package]] +name = "raw-cpuid" +version = "11.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d86a7c4638d42c44551f4791a20e687dbb4c3de1f33c43dd71e355cd429def1" +dependencies = [ + "bitflags 2.4.0", +] + [[package]] name = "svsm" version = "0.1.0" @@ -137,7 +161,9 @@ dependencies = [ "intrusive-collections", "log", "packit", + "raw-cpuid 11.0.1", "test", + "x86", ] [[package]] @@ -161,6 +187,17 @@ version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22049a19f4a68748a168c0fc439f9516686aa045927ff767eca0a85101fb6e73" +[[package]] +name = "x86" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2781db97787217ad2a2845c396a5efe286f87467a5810836db6d74926e94a385" +dependencies = [ + "bit_field", + "bitflags 1.3.2", + "raw-cpuid 10.7.0", +] + [[package]] name = "zerocopy" version = "0.6.1" diff --git a/Cargo.toml b/Cargo.toml index 568ceed01..1d1069a32 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,8 @@ gdbstub_arch = { version = "0.2.4", optional = true } intrusive-collections = "0.9.6" log = { version = "0.4.17", features = ["max_level_info", "release_max_level_info"] } packit = { git = "https://github.com/coconut-svsm/packit", version = "0.1.0" } +x86 = "0.52.0" +raw-cpuid = "11.0" [target."x86_64-unknown-none".dev-dependencies] test = { version = "0.1.0", path = "test" } diff --git a/src/cpu/msr.rs b/src/cpu/msr.rs index 6ec7264e4..bfab05b3f 100644 --- a/src/cpu/msr.rs +++ b/src/cpu/msr.rs @@ -51,6 +51,29 @@ pub fn rdtsc() -> u64 { (eax as u64) | (edx as u64) << 32 } +pub struct RdtscpOut { + pub timestamp: u64, + pub pid: u32, +} + +pub fn rdtscp() -> RdtscpOut { + let eax: u32; + let edx: u32; + let ecx: u32; + + unsafe { + asm!("rdtsc", + out("eax") eax, + out("ecx") ecx, + out("edx") edx, + options(att_syntax, nomem, nostack)); + } + RdtscpOut { + timestamp: (eax as u64) | (edx as u64) << 32, + pid: ecx, + } +} + pub fn read_flags() -> u64 { let rax: u64; unsafe { diff --git a/src/sev/ghcb.rs b/src/sev/ghcb.rs index 470c7331a..8136753d5 100644 --- a/src/sev/ghcb.rs +++ b/src/sev/ghcb.rs @@ -3,10 +3,12 @@ // Copyright (c) 2022-2023 SUSE LLC // // Author: Joerg Roedel +#![allow(dead_code, unused_imports, unused_macros)] use crate::address::{Address, PhysAddr, VirtAddr}; use crate::cpu::flush_tlb_global_sync; -use crate::cpu::msr::{write_msr, SEV_GHCB}; +use crate::cpu::msr::{rdtsc, rdtscp, write_msr, RdtscpOut, SEV_GHCB}; +use crate::cpu::percpu::this_cpu_mut; use crate::error::SvsmError; use crate::mm::pagetable::get_init_pgtable_locked; use crate::mm::validate::{ @@ -14,9 +16,13 @@ use crate::mm::validate::{ }; use crate::mm::virt_to_phys; use crate::sev::sev_snp_enabled; -use crate::sev::utils::raw_vmgexit; +use crate::sev::utils::{get_dr7, raw_vmgexit, raw_vmmcall, set_dr7}; use crate::types::{PageSize, PAGE_SIZE_2M}; +use core::arch::asm; +use core::cell::RefCell; use core::{mem, ptr}; +use raw_cpuid::CpuId; +use x86::msr; use super::msr_protocol::{invalidate_page_msr, register_ghcb_gpa_msr, validate_page_msr}; use super::{pvalidate, PvalidateOp}; @@ -482,3 +488,345 @@ impl GHCB { Ok(()) } } + +// const CPUID_APID_ID: u32 = 0x1; +// const CPUID_EXTENDED_FEATURE: u32 = 0x80000001; +// const CPUID_PROCESSOR_NAME1: u32 = 0x80000002; +// const CPUID_PROCESSOR_NAME2: u32 = 0x80000003; +// const CPUID_PROCESSOR_NAME3: u32 = 0x80000004; +const CPUID_FN_LARGEST_EXT_FUNC_NUM: u32 = 0x80000000; +const CPUID_FN_ENCRYPT_MEM_CAPAB: u32 = 0x8000001f; +const MSR_SEV_STATUS_SUPPORT_MASK: u32 = 0b10; +// const MSR_SVM_SUPPORT_MASK: u32 = 0b100; + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_sev_snp_enablement_cpuid() { + let cpuid = CpuId::new(); + let mem_encrypt_info = cpuid + .get_memory_encryption_info() + .expect("Expected cpuid to have memory encryption leaf"); + + assert!(mem_encrypt_info.has_sev_snp()); +} + +const MSR_SEV_STATUS: u32 = 0b10; +const MSR_SEV_STATUS_SEV_SNP_ENABLED: u64 = 0b100; +const GHCB_FILL_TEST_VALUE: u8 = b'1'; + +fn fill_ghcb_with_test_data() { + let ghcb = this_cpu_mut().ghcb(); + unsafe { + // The count param is 1 to only write one ghcb's worth of data + core::ptr::write_bytes(ghcb as *mut GHCB, GHCB_FILL_TEST_VALUE, 1); + } +} +fn verify_ghcb_was_altered() { + let ghcb = this_cpu_mut().ghcb(); + let ghcb_bytes: &[u8; core::mem::size_of::()] = unsafe { core::mem::transmute(ghcb) }; + let changed_byte = ghcb_bytes.iter().find(|&&v| v != GHCB_FILL_TEST_VALUE); + assert!(changed_byte.is_some()); +} + +// Wraps an expression with an assertion that the expression ended up altering the ghcb. +macro_rules! verify_ghcb_gets_altered { + ($nae_func:expr) => {{ + fill_ghcb_with_test_data(); + let result = $nae_func; + verify_ghcb_was_altered(); + result + }}; +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_sev_snp_enablement_msr() { + let sev_status = verify_ghcb_gets_altered!(unsafe { msr::rdmsr(MSR_SEV_STATUS) }); + assert_ne!(sev_status & MSR_SEV_STATUS_SEV_SNP_ENABLED, 0); +} + +// TODO: PSC test + +// cpuid processor name -- probably unnecessary? + +const APIC_DEFAULT_PHYS_BASE: u64 = 0xfee00000; // KVM's default +const APIC_BASE_PHYS_ADDR_MASK: u64 = 0xffffff000; // bit 12-35 + +unsafe fn vmmcall() {} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_rdmsr_apic() { + let apic_base = verify_ghcb_gets_altered!(unsafe { msr::rdmsr(msr::APIC_BASE) }); + assert_eq!(apic_base & APIC_BASE_PHYS_ADDR_MASK, APIC_DEFAULT_PHYS_BASE); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_rdmsr_debug_ctl() { + let apic_base = verify_ghcb_gets_altered!(unsafe { msr::rdmsr(msr::DEBUGCTLMSR) }); + assert_eq!(apic_base, 0); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_wrmsr_tsc_aux() { + let test_val = 0x1234; + verify_ghcb_gets_altered!(unsafe { msr::wrmsr(msr::IA32_TSC_AUX, test_val) }); + let readback = verify_ghcb_gets_altered!(unsafe { msr::rdmsr(msr::IA32_TSC_AUX) }); + assert_eq!(test_val, readback); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_vmmcall_error() { + let res = verify_ghcb_gets_altered!(unsafe { raw_vmmcall(1005, 0, 0, 0) }); + assert_eq!(res, -1000); +} + +const VMMCALL_HC_VAPIC_POLL_IRQ: u32 = 1; + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_vmmcall_vapic_poll_irq() { + let res = verify_ghcb_gets_altered!(unsafe { raw_vmmcall(VMMCALL_HC_VAPIC_POLL_IRQ, 0, 0, 0) }); + assert_eq!(res, 0); +} + +const DR7_DEFAULT: u64 = 0x400; +const DR7_TEST: u64 = 0x401; + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_read_write_dr7() { + let old_dr7 = verify_ghcb_gets_altered!(unsafe { get_dr7() }); + assert_eq!(old_dr7, DR7_DEFAULT); + + verify_ghcb_gets_altered!(unsafe { set_dr7(DR7_TEST) }); + let new_dr7 = verify_ghcb_gets_altered!(unsafe { get_dr7() }); + assert_eq!(new_dr7, DR7_TEST); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_rdtsc() { + let mut prev: u64 = rdtsc(); + for _ in 0..50 { + let cur = rdtsc(); + assert!(cur > prev); + prev = cur; + } +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_rdtscp() { + let expected_pid = u32::try_from(verify_ghcb_gets_altered!(unsafe { + msr::rdmsr(msr::IA32_TSC_AUX) + })) + .expect("pid should be 32 bits"); + let RdtscpOut { + timestamp: mut prev, + pid, + } = rdtscp(); + assert_eq!(pid, expected_pid); + for _ in 0..50 { + let RdtscpOut { + timestamp: cur, + pid, + } = rdtscp(); + assert_eq!(pid, expected_pid); + assert!(cur > prev); + prev = cur; + } +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_wbinvd() { + verify_ghcb_gets_altered!(unsafe { + asm!("wbinvd"); + }); +} + +const APIC_DEFAULT_VERSION_REGISTER_OFFSET: u64 = 0x30; +const EXPECTED_APIC_VERSION_NUMBER: u32 = 0x50014; + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_mmio_apic_version() { + let version: u32; + let address = i32::try_from(APIC_DEFAULT_PHYS_BASE + APIC_DEFAULT_VERSION_REGISTER_OFFSET) + .expect("APIC address should fit in 32 bits"); + verify_ghcb_gets_altered!(unsafe { + asm!( + "mov (%edx), %eax", + out("eax") version, + in("edx") address, + options(att_syntax) + ) + }); + assert_eq!(version, EXPECTED_APIC_VERSION_NUMBER); +} + +const TESTDEV_ECHO_LAST_PORT: u16 = 0xe0; +const TESTDEV_ECHO_STREAM_PORT: u16 = 0xf0; + +fn inb(port: u16) -> u8 { + unsafe { + let ret: u8; + asm!("inb %dx, %al", in("dx") port, out("al") ret, options(att_syntax)); + ret + } +} +fn inb_from_testdev_echo() -> u8 { + unsafe { + let ret: u8; + asm!("inb $0xe0, %al", out("al") ret, options(att_syntax)); + ret + } +} + +fn outb(port: u16, value: u8) { + unsafe { asm!("outb %al, %dx", in("al") value, in("dx") port, options(att_syntax)) } +} + +fn outb_to_testdev_echo(value: u8) { + unsafe { asm!("outb %al, $0xe0", in("al") value, options(att_syntax)) } +} + +fn inw(port: u16) -> u16 { + unsafe { + let ret: u16; + asm!("inw %dx, %ax", in("dx") port, out("ax") ret, options(att_syntax)); + ret + } +} +fn inw_from_testdev_echo() -> u16 { + unsafe { + let ret: u16; + asm!("inw $0xe0, %ax", out("ax") ret, options(att_syntax)); + ret + } +} + +fn outw(port: u16, value: u16) { + unsafe { asm!("outw %ax, %dx", in("ax") value, in("dx") port, options(att_syntax)) } +} + +fn outw_to_testdev_echo(value: u16) { + unsafe { asm!("outw %ax, $0xe0", in("ax") value, options(att_syntax)) } +} + +fn inl(port: u16) -> u32 { + unsafe { + let ret: u32; + asm!("inl %dx, %ax", in("dx") port, out("eax") ret, options(att_syntax)); + ret + } +} +fn inl_from_testdev_echo() -> u32 { + unsafe { + let ret: u32; + asm!("inl $0xe0, %eax", out("eax") ret, options(att_syntax)); + ret + } +} + +fn outl(port: u16, value: u32) { + unsafe { asm!("outl %eax, %dx", in("eax") value, in("dx") port, options(att_syntax)) } +} + +fn outl_to_testdev_echo(value: u32) { + unsafe { asm!("outl %eax, $0xe0", in("eax") value, options(att_syntax)) } +} + +fn rep_outsw(port: u16, data: &[u16]) { + unsafe { + asm!("rep outsw", in("dx") port, in("rsi") data.as_ptr(), in("rcx") data.len(), options(att_syntax)) + } +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_8() { + const TEST_VAL: u8 = 0x12; + verify_ghcb_gets_altered!(outb(TESTDEV_ECHO_LAST_PORT, TEST_VAL)); + assert_eq!( + TEST_VAL, + verify_ghcb_gets_altered!(inb(TESTDEV_ECHO_LAST_PORT)) + ); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_16() { + const TEST_VAL: u16 = 0x4321; + verify_ghcb_gets_altered!(outw(TESTDEV_ECHO_LAST_PORT, TEST_VAL)); + assert_eq!( + TEST_VAL, + verify_ghcb_gets_altered!(inw(TESTDEV_ECHO_LAST_PORT)) + ); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_32() { + const TEST_VAL: u32 = 0xabcd1234; + verify_ghcb_gets_altered!(outl_to_testdev_echo(TEST_VAL)); + assert_eq!(TEST_VAL, verify_ghcb_gets_altered!(inl_from_testdev_echo())); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_8_hardcoded() { + const TEST_VAL: u8 = 0x12; + verify_ghcb_gets_altered!(outb_to_testdev_echo(TEST_VAL)); + assert_eq!(TEST_VAL, verify_ghcb_gets_altered!(inb_from_testdev_echo())); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_16_hardcoded() { + const TEST_VAL: u16 = 0x4321; + verify_ghcb_gets_altered!(outw_to_testdev_echo(TEST_VAL)); + assert_eq!(TEST_VAL, verify_ghcb_gets_altered!(inw_from_testdev_echo())); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_32_hardcoded() { + const TEST_VAL: u32 = 0xabcd1234; + verify_ghcb_gets_altered!(outl_to_testdev_echo(TEST_VAL)); + assert_eq!(TEST_VAL, verify_ghcb_gets_altered!(inl_from_testdev_echo())); +} + +#[test] +#[cfg_attr(not(target_os = "none"), ignore = "Can only be run inside guest")] +#[should_panic] // #VC handler is not yet implemented, so this should fail +fn test_port_io_string_16_get_last() { + const TEST_DATA: &[u16] = &[0x1234, 0x5678, 0x9abc, 0xdef0]; + verify_ghcb_gets_altered!(rep_outsw(TESTDEV_ECHO_LAST_PORT, TEST_DATA)); + assert_eq!( + TEST_DATA.last().unwrap(), + &verify_ghcb_gets_altered!(inw(TESTDEV_ECHO_LAST_PORT)) + ); +} diff --git a/src/sev/utils.rs b/src/sev/utils.rs index 97c446373..d0b146f59 100644 --- a/src/sev/utils.rs +++ b/src/sev/utils.rs @@ -134,6 +134,32 @@ pub fn pvalidate(vaddr: VirtAddr, size: PageSize, valid: PvalidateOp) -> Result< } } +pub unsafe fn raw_vmmcall(eax: u32, ebx: u32, ecx: u32, edx: u32) -> i32 { + let new_eax; + asm!( + // bx register is reserved by llvm so it can't be passed in directly and must be + // restored + "xchg %rbx, {0:r}", + "vmmcall", + "xchg %rbx, {0:r}", + in(reg) ebx as u64, + inout("eax") eax => new_eax, + in("ecx") ecx, + in("edx") edx, + options(att_syntax)); + new_eax +} + +pub unsafe fn set_dr7(new_val: u64) { + asm!("mov {0}, %dr7", in(reg) new_val, options(att_syntax)); +} + +pub unsafe fn get_dr7() -> u64 { + let out; + asm!("mov %dr7, {0}", out(reg) out, options(att_syntax)); + out +} + pub fn raw_vmgexit() { unsafe { asm!("rep; vmmcall", options(att_syntax));