From 150ffe1973aad7bd5a87bcef69ae38c01694eeb6 Mon Sep 17 00:00:00 2001 From: Jon Lange Date: Sat, 12 Oct 2024 19:53:25 -0700 Subject: [PATCH] ghcb: disable interrupts around VMGEXIT Issuing a GHCB call via VMGEXIT requires setting and potentially reading the GHCB MSR. Since GHCB calls can be issued by interrupt routines, interrupts must be disabled during each set-exit-read sequence to prevent possible tearing due to preemption during such a sequence. Signed-off-by: Jon Lange --- kernel/src/sev/ghcb.rs | 10 +++++++-- kernel/src/sev/msr_protocol.rs | 40 +++++++++++++++++++++++++++++----- kernel/src/sev/utils.rs | 11 ++++++---- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/kernel/src/sev/ghcb.rs b/kernel/src/sev/ghcb.rs index ae55ed84a..35ec4335f 100644 --- a/kernel/src/sev/ghcb.rs +++ b/kernel/src/sev/ghcb.rs @@ -7,7 +7,7 @@ use crate::address::{Address, PhysAddr, VirtAddr}; use crate::cpu::msr::{write_msr, SEV_GHCB}; use crate::cpu::percpu::this_cpu; -use crate::cpu::{flush_tlb_global_sync, X86GeneralRegs}; +use crate::cpu::{flush_tlb_global_sync, IrqGuard, X86GeneralRegs}; use crate::error::SvsmError; use crate::mm::validate::{ valid_bitmap_clear_valid_4k, valid_bitmap_set_valid_4k, valid_bitmap_valid_addr, @@ -377,8 +377,14 @@ impl GHCB { let ghcb_address = VirtAddr::from(self as *const GHCB); let ghcb_pa = u64::from(virt_to_phys(ghcb_address)); + // Disable interrupts between writing the MSR and making the GHCB call + // to prevent reentrant use of the GHCB MSR. + let guard = IrqGuard::new(); write_msr(SEV_GHCB, ghcb_pa); - raw_vmgexit(); + unsafe { + raw_vmgexit(); + } + drop(guard); let sw_exit_info_1 = self.get_exit_info_1_valid()?; if sw_exit_info_1 != 0 { diff --git a/kernel/src/sev/msr_protocol.rs b/kernel/src/sev/msr_protocol.rs index 8f8976738..dcb59c1e6 100644 --- a/kernel/src/sev/msr_protocol.rs +++ b/kernel/src/sev/msr_protocol.rs @@ -5,7 +5,9 @@ // Author: Joerg Roedel use crate::address::{Address, PhysAddr}; +use crate::cpu::irq_state::raw_irqs_disable; use crate::cpu::msr::{read_msr, write_msr, SEV_GHCB}; +use crate::cpu::{irqs_enabled, IrqGuard}; use crate::error::SvsmError; use crate::utils::halt; use crate::utils::immut_after_init::ImmutAfterInitCell; @@ -71,9 +73,15 @@ static GHCB_HV_FEATURES: ImmutAfterInitCell = ImmutAfterInitCell /// Check that we support the hypervisor's advertised GHCB versions. pub fn verify_ghcb_version() { + // This function is normally only called early during initializtion before + // interrupts have been enabled, and before interrupt guards can safely be + // used. + assert!(!irqs_enabled()); // Request SEV information. write_msr(SEV_GHCB, GHCBMsr::SEV_INFO_REQ); - raw_vmgexit(); + unsafe { + raw_vmgexit(); + } let sev_info = read_msr(SEV_GHCB); // Parse the results. @@ -100,9 +108,13 @@ pub fn hypervisor_ghcb_features() -> GHCBHvFeatures { } pub fn init_hypervisor_ghcb_features() -> Result<(), GhcbMsrError> { + let guard = IrqGuard::new(); write_msr(SEV_GHCB, GHCBMsr::SNP_HV_FEATURES_REQ); - raw_vmgexit(); + unsafe { + raw_vmgexit(); + } let result = read_msr(SEV_GHCB); + drop(guard); if (result & 0xFFF) == GHCBMsr::SNP_HV_FEATURES_RESP { let features = GHCBHvFeatures::from_bits_truncate(result >> 12); @@ -134,9 +146,13 @@ pub fn register_ghcb_gpa_msr(addr: PhysAddr) -> Result<(), GhcbMsrError> { let mut info = addr.bits() as u64; info |= GHCBMsr::SNP_REG_GHCB_GPA_REQ; + let guard = IrqGuard::new(); write_msr(SEV_GHCB, info); - raw_vmgexit(); + unsafe { + raw_vmgexit(); + } info = read_msr(SEV_GHCB); + drop(guard); if (info & 0xfff) != GHCBMsr::SNP_REG_GHCB_GPA_RESP { return Err(GhcbMsrError::InfoMismatch); @@ -159,9 +175,13 @@ fn set_page_valid_status_msr(addr: PhysAddr, valid: bool) -> Result<(), GhcbMsrE } info |= GHCBMsr::SNP_STATE_CHANGE_REQ; + let guard = IrqGuard::new(); write_msr(SEV_GHCB, info); - raw_vmgexit(); + unsafe { + raw_vmgexit(); + } let response = read_msr(SEV_GHCB); + drop(guard); if (response & 0xfff) != GHCBMsr::SNP_STATE_CHANGE_RESP { return Err(GhcbMsrError::InfoMismatch); @@ -185,8 +205,16 @@ pub fn invalidate_page_msr(addr: PhysAddr) -> Result<(), GhcbMsrError> { pub fn request_termination_msr() -> ! { let info: u64 = GHCBMsr::TERM_REQ; - write_msr(SEV_GHCB, info); - raw_vmgexit(); + // Safety + // + // Since this processor is destined for a fatal termination, there is + // no reason to preserve interrupt state. Interrupts can be disabled + // outright prior to shutdown. + unsafe { + raw_irqs_disable(); + write_msr(SEV_GHCB, info); + raw_vmgexit(); + } loop { halt(); } diff --git a/kernel/src/sev/utils.rs b/kernel/src/sev/utils.rs index 796f808a0..49fe1fc86 100644 --- a/kernel/src/sev/utils.rs +++ b/kernel/src/sev/utils.rs @@ -164,10 +164,13 @@ pub fn get_dr7() -> u64 { out } -pub fn raw_vmgexit() { - unsafe { - asm!("rep; vmmcall", options(att_syntax)); - } +/// # Safety +/// VMGEXIT operations generally need to be performed with interrupts disabled +/// to ensure that an interrupt cannot cause the GHCB MSR to change prior to +/// exiting to the host. It is the caller's responsibility to ensure that +/// interrupt handling is configured correctly for the attemtped operation. +pub unsafe fn raw_vmgexit() { + asm!("rep; vmmcall", options(att_syntax)); } bitflags::bitflags! {