From 8d9de7b5b1d7ee7a402f5acd98d297869809b582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 23 Apr 2024 11:20:12 +0200 Subject: [PATCH 1/6] sev/ghcb: introduce GHCB field getters and setters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When accessing GHCB fields that were written to by the host, we must check that they are set as valid in the valid bitmap. Conversely, when setting fields of the GHCB to communicate with the host, we must set them as valid in the valid bitmap. To do so, we must get the offset of such field in the GHCB and translate it into an index into the valid bitmap. Instead of hardcoding GHCB field offsets, implement getters and setters for each field, which will automatically get or set the appropriate value from the valid bitmap based on that field's offset. This was not done before because there was no way of getting a field's offset at compile time before Rust 1.77, which introduced the core::mem::offset_of!() macro. Signed-off-by: Carlos López --- kernel/src/sev/ghcb.rs | 236 ++++++++++++++++++----------------------- 1 file changed, 106 insertions(+), 130 deletions(-) diff --git a/kernel/src/sev/ghcb.rs b/kernel/src/sev/ghcb.rs index f4c5fc6b1..8c870b119 100644 --- a/kernel/src/sev/ghcb.rs +++ b/kernel/src/sev/ghcb.rs @@ -17,30 +17,12 @@ use crate::mm::virt_to_phys; use crate::sev::sev_snp_enabled; use crate::sev::utils::raw_vmgexit; use crate::types::{PageSize, PAGE_SIZE_2M}; -use core::{mem, ptr}; +use core::mem::{self, offset_of}; +use core::ptr; use super::msr_protocol::{invalidate_page_msr, register_ghcb_gpa_msr, validate_page_msr}; use super::{pvalidate, PvalidateOp}; -// TODO: Fix this when Rust gets decent compile time struct offset support -const OFF_CPL: u16 = 0xcb; -const OFF_XSS: u16 = 0x140; -const OFF_DR7: u16 = 0x160; -const OFF_RAX: u16 = 0x1f8; -const OFF_RCX: u16 = 0x308; -const OFF_RDX: u16 = 0x310; -const OFF_RBX: u16 = 0x318; -const OFF_SW_EXIT_CODE: u16 = 0x390; -const OFF_SW_EXIT_INFO_1: u16 = 0x398; -const OFF_SW_EXIT_INFO_2: u16 = 0x3a0; -const OFF_SW_SCRATCH: u16 = 0x3a8; -const OFF_XCR0: u16 = 0x3e8; -const OFF_VALID_BITMAP: u16 = 0x3f0; -const OFF_X87_STATE_GPA: u16 = 0x400; -const _OFF_BUFFER: u16 = 0x800; -const OFF_VERSION: u16 = 0xffa; -const OFF_USAGE: u16 = 0xffc; - #[repr(C, packed)] #[derive(Debug, Default, Clone, Copy)] pub struct PageStateChangeHeader { @@ -70,6 +52,27 @@ const PSC_FLAG_HUGE: u64 = 1 << PSC_FLAG_HUGE_SHIFT; const GHCB_BUFFER_SIZE: usize = 0x7f0; +macro_rules! ghcb_getter { + ($name:ident, $field:ident,$t:ty) => { + #[allow(unused)] + fn $name(&self) -> Result<$t, GhcbError> { + self.is_valid(offset_of!(Self, $field)) + .then_some(self.$field) + .ok_or(GhcbError::VmgexitInvalid) + } + }; +} + +macro_rules! ghcb_setter { + ($name:ident, $field:ident, $t:ty) => { + #[allow(unused)] + fn $name(&mut self, val: $t) { + self.$field = val; + self.set_valid(offset_of!(Self, $field)); + } + }; +} + #[repr(C, packed)] #[derive(Debug)] pub struct GHCB { @@ -139,6 +142,51 @@ pub enum GHCBIOSize { } impl GHCB { + ghcb_getter!(get_cpl_valid, cpl, u8); + ghcb_setter!(set_cpl_valid, cpl, u8); + + ghcb_getter!(get_xss_valid, xss, u64); + ghcb_setter!(set_xss_valid, xss, u64); + + ghcb_getter!(get_dr7_valid, dr7, u64); + ghcb_setter!(set_dr7_valid, dr7, u64); + + ghcb_getter!(get_rax_valid, rax, u64); + ghcb_setter!(set_rax_valid, rax, u64); + + ghcb_getter!(get_rcx_valid, rcx, u64); + ghcb_setter!(set_rcx_valid, rcx, u64); + + ghcb_getter!(get_rdx_valid, rdx, u64); + ghcb_setter!(set_rdx_valid, rdx, u64); + + ghcb_getter!(get_rbx_valid, rbx, u64); + ghcb_setter!(set_rbx_valid, rbx, u64); + + ghcb_getter!(get_exit_code_valid, sw_exit_code, u64); + ghcb_setter!(set_exit_code_valid, sw_exit_code, u64); + + ghcb_getter!(get_exit_info_1_valid, sw_exit_info_1, u64); + ghcb_setter!(set_exit_info_1_valid, sw_exit_info_1, u64); + + ghcb_getter!(get_exit_info_2_valid, sw_exit_info_2, u64); + ghcb_setter!(set_exit_info_2_valid, sw_exit_info_2, u64); + + ghcb_getter!(get_sw_scratch_valid, sw_scratch, u64); + ghcb_setter!(set_sw_scratch_valid, sw_scratch, u64); + + ghcb_getter!(get_sw_xcr0_valid, xcr0, u64); + ghcb_setter!(set_sw_xcr0_valid, xcr0, u64); + + ghcb_getter!(get_sw_x87_state_gpa_valid, x87_state_gpa, u64); + ghcb_setter!(set_sw_x87_state_gpa_valid, x87_state_gpa, u64); + + ghcb_getter!(get_version_valid, version, u16); + ghcb_setter!(set_version_valid, version, u16); + + ghcb_getter!(get_usage_valid, usage, u32); + ghcb_setter!(set_usage_valid, usage, u32); + pub fn init(vaddr: VirtAddr) -> Result<(), SvsmError> { let paddr = virt_to_phys(vaddr); @@ -166,9 +214,9 @@ impl GHCB { pub fn wrmsr_regs(&mut self, regs: &X86GeneralRegs) -> Result<(), SvsmError> { self.clear(); - self.set_rcx(regs.rcx as u64); - self.set_rax(regs.rax as u64); - self.set_rdx(regs.rdx as u64); + self.set_rcx_valid(regs.rcx as u64); + self.set_rax_valid(regs.rax as u64); + self.set_rdx_valid(regs.rdx as u64); self.vmgexit(GHCBExitCode::MSR, 1, 0)?; Ok(()) @@ -177,7 +225,7 @@ impl GHCB { pub fn rdmsr_regs(&mut self, regs: &mut X86GeneralRegs) -> Result<(), SvsmError> { self.clear(); - self.set_rcx(regs.rcx as u64); + self.set_rcx_valid(regs.rcx as u64); self.vmgexit(GHCBExitCode::MSR, 0, 0)?; regs.rdx = self.rdx as usize; @@ -219,25 +267,25 @@ impl GHCB { pub fn clear(&mut self) { // Clear valid bitmap - self.valid_bitmap[0] = 0; - self.valid_bitmap[1] = 0; + self.valid_bitmap = [0, 0]; // Mark valid_bitmap valid - self.set_valid(OFF_VALID_BITMAP); - self.set_valid(OFF_VALID_BITMAP + 8); + let off = offset_of!(Self, valid_bitmap); + self.set_valid(off); + self.set_valid(off + mem::size_of::()); } - fn set_valid(&mut self, offset: u16) { - let bit: usize = (offset as usize >> 3) & 0x3f; - let index: usize = (offset as usize >> 9) & 0x1; + fn set_valid(&mut self, offset: usize) { + let bit: usize = (offset >> 3) & 0x3f; + let index: usize = (offset >> 9) & 0x1; let mask: u64 = 1 << bit; self.valid_bitmap[index] |= mask; } - fn is_valid(&self, offset: u16) -> bool { - let bit: usize = (offset as usize >> 3) & 0x3f; - let index: usize = (offset as usize >> 9) & 0x1; + fn is_valid(&self, offset: usize) -> bool { + let bit: usize = (offset >> 3) & 0x3f; + let index: usize = (offset >> 9) & 0x1; let mask: u64 = 1 << bit; (self.valid_bitmap[index] & mask) == mask @@ -250,91 +298,26 @@ impl GHCB { exit_info_2: u64, ) -> Result<(), GhcbError> { // GHCB is version 2 - self.version = 2; - self.set_valid(OFF_VERSION); - + self.set_version_valid(2); // GHCB Follows standard format - self.usage = 0; - self.set_valid(OFF_USAGE); - - self.sw_exit_code = exit_code; - self.set_valid(OFF_SW_EXIT_CODE); - - self.sw_exit_info_1 = exit_info_1; - self.set_valid(OFF_SW_EXIT_INFO_1); - - self.sw_exit_info_2 = exit_info_2; - self.set_valid(OFF_SW_EXIT_INFO_2); + self.set_usage_valid(0); + self.set_exit_code_valid(exit_code); + self.set_exit_info_1_valid(exit_info_1); + self.set_exit_info_2_valid(exit_info_2); let ghcb_address = VirtAddr::from(self as *const GHCB); let ghcb_pa = u64::from(virt_to_phys(ghcb_address)); write_msr(SEV_GHCB, ghcb_pa); raw_vmgexit(); - if !self.is_valid(OFF_SW_EXIT_INFO_1) { - return Err(GhcbError::VmgexitInvalid); - } - - if self.sw_exit_info_1 != 0 { - return Err(GhcbError::VmgexitError( - self.sw_exit_info_1, - self.sw_exit_info_2, - )); + let sw_exit_info_1 = self.get_exit_info_1_valid()?; + if sw_exit_info_1 != 0 { + return Err(GhcbError::VmgexitError(sw_exit_info_1, self.sw_exit_info_2)); } Ok(()) } - pub fn set_cpl(&mut self, cpl: u8) { - self.cpl = cpl; - self.set_valid(OFF_CPL); - } - - pub fn set_dr7(&mut self, dr7: u64) { - self.dr7 = dr7; - self.set_valid(OFF_DR7); - } - - pub fn set_xss(&mut self, xss: u64) { - self.xss = xss; - self.set_valid(OFF_XSS); - } - - pub fn set_rax(&mut self, rax: u64) { - self.rax = rax; - self.set_valid(OFF_RAX); - } - - pub fn set_rcx(&mut self, rcx: u64) { - self.rcx = rcx; - self.set_valid(OFF_RCX); - } - - pub fn set_rdx(&mut self, rdx: u64) { - self.rdx = rdx; - self.set_valid(OFF_RDX); - } - - pub fn set_rbx(&mut self, rbx: u64) { - self.rbx = rbx; - self.set_valid(OFF_RBX); - } - - pub fn set_sw_scratch(&mut self, scratch: u64) { - self.sw_scratch = scratch; - self.set_valid(OFF_SW_SCRATCH); - } - - pub fn set_sw_xcr0(&mut self, xcr0: u64) { - self.xcr0 = xcr0; - self.set_valid(OFF_XCR0); - } - - pub fn set_sw_x87_state_gpa(&mut self, x87_state_gpa: u64) { - self.x87_state_gpa = x87_state_gpa; - self.set_valid(OFF_X87_STATE_GPA); - } - pub fn ioio_in(&mut self, port: u16, size: GHCBIOSize) -> Result { self.clear(); @@ -349,10 +332,8 @@ impl GHCB { } self.vmgexit(GHCBExitCode::IOIO, info, 0)?; - if !self.is_valid(OFF_RAX) { - return Err(GhcbError::VmgexitInvalid.into()); - } - Ok(self.rax) + let rax = self.get_rax_valid()?; + Ok(rax) } pub fn ioio_out(&mut self, port: u16, size: GHCBIOSize, value: u64) -> Result<(), SvsmError> { @@ -368,7 +349,7 @@ impl GHCB { GHCBIOSize::Size32 => info |= 1 << 6, } - self.set_rax(value); + self.set_rax_valid(value); self.vmgexit(GHCBExitCode::IOIO, info, 0)?; Ok(()) } @@ -462,11 +443,11 @@ impl GHCB { let buffer_va = VirtAddr::from(self.buffer.as_ptr()); let buffer_pa = u64::from(virt_to_phys(buffer_va)); - self.set_sw_scratch(buffer_pa); + self.set_sw_scratch_valid(buffer_pa); if let Err(mut e) = self.vmgexit(GHCBExitCode::SNP_PSC, 0, 0) { - if !self.is_valid(OFF_SW_EXIT_INFO_2) { - e = GhcbError::VmgexitInvalid; + if let Err(err) = self.get_exit_info_2_valid() { + e = err; } if let GhcbError::VmgexitError(_, info2) = e { @@ -498,7 +479,7 @@ impl GHCB { self.clear(); let exit_info_1: u64 = 1 | (vmpl & 0xf) << 16 | apic_id << 32; let exit_info_2: u64 = vmsa_gpa.into(); - self.set_rax(sev_features); + self.set_rax_valid(sev_features); self.vmgexit(GHCBExitCode::AP_CREATE, exit_info_1, exit_info_2)?; Ok(()) } @@ -513,7 +494,7 @@ impl GHCB { self.clear(); let exit_info_1: u64 = (vmpl & 0xf) << 16 | apic_id << 32; let exit_info_2: u64 = vmsa_gpa.into(); - self.set_rax(sev_features); + self.set_rax_valid(sev_features); self.vmgexit(GHCBExitCode::AP_CREATE, exit_info_1, exit_info_2)?; Ok(()) } @@ -530,12 +511,9 @@ impl GHCB { self.vmgexit(GHCBExitCode::GUEST_REQUEST, info1, info2)?; - if !self.is_valid(OFF_SW_EXIT_INFO_2) { - return Err(GhcbError::VmgexitInvalid.into()); - } - - if self.sw_exit_info_2 != 0 { - return Err(GhcbError::VmgexitError(self.sw_exit_info_1, self.sw_exit_info_2).into()); + let sw_exit_info_2 = self.get_exit_info_2_valid()?; + if sw_exit_info_2 != 0 { + return Err(GhcbError::VmgexitError(self.sw_exit_info_1, sw_exit_info_2).into()); } Ok(()) @@ -554,20 +532,18 @@ impl GHCB { let info2: u64 = u64::from(virt_to_phys(resp_page)); let rax: u64 = u64::from(virt_to_phys(data_pages)); - self.set_rax(rax); - self.set_rbx(data_size); + self.set_rax_valid(rax); + self.set_rbx_valid(data_size); self.vmgexit(GHCBExitCode::GUEST_EXT_REQUEST, info1, info2)?; - if !self.is_valid(OFF_SW_EXIT_INFO_2) { - return Err(GhcbError::VmgexitInvalid.into()); - } + let sw_exit_info_2 = self.get_exit_info_2_valid()?; // On error, RBX and exit_info_2 are returned for proper error handling. // For an extended request, if the buffer provided is too small, the hypervisor // will return in RBX the number of contiguous pages required - if self.sw_exit_info_2 != 0 { - return Err(GhcbError::VmgexitError(self.rbx, self.sw_exit_info_2).into()); + if sw_exit_info_2 != 0 { + return Err(GhcbError::VmgexitError(self.rbx, sw_exit_info_2).into()); } Ok(()) From 031358825f19058c46e1f73f84e9c152fade0338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 23 Apr 2024 11:32:13 +0200 Subject: [PATCH 2/6] sev/ghcb: verify RDX and RAX are valid before reading RDMSR results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When performing a VMGEXIT to request a RDMSR we expect the instruction results to be placed in RDX and RAX. However, we must check that the hypervisor set those registers as valid before reading them. To do so, use the recently introduced getters. Signed-off-by: Carlos López --- kernel/src/sev/ghcb.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/src/sev/ghcb.rs b/kernel/src/sev/ghcb.rs index 8c870b119..011f9acd1 100644 --- a/kernel/src/sev/ghcb.rs +++ b/kernel/src/sev/ghcb.rs @@ -228,8 +228,10 @@ impl GHCB { self.set_rcx_valid(regs.rcx as u64); self.vmgexit(GHCBExitCode::MSR, 0, 0)?; - regs.rdx = self.rdx as usize; - regs.rax = self.rax as usize; + let rdx = self.get_rdx_valid()?; + let rax = self.get_rax_valid()?; + regs.rdx = rdx as usize; + regs.rax = rax as usize; Ok(()) } From 9062ee4c20fde289e390fd32a1937f99cf05672d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 23 Apr 2024 11:26:00 +0200 Subject: [PATCH 3/6] cpu/vc: implement RDTSC #VC handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement a the #VC handler for the SVM_RDTSC exit reason. This also requires decoding the RDTSC instruction. The handler simply forwards the call into the GHCB and copies out the relevant registers to the guest state. While we are at it, enable the relevant RDTSC #VC test. Signed-off-by: Carlos López --- kernel/src/cpu/insn.rs | 3 +++ kernel/src/cpu/vc.rs | 14 ++++++++------ kernel/src/sev/ghcb.rs | 11 +++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/kernel/src/cpu/insn.rs b/kernel/src/cpu/insn.rs index 975ad61d8..e50b5cb6f 100644 --- a/kernel/src/cpu/insn.rs +++ b/kernel/src/cpu/insn.rs @@ -60,6 +60,7 @@ pub enum DecodedInsn { Outw(Operand), Wrmsr, Rdmsr, + Rdtsc, } impl DecodedInsn { @@ -79,6 +80,7 @@ impl DecodedInsn { Self::Outw(Operand::Imm(..)) => 3, Self::Outl(Operand::Imm(..)) => 2, Self::Wrmsr | Self::Rdmsr => 2, + Self::Rdtsc => 2, } } } @@ -122,6 +124,7 @@ impl Instruction { }, 0x0F => match self.0[1] { 0x30 => return Ok(DecodedInsn::Wrmsr), + 0x31 => return Ok(DecodedInsn::Rdtsc), 0x32 => return Ok(DecodedInsn::Rdmsr), 0xA2 => return Ok(DecodedInsn::Cpuid), _ => (), diff --git a/kernel/src/cpu/vc.rs b/kernel/src/cpu/vc.rs index b3f5616de..ce85636f5 100644 --- a/kernel/src/cpu/vc.rs +++ b/kernel/src/cpu/vc.rs @@ -21,6 +21,7 @@ use core::fmt; pub const SVM_EXIT_EXCP_BASE: usize = 0x40; pub const SVM_EXIT_LAST_EXCP: usize = 0x5f; +pub const SVM_EXIT_RDTSC: usize = 0x6e; pub const SVM_EXIT_CPUID: usize = 0x72; pub const SVM_EXIT_IOIO: usize = 0x7b; pub const SVM_EXIT_MSR: usize = 0x7c; @@ -98,8 +99,8 @@ pub fn stage2_handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), S let err = ctx.error_code; // To handle NAE events, we're supposed to reset the VALID_BITMAP field of the GHCB. - // This is currently only relevant for IOIO handling. This field is currently reset in - // the ioio_{in,ou} methods but it would be better to move the reset out of the different + // This is currently only relevant for IOIO and RDTSC handling. This field is currently reset in + // the relevant GHCB methods but it would be better to move the reset out of the different // handlers. let mut ghcb = current_ghcb(); @@ -109,6 +110,7 @@ pub fn stage2_handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), S (SVM_EXIT_CPUID, Some(DecodedInsn::Cpuid)) => handle_cpuid(ctx), (SVM_EXIT_IOIO, Some(ins)) => handle_ioio(ctx, &mut ghcb, ins), (SVM_EXIT_MSR, Some(ins)) => handle_msr(ctx, &mut ghcb, ins), + (SVM_EXIT_RDTSC, Some(DecodedInsn::Rdtsc)) => ghcb.rdtsc_regs(&mut ctx.regs), _ => Err(VcError::new(ctx, VcErrorType::Unsupported).into()), }?; @@ -120,8 +122,8 @@ pub fn handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), SvsmErro let error_code = ctx.error_code; // To handle NAE events, we're supposed to reset the VALID_BITMAP field of the GHCB. - // This is currently only relevant for IOIO handling. This field is currently reset in - // the ioio_{in,ou} methods but it would be better to move the reset out of the different + // This is currently only relevant for IOIO and RDTSC handling. This field is currently reset in + // the relevant GHCB methods but it would be better to move the reset out of the different // handlers. let mut ghcb = current_ghcb(); @@ -138,6 +140,7 @@ pub fn handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), SvsmErro (SVM_EXIT_CPUID, Some(DecodedInsn::Cpuid)) => handle_cpuid(ctx), (SVM_EXIT_IOIO, Some(ins)) => handle_ioio(ctx, &mut ghcb, ins), (SVM_EXIT_MSR, Some(ins)) => handle_msr(ctx, &mut ghcb, ins), + (SVM_EXIT_RDTSC, Some(DecodedInsn::Rdtsc)) => ghcb.rdtsc_regs(&mut ctx.regs), _ => Err(VcError::new(ctx, VcErrorType::Unsupported).into()), }?; @@ -584,8 +587,7 @@ mod tests { } #[test] - // #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")] - #[ignore = "Currently unhandled by #VC handler"] + #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")] fn test_rdtsc() { let mut prev: u64 = rdtsc(); for _ in 0..50 { diff --git a/kernel/src/sev/ghcb.rs b/kernel/src/sev/ghcb.rs index 011f9acd1..fcedbf03d 100644 --- a/kernel/src/sev/ghcb.rs +++ b/kernel/src/sev/ghcb.rs @@ -125,6 +125,7 @@ impl From for SvsmError { enum GHCBExitCode {} impl GHCBExitCode { + pub const RDTSC: u64 = 0x6e; pub const IOIO: u64 = 0x7b; pub const MSR: u64 = 0x7c; pub const SNP_PSC: u64 = 0x8000_0010; @@ -211,6 +212,16 @@ impl GHCB { Ok(()) } + pub fn rdtsc_regs(&mut self, regs: &mut X86GeneralRegs) -> Result<(), SvsmError> { + self.clear(); + self.vmgexit(GHCBExitCode::RDTSC, 0, 0)?; + let rax = self.get_rax_valid()?; + let rdx = self.get_rdx_valid()?; + regs.rax = rax as usize; + regs.rdx = rdx as usize; + Ok(()) + } + pub fn wrmsr_regs(&mut self, regs: &X86GeneralRegs) -> Result<(), SvsmError> { self.clear(); From b68887097c564c56d86004d0458533a23900378e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 23 Apr 2024 11:47:41 +0200 Subject: [PATCH 4/6] cpu/msr: fix rdtscp() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to a typo, this function was issuing a RDTSC instruction instead of RDTSCP. Signed-off-by: Carlos López --- kernel/src/cpu/msr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/src/cpu/msr.rs b/kernel/src/cpu/msr.rs index 3475a3034..64dfbc1e3 100644 --- a/kernel/src/cpu/msr.rs +++ b/kernel/src/cpu/msr.rs @@ -63,7 +63,7 @@ pub fn rdtscp() -> RdtscpOut { let ecx: u32; unsafe { - asm!("rdtsc", + asm!("rdtscp", out("eax") eax, out("ecx") ecx, out("edx") edx, From 3b12be497555464fce3a46b4dcffad66289360b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 23 Apr 2024 11:49:53 +0200 Subject: [PATCH 5/6] cpu/vc: implement RDTSCP #VC handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement a the #VC handler for the SVM_RDTSCP exit reason. This also requires decoding the RDTSCP instruction. The handler simply forwards the call into the GHCB and copies out the relevant registers to the guest state. While we are at it, enable the relevant RDTSCP #VC test. Signed-off-by: Carlos López --- kernel/src/cpu/insn.rs | 7 +++++++ kernel/src/cpu/vc.rs | 20 ++++++++++++-------- kernel/src/sev/ghcb.rs | 13 +++++++++++++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/kernel/src/cpu/insn.rs b/kernel/src/cpu/insn.rs index e50b5cb6f..4436e5e75 100644 --- a/kernel/src/cpu/insn.rs +++ b/kernel/src/cpu/insn.rs @@ -61,6 +61,7 @@ pub enum DecodedInsn { Wrmsr, Rdmsr, Rdtsc, + Rdtscp, } impl DecodedInsn { @@ -81,6 +82,7 @@ impl DecodedInsn { Self::Outl(Operand::Imm(..)) => 2, Self::Wrmsr | Self::Rdmsr => 2, Self::Rdtsc => 2, + Self::Rdtscp => 3, } } } @@ -123,6 +125,11 @@ impl Instruction { _ => (), }, 0x0F => match self.0[1] { + 0x01 => { + if self.0[2] == 0xf9 { + return Ok(DecodedInsn::Rdtscp); + } + } 0x30 => return Ok(DecodedInsn::Wrmsr), 0x31 => return Ok(DecodedInsn::Rdtsc), 0x32 => return Ok(DecodedInsn::Rdmsr), diff --git a/kernel/src/cpu/vc.rs b/kernel/src/cpu/vc.rs index ce85636f5..169f2f679 100644 --- a/kernel/src/cpu/vc.rs +++ b/kernel/src/cpu/vc.rs @@ -25,6 +25,7 @@ pub const SVM_EXIT_RDTSC: usize = 0x6e; pub const SVM_EXIT_CPUID: usize = 0x72; pub const SVM_EXIT_IOIO: usize = 0x7b; pub const SVM_EXIT_MSR: usize = 0x7c; +pub const SVM_EXIT_RDTSCP: usize = 0x87; pub const X86_TRAP_DB: usize = 0x01; pub const X86_TRAP: usize = SVM_EXIT_EXCP_BASE + X86_TRAP_DB; @@ -98,9 +99,10 @@ pub fn stage2_handle_vc_exception_no_ghcb(ctx: &mut X86ExceptionContext) -> Resu pub fn stage2_handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), SvsmError> { let err = ctx.error_code; - // To handle NAE events, we're supposed to reset the VALID_BITMAP field of the GHCB. - // This is currently only relevant for IOIO and RDTSC handling. This field is currently reset in - // the relevant GHCB methods but it would be better to move the reset out of the different + // To handle NAE events, we're supposed to reset the VALID_BITMAP field of + // the GHCB. This is currently only relevant for IOIO, RDTSC and RDTSCP + // handling. This field is currently reset in the relevant GHCB methods + // but it would be better to move the reset out of the different // handlers. let mut ghcb = current_ghcb(); @@ -111,6 +113,7 @@ pub fn stage2_handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), S (SVM_EXIT_IOIO, Some(ins)) => handle_ioio(ctx, &mut ghcb, ins), (SVM_EXIT_MSR, Some(ins)) => handle_msr(ctx, &mut ghcb, ins), (SVM_EXIT_RDTSC, Some(DecodedInsn::Rdtsc)) => ghcb.rdtsc_regs(&mut ctx.regs), + (SVM_EXIT_RDTSCP, Some(DecodedInsn::Rdtsc)) => ghcb.rdtscp_regs(&mut ctx.regs), _ => Err(VcError::new(ctx, VcErrorType::Unsupported).into()), }?; @@ -121,9 +124,10 @@ pub fn stage2_handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), S pub fn handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), SvsmError> { let error_code = ctx.error_code; - // To handle NAE events, we're supposed to reset the VALID_BITMAP field of the GHCB. - // This is currently only relevant for IOIO and RDTSC handling. This field is currently reset in - // the relevant GHCB methods but it would be better to move the reset out of the different + // To handle NAE events, we're supposed to reset the VALID_BITMAP field of + // the GHCB. This is currently only relevant for IOIO, RDTSC and RDTSCP + // handling. This field is currently reset in the relevant GHCB methods + // but it would be better to move the reset out of the different // handlers. let mut ghcb = current_ghcb(); @@ -141,6 +145,7 @@ pub fn handle_vc_exception(ctx: &mut X86ExceptionContext) -> Result<(), SvsmErro (SVM_EXIT_IOIO, Some(ins)) => handle_ioio(ctx, &mut ghcb, ins), (SVM_EXIT_MSR, Some(ins)) => handle_msr(ctx, &mut ghcb, ins), (SVM_EXIT_RDTSC, Some(DecodedInsn::Rdtsc)) => ghcb.rdtsc_regs(&mut ctx.regs), + (SVM_EXIT_RDTSCP, Some(DecodedInsn::Rdtsc)) => ghcb.rdtscp_regs(&mut ctx.regs), _ => Err(VcError::new(ctx, VcErrorType::Unsupported).into()), }?; @@ -598,8 +603,7 @@ mod tests { } #[test] - // #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")] - #[ignore = "Currently unhandled by #VC handler"] + #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")] fn test_rdtscp() { let expected_pid = u32::try_from(verify_ghcb_gets_altered(|| read_msr(MSR_TSC_AUX))) .expect("pid should be 32 bits"); diff --git a/kernel/src/sev/ghcb.rs b/kernel/src/sev/ghcb.rs index fcedbf03d..c9de4a53e 100644 --- a/kernel/src/sev/ghcb.rs +++ b/kernel/src/sev/ghcb.rs @@ -128,6 +128,7 @@ impl GHCBExitCode { pub const RDTSC: u64 = 0x6e; pub const IOIO: u64 = 0x7b; pub const MSR: u64 = 0x7c; + pub const RDTSCP: u64 = 0x87; pub const SNP_PSC: u64 = 0x8000_0010; pub const GUEST_REQUEST: u64 = 0x8000_0011; pub const GUEST_EXT_REQUEST: u64 = 0x8000_0012; @@ -212,6 +213,18 @@ impl GHCB { Ok(()) } + pub fn rdtscp_regs(&mut self, regs: &mut X86GeneralRegs) -> Result<(), SvsmError> { + self.clear(); + self.vmgexit(GHCBExitCode::RDTSCP, 0, 0)?; + let rax = self.get_rax_valid()?; + let rdx = self.get_rdx_valid()?; + let rcx = self.get_rcx_valid()?; + regs.rax = rax as usize; + regs.rdx = rdx as usize; + regs.rcx = rcx as usize; + Ok(()) + } + pub fn rdtsc_regs(&mut self, regs: &mut X86GeneralRegs) -> Result<(), SvsmError> { self.clear(); self.vmgexit(GHCBExitCode::RDTSC, 0, 0)?; From 447e57800cd0e2c3bb75e4d9cd502f463c3ecdcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 23 Apr 2024 13:25:08 +0200 Subject: [PATCH 6/6] sev/ghcb: add GHCB layout test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a test to verify that the GHCB layout is as expected, as per the GHCB specification: https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf Signed-off-by: Carlos López --- kernel/src/sev/ghcb.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/kernel/src/sev/ghcb.rs b/kernel/src/sev/ghcb.rs index c9de4a53e..67e4f221b 100644 --- a/kernel/src/sev/ghcb.rs +++ b/kernel/src/sev/ghcb.rs @@ -581,3 +581,30 @@ impl GHCB { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_ghcb_layout() { + assert_eq!(offset_of!(GHCB, cpl), 0x0cb); + assert_eq!(offset_of!(GHCB, xss), 0x140); + assert_eq!(offset_of!(GHCB, dr7), 0x160); + assert_eq!(offset_of!(GHCB, rax), 0x1f8); + assert_eq!(offset_of!(GHCB, rcx), 0x308); + assert_eq!(offset_of!(GHCB, rdx), 0x310); + assert_eq!(offset_of!(GHCB, rbx), 0x318); + assert_eq!(offset_of!(GHCB, sw_exit_code), 0x390); + assert_eq!(offset_of!(GHCB, sw_exit_info_1), 0x398); + assert_eq!(offset_of!(GHCB, sw_exit_info_2), 0x3a0); + assert_eq!(offset_of!(GHCB, sw_scratch), 0x3a8); + assert_eq!(offset_of!(GHCB, xcr0), 0x3e8); + assert_eq!(offset_of!(GHCB, valid_bitmap), 0x3f0); + assert_eq!(offset_of!(GHCB, x87_state_gpa), 0x400); + assert_eq!(offset_of!(GHCB, buffer), 0x800); + assert_eq!(offset_of!(GHCB, version), 0xffa); + assert_eq!(offset_of!(GHCB, usage), 0xffc); + assert_eq!(mem::size_of::(), 0x1000); + } +}