From 4170523aed8bfc07e5b34e242bcad51939d62d73 Mon Sep 17 00:00:00 2001 From: Jon Lange Date: Wed, 23 Oct 2024 21:18:09 -0700 Subject: [PATCH 1/6] panic: use a platform-specific halt routine Not all platforms support HLT as a way to halt execution, and therefore the panic halt must be performed through a platform-specific routine. However, at the time of a panic, the global platform object may not yet be initialized. Instead, use a more primitive mechanism for choosing a platform-specific halt routine during panic. Signed-off-by: Jon Lange --- bootlib/src/platform.rs | 2 +- kernel/src/platform/mod.rs | 26 ++++++++++++++++++++++++++ kernel/src/platform/tdp.rs | 6 +++++- kernel/src/stage2.rs | 10 +++++++--- kernel/src/svsm.rs | 9 ++++++--- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/bootlib/src/platform.rs b/bootlib/src/platform.rs index 23dc10bad..ce8efc10a 100644 --- a/bootlib/src/platform.rs +++ b/bootlib/src/platform.rs @@ -5,7 +5,7 @@ // Author: Jon Lange (jlange@microsoft.com) /// Defines the underlying platform type on which the SVSM will run. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] #[repr(C)] pub enum SvsmPlatformType { Native = 0, diff --git a/kernel/src/platform/mod.rs b/kernel/src/platform/mod.rs index 1a6cd68c4..4822827f0 100644 --- a/kernel/src/platform/mod.rs +++ b/kernel/src/platform/mod.rs @@ -13,6 +13,7 @@ use crate::platform::native::NativePlatform; use crate::platform::snp::SnpPlatform; use crate::platform::tdp::TdpPlatform; use crate::types::PageSize; +use crate::utils; use crate::utils::immut_after_init::ImmutAfterInitCell; use crate::utils::MemoryRegion; @@ -23,6 +24,7 @@ pub mod native; pub mod snp; pub mod tdp; +static SVSM_PLATFORM_TYPE: ImmutAfterInitCell = ImmutAfterInitCell::uninit(); pub static SVSM_PLATFORM: ImmutAfterInitCell = ImmutAfterInitCell::uninit(); #[derive(Clone, Copy, Debug)] @@ -50,6 +52,14 @@ pub enum PageValidateOp { /// This defines a platform abstraction to permit the SVSM to run on different /// underlying architectures. pub trait SvsmPlatform { + /// Halts the system as required by the platform. + fn halt() + where + Self: Sized, + { + utils::halt(); + } + /// Performs basic early initialization of the runtime environment. fn env_setup(&mut self, debug_serial_port: u16, vtom: usize) -> Result<(), SvsmError>; @@ -135,6 +145,7 @@ pub enum SvsmPlatformCell { impl SvsmPlatformCell { pub fn new(platform_type: SvsmPlatformType) -> Self { + assert_eq!(platform_type, *SVSM_PLATFORM_TYPE); match platform_type { SvsmPlatformType::Native => SvsmPlatformCell::Native(NativePlatform::new()), SvsmPlatformType::Snp => SvsmPlatformCell::Snp(SnpPlatform::new()), @@ -158,3 +169,18 @@ impl SvsmPlatformCell { } } } + +pub fn init_platform_type(platform_type: SvsmPlatformType) { + SVSM_PLATFORM_TYPE.init(&platform_type).unwrap(); +} + +pub fn halt() { + // Use a platform-specific halt. However, the SVSM_PLATFORM global may not + // yet be initialized, so go choose the halt implementation based on the + // platform-specific halt instead. + match *SVSM_PLATFORM_TYPE { + SvsmPlatformType::Native => NativePlatform::halt(), + SvsmPlatformType::Snp => SnpPlatform::halt(), + SvsmPlatformType::Tdp => TdpPlatform::halt(), + } +} diff --git a/kernel/src/platform/tdp.rs b/kernel/src/platform/tdp.rs index bb210c14b..46a50e0a7 100644 --- a/kernel/src/platform/tdp.rs +++ b/kernel/src/platform/tdp.rs @@ -16,7 +16,7 @@ use crate::types::PageSize; use crate::utils::immut_after_init::ImmutAfterInitCell; use crate::utils::{zero_mem_region, MemoryRegion}; use tdx_tdcall::tdx::{ - td_accept_memory, tdvmcall_io_read_16, tdvmcall_io_read_32, tdvmcall_io_read_8, + td_accept_memory, tdvmcall_halt, tdvmcall_io_read_16, tdvmcall_io_read_32, tdvmcall_io_read_8, tdvmcall_io_write_16, tdvmcall_io_write_32, tdvmcall_io_write_8, }; @@ -39,6 +39,10 @@ impl Default for TdpPlatform { } impl SvsmPlatform for TdpPlatform { + fn halt() { + tdvmcall_halt(); + } + fn env_setup(&mut self, debug_serial_port: u16, vtom: usize) -> Result<(), SvsmError> { VTOM.init(&vtom).map_err(|_| SvsmError::PlatformInit)?; // Serial console device can be initialized immediately diff --git a/kernel/src/stage2.rs b/kernel/src/stage2.rs index 3520ad39a..96094c532 100755 --- a/kernel/src/stage2.rs +++ b/kernel/src/stage2.rs @@ -33,9 +33,12 @@ use svsm::mm::validate::{ init_valid_bitmap_alloc, valid_bitmap_addr, valid_bitmap_set_valid_range, }; use svsm::mm::{init_kernel_mapping_info, FixedAddressMappingRange, SVSM_PERCPU_BASE}; -use svsm::platform::{PageStateChangeOp, PageValidateOp, SvsmPlatform, SvsmPlatformCell}; +use svsm::platform; +use svsm::platform::{ + init_platform_type, PageStateChangeOp, PageValidateOp, SvsmPlatform, SvsmPlatformCell, +}; use svsm::types::{PageSize, PAGE_SIZE, PAGE_SIZE_2M}; -use svsm::utils::{halt, is_aligned, MemoryRegion}; +use svsm::utils::{is_aligned, MemoryRegion}; extern "C" { static mut pgtable: PageTable; @@ -345,6 +348,7 @@ fn prepare_heap( #[no_mangle] pub extern "C" fn stage2_main(launch_info: &Stage2LaunchInfo) { let platform_type = SvsmPlatformType::from(launch_info.platform_type); + init_platform_type(platform_type); let mut platform_cell = SvsmPlatformCell::new(platform_type); let platform = platform_cell.as_mut_dyn_ref(); @@ -465,6 +469,6 @@ pub extern "C" fn stage2_main(launch_info: &Stage2LaunchInfo) { fn panic(info: &PanicInfo<'_>) -> ! { log::error!("Panic: {}", info); loop { - halt(); + platform::halt(); } } diff --git a/kernel/src/svsm.rs b/kernel/src/svsm.rs index a9e74d953..1d06e3f1f 100755 --- a/kernel/src/svsm.rs +++ b/kernel/src/svsm.rs @@ -37,7 +37,8 @@ use svsm::mm::memory::{init_memory_map, write_guest_memory_map}; use svsm::mm::pagetable::paging_init; use svsm::mm::virtualrange::virt_log_usage; use svsm::mm::{init_kernel_mapping_info, FixedAddressMappingRange, PerCPUPageMappingGuard}; -use svsm::platform::{SvsmPlatformCell, SVSM_PLATFORM}; +use svsm::platform; +use svsm::platform::{init_platform_type, SvsmPlatformCell, SVSM_PLATFORM}; use svsm::requests::{request_loop, request_processing_main, update_mappings}; use svsm::sev::utils::{rmp_adjust, RMPFlags}; use svsm::sev::{secrets_page, secrets_page_mut}; @@ -45,7 +46,7 @@ use svsm::svsm_paging::{init_page_table, invalidate_early_boot_memory}; use svsm::task::exec_user; use svsm::task::{create_kernel_task, schedule_init}; use svsm::types::{PageSize, GUEST_VMPL, PAGE_SIZE}; -use svsm::utils::{halt, immut_after_init::ImmutAfterInitCell, zero_mem_region}; +use svsm::utils::{immut_after_init::ImmutAfterInitCell, zero_mem_region}; #[cfg(all(feature = "mstpm", not(test)))] use svsm::vtpm::vtpm_init; @@ -275,6 +276,8 @@ fn init_cpuid_table(addr: VirtAddr) { #[no_mangle] pub extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) { let launch_info: KernelLaunchInfo = *li; + init_platform_type(launch_info.platform_type); + let vb_ptr = core::ptr::NonNull::new(VirtAddr::new(vb_addr).as_mut_ptr::()).unwrap(); mapping_info_init(&launch_info); @@ -481,6 +484,6 @@ fn panic(info: &PanicInfo<'_>) -> ! { loop { debug_break(); - halt(); + platform::halt(); } } From 2d918acca5d5e541f4c9a62af3957c8a793c2f2f Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Mon, 28 Oct 2024 09:08:54 +0000 Subject: [PATCH 2/6] libmstpm: store generated bindings in OUT_DIR Generated code shouldn't be stored directly in the src directory. All files generated by build-scripts should be stored in the directory pointed to in the OUT_DIR environment variable. One advantage of this is that we can tell the build-script to emit rerun-if statements that tell cargo to only rerun if any of the files in this directory have changed. If we generate files into this directory, cargo may not always realize that the generated files shouldn't trigger a re-run. Signed-off-by: Tom Dohrmann --- libmstpm/.gitignore | 1 - libmstpm/Makefile | 13 +++++-------- libmstpm/src/lib.rs | 10 +++++++++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/libmstpm/.gitignore b/libmstpm/.gitignore index 5fbcfbb2f..e9a3e9fb1 100644 --- a/libmstpm/.gitignore +++ b/libmstpm/.gitignore @@ -1,2 +1 @@ libmstpm.a -src/bindings.rs diff --git a/libmstpm/Makefile b/libmstpm/Makefile index e78ff0870..162a9a501 100644 --- a/libmstpm/Makefile +++ b/libmstpm/Makefile @@ -26,7 +26,9 @@ MSTPM_MAKEFILE = $(MSTPM_DIR)/Makefile LIBS = $(LIBCRT) $(LIBCRYPTO) $(LIBTPM) $(LIBPLATFORM) -all: libmstpm.a src/bindings.rs +OUT_DIR ?= $(CWD) + +all: libmstpm.a $(OUT_DIR)/bindings.rs libmstpm.a: $(LIBS) rm -f $@ @@ -123,13 +125,8 @@ $(MSTPM_MAKEFILE): BINDGEN_FLAGS = --use-core CLANG_FLAGS = -Wno-incompatible-library-redeclaration -src/bindings.rs: deps/libmstpm.h $(LIBTPM) - echo "#![allow(non_upper_case_globals)]" > $@ - echo "#![allow(non_camel_case_types)]" >> $@ - echo "#![allow(non_snake_case)]" >> $@ - echo "#![allow(unused)]" >> $@ - echo "#![allow(improper_ctypes)]" >> $@ - bindgen $(BINDGEN_FLAGS) deps/libmstpm.h -- $(CLANG_FLAGS) >> $@ +$(OUT_DIR)/bindings.rs: deps/libmstpm.h $(LIBTPM) + bindgen $(BINDGEN_FLAGS) --output $@ deps/libmstpm.h -- $(CLANG_FLAGS) clean: $(OPENSSL_MAKEFILE) $(MSTPM_MAKEFILE) make -C $(LIBCRT_DIR) clean diff --git a/libmstpm/src/lib.rs b/libmstpm/src/lib.rs index 55cfd93cd..e8954b1e8 100644 --- a/libmstpm/src/lib.rs +++ b/libmstpm/src/lib.rs @@ -10,4 +10,12 @@ #![no_std] /// C bindings -pub mod bindings; +pub mod bindings { + #![allow(non_upper_case_globals)] + #![allow(non_camel_case_types)] + #![allow(non_snake_case)] + #![allow(unused)] + #![allow(improper_ctypes)] + + include!(concat!(env!("OUT_DIR"), "/bindings.rs")); +} From 922106df7ea477eba94e769cf5fb159fe125fff6 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Mon, 28 Oct 2024 09:30:59 +0000 Subject: [PATCH 3/6] kernel: move linker args in build.rs to libmstpm Linker args shouldn't be set by the crate consuming a library, but by the crate producing a library. Signed-off-by: Tom Dohrmann --- kernel/build.rs | 4 ---- libmstpm/build.rs | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/kernel/build.rs b/kernel/build.rs index 2a43d34fb..526b10203 100644 --- a/kernel/build.rs +++ b/kernel/build.rs @@ -21,10 +21,6 @@ fn main() { println!("cargo:rustc-link-arg-bin=svsm=--no-relax"); println!("cargo:rustc-link-arg-bin=svsm=-Tkernel/src/svsm.lds"); println!("cargo:rustc-link-arg-bin=svsm=-no-pie"); - if std::env::var("CARGO_FEATURE_MSTPM").is_ok() && std::env::var("CARGO_CFG_TEST").is_err() { - println!("cargo:rustc-link-arg-bin=svsm=-Llibmstpm"); - println!("cargo:rustc-link-arg-bin=svsm=-lmstpm"); - } // Extra linker args for tests. println!("cargo:rerun-if-env-changed=LINK_TEST"); diff --git a/libmstpm/build.rs b/libmstpm/build.rs index b096b0428..b9f5e6b60 100644 --- a/libmstpm/build.rs +++ b/libmstpm/build.rs @@ -4,6 +4,7 @@ // // Authors: Claudio Carvalho +use std::env::current_dir; use std::process::Command; use std::process::Stdio; @@ -17,4 +18,10 @@ fn main() { if !output.status.success() { panic!(); } + + // Tell cargo to link libmstpm and where to find it. + let cwd = current_dir().unwrap(); + let cwd = cwd.as_os_str().to_str().unwrap(); + println!("cargo:rustc-link-search={cwd}"); + println!("cargo:rustc-link-lib=mstpm"); } From 5f4987a1967c706ea832fa208783078b827a683b Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Mon, 28 Oct 2024 09:32:21 +0000 Subject: [PATCH 4/6] libmstpm: store library in OUT_DIR Much like the generated Rust bindings, the built static library should reside in OUT_DIR. Signed-off-by: Tom Dohrmann --- libmstpm/.gitignore | 1 - libmstpm/Makefile | 4 ++-- libmstpm/build.rs | 6 ++---- 3 files changed, 4 insertions(+), 7 deletions(-) delete mode 100644 libmstpm/.gitignore diff --git a/libmstpm/.gitignore b/libmstpm/.gitignore deleted file mode 100644 index e9a3e9fb1..000000000 --- a/libmstpm/.gitignore +++ /dev/null @@ -1 +0,0 @@ -libmstpm.a diff --git a/libmstpm/Makefile b/libmstpm/Makefile index 162a9a501..5597b78cb 100644 --- a/libmstpm/Makefile +++ b/libmstpm/Makefile @@ -28,9 +28,9 @@ LIBS = $(LIBCRT) $(LIBCRYPTO) $(LIBTPM) $(LIBPLATFORM) OUT_DIR ?= $(CWD) -all: libmstpm.a $(OUT_DIR)/bindings.rs +all: $(OUT_DIR)/libmstpm.a $(OUT_DIR)/bindings.rs -libmstpm.a: $(LIBS) +$(OUT_DIR)/libmstpm.a: $(LIBS) rm -f $@ ar rcsTPD $@ $^ diff --git a/libmstpm/build.rs b/libmstpm/build.rs index b9f5e6b60..73b1e7502 100644 --- a/libmstpm/build.rs +++ b/libmstpm/build.rs @@ -4,7 +4,6 @@ // // Authors: Claudio Carvalho -use std::env::current_dir; use std::process::Command; use std::process::Stdio; @@ -20,8 +19,7 @@ fn main() { } // Tell cargo to link libmstpm and where to find it. - let cwd = current_dir().unwrap(); - let cwd = cwd.as_os_str().to_str().unwrap(); - println!("cargo:rustc-link-search={cwd}"); + let out_dir = std::env::var("OUT_DIR").unwrap(); + println!("cargo:rustc-link-search={out_dir}"); println!("cargo:rustc-link-lib=mstpm"); } From b15e4b7a8b4fa0c1bafd4fbdc4911313aa247fcf Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Mon, 28 Oct 2024 09:33:55 +0000 Subject: [PATCH 5/6] libmstpm: add rerun-if-changed output to build.rs This tell cargo to rerun the build-script if any of the files in this directory have changed. Signed-off-by: Tom Dohrmann --- libmstpm/build.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libmstpm/build.rs b/libmstpm/build.rs index 73b1e7502..feb9a686c 100644 --- a/libmstpm/build.rs +++ b/libmstpm/build.rs @@ -4,6 +4,7 @@ // // Authors: Claudio Carvalho +use std::env::current_dir; use std::process::Command; use std::process::Stdio; @@ -22,4 +23,10 @@ fn main() { let out_dir = std::env::var("OUT_DIR").unwrap(); println!("cargo:rustc-link-search={out_dir}"); println!("cargo:rustc-link-lib=mstpm"); + + // Tell cargo not to rerun the build-script unless anything in this + // directory changes. + let cwd = current_dir().unwrap(); + let cwd = cwd.as_os_str().to_str().unwrap(); + println!("cargo:rerun-if-changed={cwd}"); } From d29f6bf8790b0c822631612fcc22a37d800a8464 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Mon, 28 Oct 2024 09:35:33 +0000 Subject: [PATCH 6/6] libmstpm: simplify make invocation We don't care about the output, we only care about the status. Signed-off-by: Tom Dohrmann --- libmstpm/build.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libmstpm/build.rs b/libmstpm/build.rs index feb9a686c..539b6a691 100644 --- a/libmstpm/build.rs +++ b/libmstpm/build.rs @@ -9,15 +9,13 @@ use std::process::Command; use std::process::Stdio; fn main() { - let output = Command::new("make") + // Build libmstpm. + let status = Command::new("make") .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) - .output() + .status() .unwrap(); - - if !output.status.success() { - panic!(); - } + assert!(status.success()); // Tell cargo to link libmstpm and where to find it. let out_dir = std::env::var("OUT_DIR").unwrap();