From fca59436b03dd27964d2ad5b1ef86939b171bdee Mon Sep 17 00:00:00 2001 From: Vikram Narayanan Date: Wed, 23 Nov 2022 04:09:59 +0000 Subject: [PATCH 1/2] vc: mark all vc_terminate helpers as noreturn vc_terminate helpers never return as they internally call `vc_terminate` with a specific return code and reason. Marking all of these as noreturn will allow us use these functions in places where this is part of a match arm. Also fix warning on unused assignments. Technically, the initialized var `pa` is never read, it is just over-written. Changing the return value in the error path function unmasked this warning. Signed-off-by: Vikram Narayanan --- src/cpu/vc.rs | 28 ++++++++++++++-------------- src/mem/pgtable.rs | 12 ++++-------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/cpu/vc.rs b/src/cpu/vc.rs index ba216d8..3a842d0 100644 --- a/src/cpu/vc.rs +++ b/src/cpu/vc.rs @@ -168,85 +168,85 @@ pub fn vc_terminate(reason_set: u64, reason_code: u64) -> ! { /// Terminate SVSM with generic SVSM reason #[inline] -pub fn vc_terminate_svsm_general() { +pub fn vc_terminate_svsm_general() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_GENERAL); } /// Terminate SVSM due to lack of memory #[inline] -pub fn vc_terminate_svsm_enomem() { +pub fn vc_terminate_svsm_enomem() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM); } /// Terminate SVSM due to firmware configuration error #[inline] -pub fn vc_terminate_svsm_fwcfg() { +pub fn vc_terminate_svsm_fwcfg() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_FW_CFG_ERROR); } /// Terminate SVSM due to invalid GHCB response #[inline] -pub fn vc_terminate_svsm_resp_invalid() { +pub fn vc_terminate_svsm_resp_invalid() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_GHCB_RESP_INVALID); } /// Terminate SVSM due to a page-related error #[inline] -pub fn vc_terminate_svsm_page_err() { +pub fn vc_terminate_svsm_page_err() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_SET_PAGE_ERROR); } /// Terminate SVSM due to a PSC-related error #[inline] -pub fn vc_terminate_svsm_psc() { +pub fn vc_terminate_svsm_psc() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_PSC_ERROR); } /// Terminate SVSM due to a BIOS-format related error #[inline] -pub fn vc_terminate_svsm_bios() { +pub fn vc_terminate_svsm_bios() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_BIOS_FORMAT); } /// Terminate SVSM due to an unhandled #VC exception #[inline] -pub fn vc_terminate_unhandled_vc() { +pub fn vc_terminate_unhandled_vc() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_UNHANDLED_VC); } /// Terminate SVSM with generic GHCB reason #[inline] -pub fn vc_terminate_ghcb_general() { +pub fn vc_terminate_ghcb_general() -> ! { vc_terminate(GHCB_REASON_CODE_SET, GHCB_TERM_GENERAL); } /// Terminate SVSM due to unsupported GHCB protocol #[inline] -pub fn vc_terminate_ghcb_unsupported_protocol() { +pub fn vc_terminate_ghcb_unsupported_protocol() -> ! { vc_terminate(GHCB_REASON_CODE_SET, GHCB_TERM_UNSUPPORTED_PROTOCOL); } /// Terminate SVSM due to error related with feature support #[inline] -pub fn vc_terminate_ghcb_feature() { +pub fn vc_terminate_ghcb_feature() -> ! { vc_terminate(GHCB_REASON_CODE_SET, GHCB_TERM_FEATURE_SUPPORT); } /// Terminate SVSM due to incorrect SEV features for VMPL1 #[inline] -pub fn vc_terminate_vmpl1_sev_features() { +pub fn vc_terminate_vmpl1_sev_features() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_VMPL1_SEV_FEATURES); } /// Terminate SVSM due to incorrect SEV features for VMPL0 #[inline] -pub fn vc_terminate_vmpl0_sev_features() { +pub fn vc_terminate_vmpl0_sev_features() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_VMPL0_SEV_FEATURES); } /// Terminate SVSM due to incorrect VMPL level on VMSA #[inline] -pub fn vc_terminate_svsm_incorrect_vmpl() { +pub fn vc_terminate_svsm_incorrect_vmpl() -> ! { vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_INCORRECT_VMPL); } diff --git a/src/mem/pgtable.rs b/src/mem/pgtable.rs index 92373ac..bdc42b4 100644 --- a/src/mem/pgtable.rs +++ b/src/mem/pgtable.rs @@ -20,7 +20,7 @@ use x86_64::addr::{PhysAddr, VirtAddr}; use x86_64::registers::control::{Cr3, Cr3Flags}; use x86_64::structures::paging::frame::PhysFrame; use x86_64::structures::paging::mapper::{ - FlagUpdateError, MapToError, MapperFlush, TranslateResult, UnmapError, + FlagUpdateError, MapToError, MapperFlush, TranslateResult, }; use x86_64::structures::paging::page::Page; use x86_64::structures::paging::page::{PageRange, Size4KiB}; @@ -70,14 +70,10 @@ fn remap_page(page: Page, page_type: PageType, flush: bool) { let mut allocator: PageTableAllocator = PageTableAllocator::new(); unsafe { - let mut pa: PhysAddr = PhysAddr::new(0); - - let result: Result<(PhysFrame, MapperFlush), UnmapError> = - PGTABLE.lock().unmap(page); - match result { - Ok(r) => pa = r.0.start_address(), + let pa: PhysAddr = match PGTABLE.lock().unmap(page) { + Ok(r) => r.0.start_address(), Err(_e) => vc_terminate_svsm_page_err(), - } + }; let map_pa: PhysAddr; if page_type == PageType::Private { From 901b4fc316094a81f41b9b724012a8671a85ddff Mon Sep 17 00:00:00 2001 From: Vikram Narayanan Date: Wed, 23 Nov 2022 04:13:08 +0000 Subject: [PATCH 2/2] use vc_terminate helpers directly Since the vc_terminate helpers are now marked as noreturn, we can use them directly in all places such that the compiler will not complain that the Err path in the match arm is not returning the same value as success path. Signed-off-by: Vikram Narayanan --- src/bios.rs | 8 ++++---- src/cpu/percpu.rs | 4 ++-- src/cpu/smp.rs | 6 +++--- src/cpu/tss.rs | 4 ++-- src/cpu/vc.rs | 4 ++-- src/mem/fwcfg.rs | 6 +++--- src/mem/ghcb.rs | 4 ++-- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index eceabf4..a7ac53d 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -176,7 +176,7 @@ fn find_bios_guid_entry(bios_info: &mut BiosInfo, guid: &str) -> Option { let target_guid: Uuid = match Uuid::parse_str(guid) { Ok(g) => g, - Err(_e) => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_BIOS_FORMAT), + Err(_e) => vc_terminate_svsm_bios(), }; unsafe { __find_bios_guid_entry(bios_info, target_guid, &mut avail_len, &mut p) } @@ -216,7 +216,7 @@ unsafe fn __find_snp_section(bios_info: &mut BiosInfo, stype: u32, p: u64) -> Op fn find_snp_section(bios_info: &mut BiosInfo, stype: u32) -> Option { let p: u64 = match find_bios_guid_entry(bios_info, OVMF_SNP_ENTRY_GUID) { Some(p) => p, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_BIOS_FORMAT), + None => vc_terminate_svsm_bios(), }; unsafe { __find_snp_section(bios_info, stype, p) } @@ -328,7 +328,7 @@ fn parse_bios_guid_table(bios_info: &mut BiosInfo) -> bool { pub fn start_bios() { let (bios_va, bios_size) = match fwcfg_map_bios() { Some(t) => t, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_FW_CFG_ERROR), + None => vc_terminate_svsm_fwcfg(), }; let mut bios_info: BiosInfo = BiosInfo::new(bios_va, bios_size); @@ -338,7 +338,7 @@ pub fn start_bios() { let caa: PhysAddr = match locate_bios_ca_page(&mut bios_info) { Some(p) => p, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_BIOS_FORMAT), + None => vc_terminate_svsm_bios(), }; unsafe { diff --git a/src/cpu/percpu.rs b/src/cpu/percpu.rs index 87b3094..3def6d4 100644 --- a/src/cpu/percpu.rs +++ b/src/cpu/percpu.rs @@ -422,7 +422,7 @@ unsafe fn __percpu_init(init_frame: PhysFrame, init_count: u64) -> u64 { if count != init_count { frame = match mem_allocate_frames(count) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; } else { frame = init_frame; @@ -456,7 +456,7 @@ pub fn percpu_init() { let init_count: u64 = PAGE_COUNT!(PERCPU_SIZE); let init_frame: PhysFrame = match mem_allocate_frames(init_count) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; let count: u64; diff --git a/src/cpu/smp.rs b/src/cpu/smp.rs index 9ef1a39..4f59a35 100644 --- a/src/cpu/smp.rs +++ b/src/cpu/smp.rs @@ -93,7 +93,7 @@ fn alloc_vmsa() -> PhysFrame { // Allocate one frame let mut frame: PhysFrame = match mem_allocate_frames(1) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; // VMSA pages must not be 2MB aligned, check for that @@ -104,7 +104,7 @@ fn alloc_vmsa() -> PhysFrame { // Allocate two frames and ... frame = match mem_allocate_frames(2) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; // ... chose a frame which is not 2MB aligned @@ -156,7 +156,7 @@ fn create_bios_vmsa() -> VirtAddr { fn create_svsm_stack() -> VirtAddr { let frame: PhysFrame = match mem_allocate_frames(SVSM_STACK_PAGES) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; let guard_va: VirtAddr = pgtable_pa_to_va(frame.start_address()); diff --git a/src/cpu/tss.rs b/src/cpu/tss.rs index a8ad5f3..e75d6da 100644 --- a/src/cpu/tss.rs +++ b/src/cpu/tss.rs @@ -26,7 +26,7 @@ const IST_STACK_PAGES: u64 = 3; unsafe fn create_tss() -> VirtAddr { let tss_va: VirtAddr = match mem_allocate(size_of::()) { Ok(f) => f, - Err(()) => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + Err(()) => vc_terminate_svsm_enomem(), }; let tss: *mut TaskStateSegment = tss_va.as_mut_ptr(); @@ -37,7 +37,7 @@ unsafe fn create_tss() -> VirtAddr { let frame: PhysFrame = match mem_allocate_frames(IST_STACK_PAGES) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; let guard_va: VirtAddr = pgtable_pa_to_va(frame.start_address()); diff --git a/src/cpu/vc.rs b/src/cpu/vc.rs index 3a842d0..e18367c 100644 --- a/src/cpu/vc.rs +++ b/src/cpu/vc.rs @@ -397,7 +397,7 @@ pub fn vc_get_apic_ids(bsp_apic_id: u32) -> Vec { let frame: PhysFrame = match mem_allocate_frames(pages) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; let pa: PhysAddr = frame.start_address(); let va: VirtAddr = pgtable_pa_to_va(pa); @@ -955,7 +955,7 @@ fn perform_page_state_change(ghcb: *mut Ghcb, begin: PhysFrame, end: PhysFrame, while op.header.cur_entry <= last_entry { vc_perform_vmgexit(ghcb, GHCB_NAE_PSC, 0, 0); if !(*ghcb).is_sw_exit_info_2_valid() || (*ghcb).sw_exit_info_2() != 0 { - vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_PSC_ERROR); + vc_terminate_svsm_psc(); } (*ghcb).shared_buffer(get_bytes, size); diff --git a/src/mem/fwcfg.rs b/src/mem/fwcfg.rs index 3eb091c..2bf83dd 100644 --- a/src/mem/fwcfg.rs +++ b/src/mem/fwcfg.rs @@ -115,7 +115,7 @@ lazy_static! { static ref FW_CFG_DMA: SpinLock<&'static mut FwCfgDma> = { let frame: PhysFrame = match mem_allocate_frame() { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; let va: VirtAddr = pgtable_pa_to_va(frame.start_address()); @@ -219,11 +219,11 @@ fn find_file_selector(fname: &str) -> Option { for f in files.iter() { let nul: usize = match memchr(0, &f.name) { Some(n) => n, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_FW_CFG_ERROR), + None => vc_terminate_svsm_fwcfg(), }; let n: &str = match core::str::from_utf8(&f.name[0..nul]) { Ok(n) => n, - Err(_e) => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_FW_CFG_ERROR), + Err(_e) => vc_terminate_svsm_fwcfg(), }; if n.eq(fname) { diff --git a/src/mem/ghcb.rs b/src/mem/ghcb.rs index 9664648..1eefdaa 100644 --- a/src/mem/ghcb.rs +++ b/src/mem/ghcb.rs @@ -9,7 +9,7 @@ use crate::cpu::percpu::PERCPU; use crate::cpu::percpu_count; use crate::cpu::vc_register_ghcb; -use crate::cpu::vc_terminate; +use crate::cpu::vc_terminate_svsm_enomem; use crate::globals::*; use crate::mem::mem_allocate_frames; use crate::mem::pgtable_make_pages_shared; @@ -146,7 +146,7 @@ pub fn ghcb_init() { let count: usize = percpu_count(); let frame: PhysFrame = match mem_allocate_frames(count as u64) { Some(f) => f, - None => vc_terminate(SVSM_REASON_CODE_SET, SVSM_TERM_ENOMEM), + None => vc_terminate_svsm_enomem(), }; let mut va: VirtAddr = pgtable_pa_to_va(frame.start_address());