From 60be64572eeebcef007b7a457dd9637634b10a58 Mon Sep 17 00:00:00 2001 From: whensun Date: Thu, 29 Oct 2026 20:04:44 -0700 Subject: [PATCH 1/4] Fix RISC-V vector state preservation across context switches Signed-off-by: whensun --- .../enter_from_confidential_hart.S | 4 +- .../riscv/extensions/vector_unit.rs | 205 +++++++++++++++++- .../riscv/hart_architectural_state.rs | 14 +- .../src/core/architecture/riscv/mod.rs | 1 + .../core/architecture/riscv/specification.rs | 3 + .../core/control_data/confidential_hart.rs | 15 ++ .../src/core/control_data/hypervisor_hart.rs | 12 +- .../enter_from_hypervisor_or_vm.S | 4 +- 8 files changed, 247 insertions(+), 11 deletions(-) diff --git a/security-monitor/src/confidential_flow/lightweight_context_switch/enter_from_confidential_hart.S b/security-monitor/src/confidential_flow/lightweight_context_switch/enter_from_confidential_hart.S index 02035f08..1fb20187 100644 --- a/security-monitor/src/confidential_flow/lightweight_context_switch/enter_from_confidential_hart.S +++ b/security-monitor/src/confidential_flow/lightweight_context_switch/enter_from_confidential_hart.S @@ -62,7 +62,9 @@ enter_from_confidential_hart_asm: # Recover the stack pointer for this hart. # The stack pointer is stored in the memory dump area of the hart (pointed by `mscratch`) csrr a0, mscratch - ld sp, ({HART_STACK_ADDRESS_OFFSET})(a0) + li t0, {HART_STACK_ADDRESS_OFFSET} + add t0, a0, t0 + ld sp, 0(t0) # Before calling Rust code, we must guarantee that: # * `mscratch` points to the dump area of this hardware hart diff --git a/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs b/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs index 8403b853..1e3a63ba 100644 --- a/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs +++ b/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs @@ -1,16 +1,207 @@ // SPDX-FileCopyrightText: 2023 IBM Corporation // SPDX-FileContributor: Wojciech Ozga , IBM Research - Zurich // SPDX-License-Identifier: Apache-2.0 -#![allow(unused)] +use crate::core::architecture::riscv::specification::{CSR_VCSR, CSR_VL, CSR_VLENB, CSR_VSTART, CSR_VTYPE, SR_VS_DIRTY}; +use crate::core::architecture::{CSR, HardwareExtension}; +use crate::core::hardware_setup::HardwareSetup; +use alloc::vec; use alloc::vec::Vec; -use core::ops::Range; +use core::arch::asm; -pub const MAX_NUMBER_OF_REGISTER_LENGTH: usize = 32; -const NUMBER_OF_VALUES_IN_REGISTER: usize = MAX_NUMBER_OF_REGISTER_LENGTH >> 3; const NUMBER_OF_VECTOR_REGISTERS: usize = 32; #[repr(C)] -pub struct VectorRegister(pub [u64; NUMBER_OF_VALUES_IN_REGISTER]); +pub struct VectorUnit { + registers: Vec, + vlenb: usize, + vl: usize, + vtype: usize, + vstart: usize, + vcsr: usize, +} -#[repr(C)] -pub struct VectorRegisters(pub [usize; NUMBER_OF_VECTOR_REGISTERS]); +impl VectorUnit { + pub fn empty() -> Self { + debug!("ACE_DIAG: Enter VectorUnit::empty"); + + let sp: usize; + + unsafe { + asm!( + "mv {value}, sp", + value = out(reg) sp, + options(nomem, nostack) + ); + } + + debug!("ACE_DIAG: VectorUnit::empty ENTRY sp={:#x} initialized={}", sp, HardwareSetup::is_initialized()); + + let vector_supported = HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension); + + debug!("ACE_DIAG: VectorUnit supported = {}", vector_supported); + + if !vector_supported { + return Self { registers: Vec::new(), vlenb: 0, vl: 0, vtype: 0, vstart: 0, vcsr: 0 }; + } + + debug!("ACE_DIAG: BEFORE mstatus/CSR, initialized={}", HardwareSetup::is_initialized()); + + let old_mstatus = CSR.mstatus.read_and_set_bits(SR_VS_DIRTY); + + let vlenb: usize; + unsafe { + asm!( + "csrr {value}, {csr}", + value = out(reg) vlenb, + csr = const CSR_VLENB + ); + } + + CSR.mstatus.write(old_mstatus); + + debug!("ACE_DIAG: AFTER mstatus/CSR, initialized={}, vlenb={}", HardwareSetup::is_initialized(), vlenb); + + assert!(vlenb > 0); + + let length = NUMBER_OF_VECTOR_REGISTERS.checked_mul(vlenb).expect("Invalid vector register size"); + + debug!("ACE_DIAG: BEFORE vector allocation, initialized={}, length={}", HardwareSetup::is_initialized(), length); + + let registers = vec![0; length]; + + debug!("ACE_DIAG: AFTER vector allocation, initialized={}, ptr={:p}", HardwareSetup::is_initialized(), registers.as_ptr()); + + debug!("ACE_DIAG: Leaving VectorUnit::empty"); + + Self { registers, vlenb, vl: 0, vtype: 0, vstart: 0, vcsr: 0 } + } + + unsafe fn validate_vlenb(&self) { + let mut vlenb: usize; + unsafe { asm!("csrr {value}, {csr}", value = out(reg) vlenb, csr = const CSR_VLENB) }; + assert_eq!(self.vlenb, vlenb); + assert_eq!(self.registers.len(), NUMBER_OF_VECTOR_REGISTERS * vlenb); + } + + pub unsafe fn save_in_main_memory(&mut self) { + unsafe { self.validate_vlenb() }; + unsafe { + asm!("csrr {value}, {csr}", value = out(reg) self.vl, csr = const CSR_VL); + asm!("csrr {value}, {csr}", value = out(reg) self.vtype, csr = const CSR_VTYPE); + asm!("csrr {value}, {csr}", value = out(reg) self.vstart, csr = const CSR_VSTART); + asm!("csrr {value}, {csr}", value = out(reg) self.vcsr, csr = const CSR_VCSR); + asm!("csrw {csr}, zero", csr = const CSR_VSTART); + } + + macro_rules! store { + ($index:literal) => { + unsafe { + asm!( + ".option push", + ".option arch, +v", + concat!("vs1r.v v", stringify!($index), ", ({address})"), + ".option pop", + address = in(reg) self.registers.as_mut_ptr().add($index * self.vlenb) + ); + } + }; + } + + store!(0); + store!(1); + store!(2); + store!(3); + store!(4); + store!(5); + store!(6); + store!(7); + store!(8); + store!(9); + store!(10); + store!(11); + store!(12); + store!(13); + store!(14); + store!(15); + store!(16); + store!(17); + store!(18); + store!(19); + store!(20); + store!(21); + store!(22); + store!(23); + store!(24); + store!(25); + store!(26); + store!(27); + store!(28); + store!(29); + store!(30); + store!(31); + } + + pub unsafe fn restore_from_main_memory(&self) { + unsafe { self.validate_vlenb() }; + unsafe { asm!("csrw {csr}, zero", csr = const CSR_VSTART) }; + + macro_rules! load { + ($index:literal) => { + unsafe { + asm!( + ".option push", + ".option arch, +v", + concat!("vl1re8.v v", stringify!($index), ", ({address})"), + ".option pop", + address = in(reg) self.registers.as_ptr().add($index * self.vlenb) + ); + } + }; + } + + load!(0); + load!(1); + load!(2); + load!(3); + load!(4); + load!(5); + load!(6); + load!(7); + load!(8); + load!(9); + load!(10); + load!(11); + load!(12); + load!(13); + load!(14); + load!(15); + load!(16); + load!(17); + load!(18); + load!(19); + load!(20); + load!(21); + load!(22); + load!(23); + load!(24); + load!(25); + load!(26); + load!(27); + load!(28); + load!(29); + load!(30); + load!(31); + unsafe { + asm!( + ".option push", + ".option arch, +v", + "vsetvl zero, {saved_vl}, {saved_vtype}", + ".option pop", + saved_vl = in(reg) self.vl, + saved_vtype = in(reg) self.vtype + ); + asm!("csrw {csr}, {value}", csr = const CSR_VCSR, value = in(reg) self.vcsr); + asm!("csrw {csr}, {value}", csr = const CSR_VSTART, value = in(reg) self.vstart); + } + } +} diff --git a/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs b/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs index b38f9862..402bb940 100644 --- a/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs +++ b/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs @@ -3,7 +3,9 @@ // SPDX-License-Identifier: Apache-2.0 use crate::core::architecture::riscv::extensions::supervisor_timer_extension::SupervisorTimerExtension; use crate::core::architecture::riscv::specification::*; -use crate::core::architecture::riscv::{ControlStatusRegisters, FloatingPointUnit, GeneralPurposeRegister, GeneralPurposeRegisters}; +use crate::core::architecture::riscv::{ + ControlStatusRegisters, FloatingPointUnit, GeneralPurposeRegister, GeneralPurposeRegisters, VectorUnit, +}; /// Defines the state of a processor's core (hart) when stored in main memory. #[repr(C)] @@ -12,6 +14,7 @@ pub struct HartArchitecturalState { csrs: ControlStatusRegisters, fprs: FloatingPointUnit, sstc: SupervisorTimerExtension, + vector: VectorUnit, } impl HartArchitecturalState { @@ -21,9 +24,18 @@ impl HartArchitecturalState { csrs: ControlStatusRegisters::empty(), fprs: FloatingPointUnit::empty(), sstc: SupervisorTimerExtension::empty(), + vector: VectorUnit::empty(), } } + pub fn vector(&self) -> &VectorUnit { + &self.vector + } + + pub fn vector_mut(&mut self) -> &mut VectorUnit { + &mut self.vector + } + pub fn set_gprs(&mut self, gprs: GeneralPurposeRegisters) { self.gprs = gprs; } diff --git a/security-monitor/src/core/architecture/riscv/mod.rs b/security-monitor/src/core/architecture/riscv/mod.rs index 3d63b6d8..a29b0b1c 100644 --- a/security-monitor/src/core/architecture/riscv/mod.rs +++ b/security-monitor/src/core/architecture/riscv/mod.rs @@ -7,6 +7,7 @@ pub use extensions::HardwareExtension; pub use extensions::compressed_instructions::decode_result_register; pub use extensions::floating_point_unit::FloatingPointUnit; pub use extensions::supervisor_timer_extension::SupervisorTimerExtension; +pub use extensions::vector_unit::VectorUnit; pub use general_purpose_registers::{GeneralPurposeRegister, GeneralPurposeRegisters}; pub use hart_architectural_state::HartArchitecturalState; pub use hart_lifecycle_state::HartLifecycleState; diff --git a/security-monitor/src/core/architecture/riscv/specification.rs b/security-monitor/src/core/architecture/riscv/specification.rs index 5234f746..5b6beb41 100644 --- a/security-monitor/src/core/architecture/riscv/specification.rs +++ b/security-monitor/src/core/architecture/riscv/specification.rs @@ -422,6 +422,9 @@ pub const MTVEC_BASE_SHIFT: usize = 2; pub const CSR_STATUS_SIE: usize = 1; +pub const SR_VS: usize = 0x00000600; +pub const SR_VS_INITIAL: usize = 0x00000200; +pub const SR_VS_DIRTY: usize = 0x00000600; pub const SR_FS: usize = 0x00006000; pub const SR_FS_OFF: usize = 0x00000000; pub const SR_FS_INITIAL: usize = 0x00002000; diff --git a/security-monitor/src/core/control_data/confidential_hart.rs b/security-monitor/src/core/control_data/confidential_hart.rs index 7f486cef..bb22561c 100644 --- a/security-monitor/src/core/control_data/confidential_hart.rs +++ b/security-monitor/src/core/control_data/confidential_hart.rs @@ -116,6 +116,11 @@ impl ConfidentialHart { confidential_hart_state.csrs_mut().mstatus.enable_bits_on_saved_value(SR_FS_INITIAL); } + if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + confidential_hart_state.csrs_mut().mstatus.enable_bits_on_saved_value(SR_VS_INITIAL); + confidential_hart_state.csrs_mut().vsstatus.enable_bits_on_saved_value(SR_VS_INITIAL); + } + Self { confidential_vm_id: None, confidential_hart_state, @@ -171,6 +176,11 @@ impl ConfidentialHart { self.csrs_mut().vsstatus.disable_bits_on_saved_value(SR_FS_DIRTY); self.csrs_mut().vsstatus.enable_bits_on_saved_value(SR_FS_CLEAN); } + + if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); + unsafe { self.confidential_hart_state.vector_mut().save_in_main_memory() }; + } } /// Restores the state of the confidential hart from the main memory. This is part of the heavy context switch. @@ -186,6 +196,11 @@ impl ConfidentialHart { // below unsafe is ok because we checked that FPU hardware is available and we enabled it in mstatus. unsafe { self.confidential_hart_state.fprs_mut().restore_from_main_memory() }; } + + if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); + unsafe { self.confidential_hart_state.vector().restore_from_main_memory() }; + } } pub fn measure(&self, digest: &mut MeasurementDigest) { diff --git a/security-monitor/src/core/control_data/hypervisor_hart.rs b/security-monitor/src/core/control_data/hypervisor_hart.rs index 5f8c1a18..432908f7 100644 --- a/security-monitor/src/core/control_data/hypervisor_hart.rs +++ b/security-monitor/src/core/control_data/hypervisor_hart.rs @@ -2,7 +2,7 @@ // SPDX-FileContributor: Wojciech Ozga , IBM Research - Zurich // SPDX-License-Identifier: Apache-2.0 use crate::core::architecture::riscv::sbi::NaclSharedMemory; -use crate::core::architecture::riscv::specification::SR_FS; +use crate::core::architecture::riscv::specification::{SR_FS, SR_VS_DIRTY}; use crate::core::architecture::specification::SR_FS_DIRTY; use crate::core::architecture::{ ControlStatusRegisters, GeneralPurposeRegisters, HardwareExtension, HartArchitecturalState, SupervisorTimerExtension, @@ -52,6 +52,11 @@ impl HypervisorHart { } } } + + if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); + unsafe { self.hypervisor_hart_state.vector_mut().save_in_main_memory() }; + } } /// Restores the state of the hypervisor hart from the main memory. This is part of the heavy context switch. @@ -65,6 +70,11 @@ impl HypervisorHart { // below unsafe is ok because we checked that FPU hardware is available and we enabled it in mstatus. unsafe { self.hypervisor_hart_state.fprs_mut().restore_from_main_memory() }; } + + if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); + unsafe { self.hypervisor_hart_state.vector().restore_from_main_memory() }; + } } pub fn set_shared_memory(&mut self, base_address: NonConfidentialMemoryAddress) -> Result<(), Error> { diff --git a/security-monitor/src/non_confidential_flow/lightweight_context_switch/enter_from_hypervisor_or_vm.S b/security-monitor/src/non_confidential_flow/lightweight_context_switch/enter_from_hypervisor_or_vm.S index cc070697..8edaec3d 100644 --- a/security-monitor/src/non_confidential_flow/lightweight_context_switch/enter_from_hypervisor_or_vm.S +++ b/security-monitor/src/non_confidential_flow/lightweight_context_switch/enter_from_hypervisor_or_vm.S @@ -61,6 +61,8 @@ enter_from_hypervisor_or_vm_asm: sd t0, ({HART_A0_OFFSET})(a0) # Set the stack for the security monitor execution on this physical hart - ld sp, ({HART_STACK_ADDRESS_OFFSET})(a0) + li t0, {HART_STACK_ADDRESS_OFFSET} + add t0, a0, t0 + ld sp, 0(t0) j route_trap_from_hypervisor_or_vm \ No newline at end of file From f8137af86a13cada8d92bd7b3900c12aa6d0fd18 Mon Sep 17 00:00:00 2001 From: whensun Date: Thu, 29 Oct 2026 20:11:27 -0700 Subject: [PATCH 2/4] Remove temporary VectorUnit diagnostics Signed-off-by: whensun --- .../riscv/extensions/vector_unit.rs | 34 ++----------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs b/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs index 1e3a63ba..94460353 100644 --- a/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs +++ b/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs @@ -22,30 +22,10 @@ pub struct VectorUnit { impl VectorUnit { pub fn empty() -> Self { - debug!("ACE_DIAG: Enter VectorUnit::empty"); - - let sp: usize; - - unsafe { - asm!( - "mv {value}, sp", - value = out(reg) sp, - options(nomem, nostack) - ); - } - - debug!("ACE_DIAG: VectorUnit::empty ENTRY sp={:#x} initialized={}", sp, HardwareSetup::is_initialized()); - - let vector_supported = HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension); - - debug!("ACE_DIAG: VectorUnit supported = {}", vector_supported); - - if !vector_supported { + if !HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { return Self { registers: Vec::new(), vlenb: 0, vl: 0, vtype: 0, vstart: 0, vcsr: 0 }; } - debug!("ACE_DIAG: BEFORE mstatus/CSR, initialized={}", HardwareSetup::is_initialized()); - let old_mstatus = CSR.mstatus.read_and_set_bits(SR_VS_DIRTY); let vlenb: usize; @@ -59,21 +39,11 @@ impl VectorUnit { CSR.mstatus.write(old_mstatus); - debug!("ACE_DIAG: AFTER mstatus/CSR, initialized={}, vlenb={}", HardwareSetup::is_initialized(), vlenb); - assert!(vlenb > 0); let length = NUMBER_OF_VECTOR_REGISTERS.checked_mul(vlenb).expect("Invalid vector register size"); - debug!("ACE_DIAG: BEFORE vector allocation, initialized={}, length={}", HardwareSetup::is_initialized(), length); - - let registers = vec![0; length]; - - debug!("ACE_DIAG: AFTER vector allocation, initialized={}, ptr={:p}", HardwareSetup::is_initialized(), registers.as_ptr()); - - debug!("ACE_DIAG: Leaving VectorUnit::empty"); - - Self { registers, vlenb, vl: 0, vtype: 0, vstart: 0, vcsr: 0 } + Self { registers: vec![0; length], vlenb, vl: 0, vtype: 0, vstart: 0, vcsr: 0 } } unsafe fn validate_vlenb(&self) { From dbc8b67699bb97a94d18917f9a2d957356bd274b Mon Sep 17 00:00:00 2001 From: whensun Date: Tue, 3 Nov 2026 21:15:34 -0700 Subject: [PATCH 3/4] Fix false-positive RISC-V Vector extension detection Signed-off-by: whensun --- security-monitor/src/core/initialization/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/security-monitor/src/core/initialization/mod.rs b/security-monitor/src/core/initialization/mod.rs index 7b8ccf8c..4a04966a 100644 --- a/security-monitor/src/core/initialization/mod.rs +++ b/security-monitor/src/core/initialization/mod.rs @@ -95,8 +95,10 @@ fn verify_harts(fdt: &FlattenedDeviceTree) -> Result { HardwareExtension::all().into_iter().for_each(|ext| { let is_extension_supported_by_all_harts = fdt.harts().all(|hart| { let prop = hart.property_str(FDT_RISCV_ISA).ok_or(Error::FdtParsing()).unwrap_or(""); - let extensions = &prop.split('_').collect::>(); - extensions[0].contains(&ext.code()) || extensions.contains(&ext.code()) + let extensions = prop.split('_').collect::>(); + let base_extensions = extensions[0].strip_prefix(RISCV_ARCH).unwrap_or(""); + + base_extensions.contains(ext.code()) || extensions.iter().skip(1).any(|extension| *extension == ext.code()) }); if is_extension_supported_by_all_harts { debug!("Enabling support for extension: {:?}", ext); From d237db47f7059ea65a14d5a29c1474303271b92b Mon Sep 17 00:00:00 2001 From: whensun Date: Sat, 7 Nov 2026 14:41:30 -0700 Subject: [PATCH 4/4] Address review feedback on vector state preservation Signed-off-by: whensun --- .../riscv/extensions/vector_unit.rs | 176 +++++++----------- .../riscv/hart_architectural_state.rs | 19 +- .../src/core/architecture/riscv/mod.rs | 1 + .../core/control_data/confidential_hart.rs | 32 ++-- .../src/core/control_data/hardware_hart.rs | 11 +- .../src/core/control_data/hypervisor_hart.rs | 16 +- .../src/core/hardware_setup/mod.rs | 54 +++++- .../src/core/initialization/mod.rs | 15 +- security-monitor/src/error.rs | 2 + .../promote_to_confidential_vm.rs | 4 +- 10 files changed, 178 insertions(+), 152 deletions(-) diff --git a/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs b/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs index 94460353..a97c9f41 100644 --- a/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs +++ b/security-monitor/src/core/architecture/riscv/extensions/vector_unit.rs @@ -1,10 +1,11 @@ // SPDX-FileCopyrightText: 2023 IBM Corporation // SPDX-FileContributor: Wojciech Ozga , IBM Research - Zurich // SPDX-License-Identifier: Apache-2.0 -use crate::core::architecture::riscv::specification::{CSR_VCSR, CSR_VL, CSR_VLENB, CSR_VSTART, CSR_VTYPE, SR_VS_DIRTY}; -use crate::core::architecture::{CSR, HardwareExtension}; + +use crate::core::architecture::riscv::control_status_registers::ReadWriteRiscvCsr; +use crate::core::architecture::riscv::specification::{CSR_VCSR, CSR_VL, CSR_VSTART, CSR_VTYPE}; use crate::core::hardware_setup::HardwareSetup; -use alloc::vec; +use crate::error::Error; use alloc::vec::Vec; use core::arch::asm; @@ -14,54 +15,44 @@ const NUMBER_OF_VECTOR_REGISTERS: usize = 32; pub struct VectorUnit { registers: Vec, vlenb: usize, - vl: usize, - vtype: usize, - vstart: usize, - vcsr: usize, + vl: ReadWriteRiscvCsr, + vtype: ReadWriteRiscvCsr, + vstart: ReadWriteRiscvCsr, + vcsr: ReadWriteRiscvCsr, } impl VectorUnit { - pub fn empty() -> Self { - if !HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { - return Self { registers: Vec::new(), vlenb: 0, vl: 0, vtype: 0, vstart: 0, vcsr: 0 }; - } - - let old_mstatus = CSR.mstatus.read_and_set_bits(SR_VS_DIRTY); - - let vlenb: usize; - unsafe { - asm!( - "csrr {value}, {csr}", - value = out(reg) vlenb, - csr = const CSR_VLENB - ); - } - - CSR.mstatus.write(old_mstatus); - - assert!(vlenb > 0); - - let length = NUMBER_OF_VECTOR_REGISTERS.checked_mul(vlenb).expect("Invalid vector register size"); - - Self { registers: vec![0; length], vlenb, vl: 0, vtype: 0, vstart: 0, vcsr: 0 } - } - - unsafe fn validate_vlenb(&self) { - let mut vlenb: usize; - unsafe { asm!("csrr {value}, {csr}", value = out(reg) vlenb, csr = const CSR_VLENB) }; - assert_eq!(self.vlenb, vlenb); - assert_eq!(self.registers.len(), NUMBER_OF_VECTOR_REGISTERS * vlenb); + pub fn empty() -> Result { + let vlenb = HardwareSetup::vector_vlenb()?; + + let length = NUMBER_OF_VECTOR_REGISTERS.checked_mul(vlenb).ok_or(Error::InvalidVectorRegisterSize())?; + + let mut registers = Vec::new(); + registers.try_reserve_exact(length).map_err(|_| Error::NotEnoughMemory())?; + registers.resize(length, 0); + + Ok(Self { + registers, + vlenb, + vl: ReadWriteRiscvCsr::new(), + vtype: ReadWriteRiscvCsr::new(), + vstart: ReadWriteRiscvCsr::new(), + vcsr: ReadWriteRiscvCsr::new(), + }) } + /// Saves vector state in main memory. + /// + /// # Safety + /// + /// The vector extension must be supported and enabled. pub unsafe fn save_in_main_memory(&mut self) { - unsafe { self.validate_vlenb() }; - unsafe { - asm!("csrr {value}, {csr}", value = out(reg) self.vl, csr = const CSR_VL); - asm!("csrr {value}, {csr}", value = out(reg) self.vtype, csr = const CSR_VTYPE); - asm!("csrr {value}, {csr}", value = out(reg) self.vstart, csr = const CSR_VSTART); - asm!("csrr {value}, {csr}", value = out(reg) self.vcsr, csr = const CSR_VCSR); - asm!("csrw {csr}, zero", csr = const CSR_VSTART); - } + self.vl.save_in_main_memory(); + self.vtype.save_in_main_memory(); + self.vstart.save_in_main_memory(); + self.vcsr.save_in_main_memory(); + + self.vstart.write(0); macro_rules! store { ($index:literal) => { @@ -69,51 +60,34 @@ impl VectorUnit { asm!( ".option push", ".option arch, +v", - concat!("vs1r.v v", stringify!($index), ", ({address})"), + concat!( + "vs8r.v v", + stringify!($index), + ", ({address})" + ), ".option pop", - address = in(reg) self.registers.as_mut_ptr().add($index * self.vlenb) + address = in(reg) self.registers + .as_mut_ptr() + .add($index * self.vlenb), ); } }; } store!(0); - store!(1); - store!(2); - store!(3); - store!(4); - store!(5); - store!(6); - store!(7); store!(8); - store!(9); - store!(10); - store!(11); - store!(12); - store!(13); - store!(14); - store!(15); store!(16); - store!(17); - store!(18); - store!(19); - store!(20); - store!(21); - store!(22); - store!(23); store!(24); - store!(25); - store!(26); - store!(27); - store!(28); - store!(29); - store!(30); - store!(31); } + /// Restores vector state from main memory. + /// + /// # Safety + /// + /// The vector extension must be supported and enabled. + /// The current hart must have a compatible vector configuration. pub unsafe fn restore_from_main_memory(&self) { - unsafe { self.validate_vlenb() }; - unsafe { asm!("csrw {csr}, zero", csr = const CSR_VSTART) }; + self.vstart.write(0); macro_rules! load { ($index:literal) => { @@ -121,57 +95,37 @@ impl VectorUnit { asm!( ".option push", ".option arch, +v", - concat!("vl1re8.v v", stringify!($index), ", ({address})"), + concat!( + "vl8re8.v v", + stringify!($index), + ", ({address})" + ), ".option pop", - address = in(reg) self.registers.as_ptr().add($index * self.vlenb) + address = in(reg) self.registers + .as_ptr() + .add($index * self.vlenb), ); } }; } load!(0); - load!(1); - load!(2); - load!(3); - load!(4); - load!(5); - load!(6); - load!(7); load!(8); - load!(9); - load!(10); - load!(11); - load!(12); - load!(13); - load!(14); - load!(15); load!(16); - load!(17); - load!(18); - load!(19); - load!(20); - load!(21); - load!(22); - load!(23); load!(24); - load!(25); - load!(26); - load!(27); - load!(28); - load!(29); - load!(30); - load!(31); + unsafe { asm!( ".option push", ".option arch, +v", "vsetvl zero, {saved_vl}, {saved_vtype}", ".option pop", - saved_vl = in(reg) self.vl, - saved_vtype = in(reg) self.vtype + saved_vl = in(reg) self.vl.read_from_main_memory(), + saved_vtype = in(reg) self.vtype.read_from_main_memory(), ); - asm!("csrw {csr}, {value}", csr = const CSR_VCSR, value = in(reg) self.vcsr); - asm!("csrw {csr}, {value}", csr = const CSR_VSTART, value = in(reg) self.vstart); } + + self.vcsr.restore_from_main_memory(); + self.vstart.restore_from_main_memory(); } } diff --git a/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs b/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs index 402bb940..631a9091 100644 --- a/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs +++ b/security-monitor/src/core/architecture/riscv/hart_architectural_state.rs @@ -6,6 +6,7 @@ use crate::core::architecture::riscv::specification::*; use crate::core::architecture::riscv::{ ControlStatusRegisters, FloatingPointUnit, GeneralPurposeRegister, GeneralPurposeRegisters, VectorUnit, }; +use crate::error::Error; /// Defines the state of a processor's core (hart) when stored in main memory. #[repr(C)] @@ -14,26 +15,26 @@ pub struct HartArchitecturalState { csrs: ControlStatusRegisters, fprs: FloatingPointUnit, sstc: SupervisorTimerExtension, - vector: VectorUnit, + vrs: VectorUnit, } impl HartArchitecturalState { - pub fn empty() -> Self { - Self { + pub fn empty() -> Result { + Ok(Self { gprs: GeneralPurposeRegisters::empty(), csrs: ControlStatusRegisters::empty(), fprs: FloatingPointUnit::empty(), sstc: SupervisorTimerExtension::empty(), - vector: VectorUnit::empty(), - } + vrs: VectorUnit::empty()?, + }) } - pub fn vector(&self) -> &VectorUnit { - &self.vector + pub fn vrs(&self) -> &VectorUnit { + &self.vrs } - pub fn vector_mut(&mut self) -> &mut VectorUnit { - &mut self.vector + pub fn vrs_mut(&mut self) -> &mut VectorUnit { + &mut self.vrs } pub fn set_gprs(&mut self, gprs: GeneralPurposeRegisters) { diff --git a/security-monitor/src/core/architecture/riscv/mod.rs b/security-monitor/src/core/architecture/riscv/mod.rs index a29b0b1c..d9876b41 100644 --- a/security-monitor/src/core/architecture/riscv/mod.rs +++ b/security-monitor/src/core/architecture/riscv/mod.rs @@ -2,6 +2,7 @@ // SPDX-FileContributor: Wojciech Ozga , IBM Research - Zurich // SPDX-License-Identifier: Apache-2.0 #![allow(unused)] +pub(crate) use control_status_registers::ReadWriteRiscvCsr; pub use control_status_registers::{CSR, ControlStatusRegister, ControlStatusRegisters}; pub use extensions::HardwareExtension; pub use extensions::compressed_instructions::decode_result_register; diff --git a/security-monitor/src/core/control_data/confidential_hart.rs b/security-monitor/src/core/control_data/confidential_hart.rs index bb22561c..39acd4b2 100644 --- a/security-monitor/src/core/control_data/confidential_hart.rs +++ b/security-monitor/src/core/control_data/confidential_hart.rs @@ -67,23 +67,23 @@ impl ConfidentialHart { /// confidential hart has been assigned to a hardware hart for execution. A dummy confidential hart has the id of the hardware hart it /// represents. This is required to track information on which hardware hart a stolen confidential hart executes (for example to /// interrupt it with IPI). - pub fn dummy(hardware_hart_id: usize) -> Self { + pub fn dummy(hardware_hart_id: usize) -> Result { // We set the lifecycle state of the dummy hart to `Started` because this state will be used as the state of the confidential hart // that has been assigned to physical hart. Specifically, after a confidential hart gets assigned to hardware hart, the confidential // VM will be left with a dummy hart. Any other code or request that will inspect the state of the (stolen) confidential hart, will // in reality look into the state of the dummy hart which will correctly say that the confidential hart is in the `Started` state. - Self { + Ok(Self { confidential_vm_id: None, - confidential_hart_state: HartArchitecturalState::empty(), + confidential_hart_state: HartArchitecturalState::empty()?, lifecycle_state: HartLifecycleState::Started, resumable_operation: None, id: hardware_hart_id, - } + }) } /// Constructs a confidential hart with the state after a reset. - pub fn from_vm_hart_reset(id: usize, htimedelta: usize, shared_memory: &NaclSharedMemory) -> Self { - let mut confidential_hart_state = HartArchitecturalState::empty(); + pub fn from_vm_hart_reset(id: usize, htimedelta: usize, shared_memory: &NaclSharedMemory) -> Result { + let mut confidential_hart_state = HartArchitecturalState::empty()?; confidential_hart_state.csrs_mut().mstatus.save_value_in_main_memory(Self::INITIAL_MSTATUS); confidential_hart_state.csrs_mut().sstatus.save_value_in_main_memory(Self::INITIAL_SSTATUS); confidential_hart_state.csrs_mut().hstatus.save_value_in_main_memory(Self::INITIAL_HSTATUS); @@ -121,22 +121,22 @@ impl ConfidentialHart { confidential_hart_state.csrs_mut().vsstatus.enable_bits_on_saved_value(SR_VS_INITIAL); } - Self { + Ok(Self { confidential_vm_id: None, confidential_hart_state, lifecycle_state: HartLifecycleState::Stopped, resumable_operation: None, id, - } + }) } /// Constructs a confidential hart with the state of the non-confidential hart that made a call to promote the VM to confidential VM pub fn from_vm_hart( id: usize, program_counter: usize, fdt_address: &ConfidentialVmPhysicalAddress, htimedelta: usize, shared_memory: &NaclSharedMemory, - ) -> Self { + ) -> Result { // We first create a confidential hart in the reset state and then fill this state with the runtime state of the hart that made a // call to promote to confidential VM. This state consists of GPRs and VS-level CSRs. - let mut confidential_hart = Self::from_vm_hart_reset(id, htimedelta, shared_memory); + let mut confidential_hart = Self::from_vm_hart_reset(id, htimedelta, shared_memory)?; let confidential_hart_state = &mut confidential_hart.confidential_hart_state; confidential_hart_state.set_gprs(shared_memory.gprs()); confidential_hart_state.gprs_mut().write(GeneralPurposeRegister::a0, id); @@ -152,7 +152,7 @@ impl ConfidentialHart { // Store the program counter of the VM, so that we can resume confidential VM at the point it became promoted. confidential_hart_state.csrs_mut().mepc.save_value_in_main_memory(program_counter); confidential_hart.lifecycle_state = HartLifecycleState::Started; - confidential_hart + Ok(confidential_hart) } /// Saves the state of the confidential hart in the main memory. This is part of the heavy context switch. @@ -178,8 +178,11 @@ impl ConfidentialHart { } if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + // Enable the vector unit in mstatus to access vector + // registers and execute vector instructions. self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); - unsafe { self.confidential_hart_state.vector_mut().save_in_main_memory() }; + + unsafe { self.confidential_hart_state.vrs_mut().save_in_main_memory() }; } } @@ -198,8 +201,11 @@ impl ConfidentialHart { } if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + // Enable the vector unit in mstatus to access vector + // registers and execute vector instructions. self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); - unsafe { self.confidential_hart_state.vector().restore_from_main_memory() }; + + unsafe { self.confidential_hart_state.vrs().restore_from_main_memory() }; } } diff --git a/security-monitor/src/core/control_data/hardware_hart.rs b/security-monitor/src/core/control_data/hardware_hart.rs index 5b8d95be..c3cff2fe 100644 --- a/security-monitor/src/core/control_data/hardware_hart.rs +++ b/security-monitor/src/core/control_data/hardware_hart.rs @@ -5,6 +5,7 @@ use crate::core::architecture::CSR; use crate::core::control_data::{ConfidentialHart, HypervisorHart}; use crate::core::memory_protector::HypervisorMemoryProtector; use crate::core::page_allocator::{Allocated, Page, UnAllocated}; +use crate::error::Error; use opensbi_sys::sbi_trap_regs; unsafe extern "C" { @@ -43,14 +44,14 @@ pub struct HardwareHart { impl HardwareHart { /// Creates the instance of a state associated with the physical hart. Id is the unique physical hart identifier used to by hardware /// during inter process interrupts (IPIs), e.g., on RISC-V this will be a number reported by mhartid. - pub fn init(id: usize, stack: Page, hypervisor_memory_protector: HypervisorMemoryProtector) -> Self { - Self { - hypervisor_hart: HypervisorHart::new(hypervisor_memory_protector), - confidential_hart: ConfidentialHart::dummy(id), + pub fn init(id: usize, stack: Page, hypervisor_memory_protector: HypervisorMemoryProtector) -> Result { + Ok(Self { + hypervisor_hart: HypervisorHart::new(hypervisor_memory_protector)?, + confidential_hart: ConfidentialHart::dummy(id)?, stack_address: stack.end_address(), stack: stack.zeroize(), previous_mscratch: 0, - } + }) } // Safety: this function can be called only once during initialization. We cannot do it in constructor, because physical harts art not diff --git a/security-monitor/src/core/control_data/hypervisor_hart.rs b/security-monitor/src/core/control_data/hypervisor_hart.rs index 432908f7..942b0b0e 100644 --- a/security-monitor/src/core/control_data/hypervisor_hart.rs +++ b/security-monitor/src/core/control_data/hypervisor_hart.rs @@ -28,12 +28,12 @@ pub struct HypervisorHart { } impl HypervisorHart { - pub fn new(hypervisor_memory_protector: HypervisorMemoryProtector) -> Self { - Self { - hypervisor_hart_state: HartArchitecturalState::empty(), + pub fn new(hypervisor_memory_protector: HypervisorMemoryProtector) -> Result { + Ok(Self { + hypervisor_hart_state: HartArchitecturalState::empty()?, shared_memory: NaclSharedMemory::uninitialized(), hypervisor_memory_protector, - } + }) } /// Saves the state of the hypervisor hart in the main memory. This is part of the heavy context switch. @@ -54,8 +54,10 @@ impl HypervisorHart { } if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + // Enable the vector unit in mstatus to access vector + // registers and execute vector instructions. self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); - unsafe { self.hypervisor_hart_state.vector_mut().save_in_main_memory() }; + unsafe { self.hypervisor_hart_state.vrs_mut().save_in_main_memory() }; } } @@ -72,8 +74,10 @@ impl HypervisorHart { } if HardwareSetup::is_extension_supported(HardwareExtension::VectorExtension) { + // Enable the vector unit in mstatus to access vector + // registers and execute vector instructions. self.csrs().mstatus.read_and_set_bits(SR_VS_DIRTY); - unsafe { self.hypervisor_hart_state.vector().restore_from_main_memory() }; + unsafe { self.hypervisor_hart_state.vrs().restore_from_main_memory() }; } } diff --git a/security-monitor/src/core/hardware_setup/mod.rs b/security-monitor/src/core/hardware_setup/mod.rs index cd082036..23e1bb85 100644 --- a/security-monitor/src/core/hardware_setup/mod.rs +++ b/security-monitor/src/core/hardware_setup/mod.rs @@ -1,41 +1,50 @@ // SPDX-FileCopyrightText: 2023 IBM Corporation // SPDX-FileContributor: Wojciech Ozga , IBM Research - Zurich // SPDX-License-Identifier: Apache-2.0 -use crate::core::architecture::HardwareExtension; + +use crate::core::architecture::riscv::ReadWriteRiscvCsr; use crate::core::architecture::riscv::specification::*; +use crate::core::architecture::{CSR, HardwareExtension}; use crate::error::Error; use alloc::vec::Vec; use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; -/// Global static description of hardware setup. It is created during the system initiization and used in runtime to perform operations that -/// depend on the presence of specific hardware features, like, for example, floating point unit extension. static HARDWARE_SETUP: Once> = Once::new(); pub struct HardwareSetup { isa_extensions: Vec, + vector_vlenb: Option, } impl HardwareSetup { const NOT_INITIALIZED: &'static str = "Bug: Hardware configuration setup not initialized"; + const REQUIRED_BASE_EXTENSIONS: &'static [&'static str] = &[ATOMIC_EXTENSION, HYPERVISOR_EXTENSION]; + const REQUIRED_EXTENSIONS: &'static [&'static str] = &[SSTC_EXTENSION, IFENCEI_EXTENSION]; pub fn initialize() -> Result<(), Error> { - let hardware_setup = Self { isa_extensions: Vec::new() }; + let hardware_setup = Self { isa_extensions: Vec::new(), vector_vlenb: None }; + ensure_not!(HARDWARE_SETUP.is_completed(), Error::Reinitialization())?; + HARDWARE_SETUP.call_once(|| RwLock::new(hardware_setup)); Ok(()) } pub fn check_isa_extensions(prop: &str) -> Result<(), Error> { - // example prop value rv64imafdch_zicsr_zifencei_zba_zbb_zbc_zbs debug!("{}", prop); + ensure!(prop.starts_with(RISCV_ARCH), Error::InvalidCpuArch())?; + let extensions = &prop.split('_').collect::>(); + Self::REQUIRED_BASE_EXTENSIONS .into_iter() .try_for_each(|ext| ensure!(extensions[0].contains(*ext), Error::MissingCpuExtension()))?; + Self::REQUIRED_EXTENSIONS.into_iter().try_for_each(|ext| ensure!(extensions.contains(ext), Error::MissingCpuExtension()))?; + Ok(()) } @@ -47,6 +56,41 @@ impl HardwareSetup { Self::try_read(|hardware_setup| Ok(hardware_setup.isa_extensions.contains(&extension))).unwrap_or(false) } + pub fn vector_vlenb() -> Result { + Self::try_read(|hardware_setup| { + if !hardware_setup.isa_extensions.contains(&HardwareExtension::VectorExtension) { + return Ok(0); + } + + hardware_setup.vector_vlenb.ok_or(Error::InvalidVectorRegisterSize()) + }) + } + + pub fn validate_vector_vlenb() -> Result<(), Error> { + if !Self::is_extension_supported(HardwareExtension::VectorExtension) { + return Ok(()); + } + + let old_mstatus = CSR.mstatus.read_and_set_bits(SR_VS_DIRTY); + + let vlenb = ReadWriteRiscvCsr::::new().read(); + + CSR.mstatus.write(old_mstatus); + + if !vlenb.is_power_of_two() || vlenb > 8192 { + return Err(Error::InvalidVectorRegisterSize()); + } + + Self::try_write(|hardware_setup| match hardware_setup.vector_vlenb { + None => { + hardware_setup.vector_vlenb = Some(vlenb); + Ok(()) + } + Some(expected) if expected == vlenb => Ok(()), + Some(_) => Err(Error::InvalidVectorRegisterSize()), + }) + } + fn try_read(op: O) -> Result where O: FnOnce(&RwLockReadGuard<'_, Self>) -> Result { op(&HARDWARE_SETUP.get().expect(Self::NOT_INITIALIZED).read()) diff --git a/security-monitor/src/core/initialization/mod.rs b/security-monitor/src/core/initialization/mod.rs index 4a04966a..1efe0d29 100644 --- a/security-monitor/src/core/initialization/mod.rs +++ b/security-monitor/src/core/initialization/mod.rs @@ -45,6 +45,10 @@ extern "C" fn init_security_monitor_asm(cold_boot: bool, flattened_device_tree_a if let Err(error) = init_security_monitor(flattened_device_tree_address) { // TODO: lock access to attestation keys/seed/credentials. debug!("Failed to initialize the security monitor: {:?}", error); + + loop { + core::hint::spin_loop(); + } } } } @@ -108,6 +112,7 @@ fn verify_harts(fdt: &FlattenedDeviceTree) -> Result { // TODO: make sure there are enough PMPs + HardwareSetup::validate_vector_vlenb()?; Ok(fdt.harts().count()) } @@ -201,7 +206,7 @@ fn prepare_harts(number_of_harts: usize) -> Result<(), Error> { let stack = PageAllocator::acquire_page(PageSize::Size2MiB)?; let hypervisor_memory_protector = HypervisorMemoryProtector::create(); debug!("Hart[{}] stack \t 0x{:x}-0x{:x}", hart_id, stack.start_address(), stack.end_address()); - harts_states.insert(hart_id, HardwareHart::init(hart_id, stack, hypervisor_memory_protector)); + harts_states.insert(hart_id, HardwareHart::init(hart_id, stack, hypervisor_memory_protector)?); } HARTS_STATES.call_once(|| Mutex::new(harts_states)); fence_wo(); @@ -220,6 +225,14 @@ extern "C" fn ace_setup_this_hart() { let hart_id = CSR.mhartid.read(); debug!("Setting up hardware hart id={}", hart_id); + if let Err(error) = HardwareSetup::validate_vector_vlenb() { + debug!("Hart {} has incompatible vector configuration: {:?}", CSR.mhartid.read(), error); + + loop { + core::hint::spin_loop(); + } + } + // OpenSBI requires that mscratch points to an internal OpenSBI's structure. We have to store this pointer during // init and restore it every time we delegate exception/interrupt to the Sbi firmware (e.g., OpenSbi). let mut harts = HARTS_STATES.get().expect("Bug. Could not set mscratch before initializing memory region for harts states").lock(); diff --git a/security-monitor/src/error.rs b/security-monitor/src/error.rs index b0f8fb01..3f5cb21b 100644 --- a/security-monitor/src/error.rs +++ b/security-monitor/src/error.rs @@ -22,6 +22,8 @@ pub enum Error { MissingCpuExtension(), #[error("Not supported hardware: Not enough PMPs")] NotEnoughPmps(), + #[error("Not supported hardware: Invalid or inconsistent vector register size")] + InvalidVectorRegisterSize(), #[error("Error while searching FDT for a property")] FdtParsing(), diff --git a/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/promote_to_confidential_vm.rs b/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/promote_to_confidential_vm.rs index 4564e963..cec9f365 100644 --- a/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/promote_to_confidential_vm.rs +++ b/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/promote_to_confidential_vm.rs @@ -90,14 +90,14 @@ impl PromoteToConfidentialVm { debug!("Number of confidential harts: {}", number_of_confidential_harts); // We create a fixed number of harts (all but the boot hart are in the reset state). - let confidential_harts: Vec<_> = (0..number_of_confidential_harts) + let confidential_harts: Vec = (0..number_of_confidential_harts) .map(|id| match id { Self::BOOT_HART_ID => { ConfidentialHart::from_vm_hart(id, self.program_counter, &fdt_address, self.htimedelta, shared_memory) } _ => ConfidentialHart::from_vm_hart_reset(id, self.htimedelta, shared_memory), }) - .collect(); + .collect::, Error>>()?; let payload = self .read_attestation_payload(large_page, &memory_protector)