From 60fe23ae4314d9f84e1acae1fc25b8f345d854e9 Mon Sep 17 00:00:00 2001 From: Flgodd67 Date: Tue, 9 Jun 2026 16:52:30 +0100 Subject: [PATCH 1/5] cca: Instruction Abort and testing - unit test added that causes an Instruction Abort by fetching from an address outside the PAR - unit test added that causes an Instruction Abort by fetching from an address with state RIPAS_EMPTY - unit test added that causes an Instruction Abort by fetching from an address with no permissions - added a HpfarEl2 structure in order to retrieve the fipa more conveniently. Added the getter function in CcaExit. - added ioctl in openVMM and kernel to get ipa state - hcl_rsi_ipa_state_read - adding a new location where the AArch64 tests can be placed: tmk/simple_tmk/src/aarch64. Keeping architecture specific code and tests grouped together - adding Instruction abort handling, displaying to the user the error reason with additional useful information about the plane exit - implementation for #[tmk_test(expected_failure)] macro. Tests marked as expected failures now pass when they fail or fault. Allows representation of intentional failure cases. --- .../src/_jobs/local_install_cca_emu.rs | 2 +- openhcl/hcl/src/ioctl.rs | 9 + openhcl/hcl/src/ioctl/cca.rs | 39 ++++ .../virt_mshv_vtl/src/processor/cca/mod.rs | 167 +++++++++++++++++- tmk/simple_tmk/src/aarch64/mod.rs | 70 ++++++++ tmk/simple_tmk/src/main.rs | 1 + tmk/tmk_core/src/lib.rs | 9 + tmk/tmk_macros/src/lib.rs | 19 +- tmk/tmk_protocol/src/lib.rs | 5 + tmk/tmk_vmm/src/load.rs | 2 + tmk/tmk_vmm/src/run.rs | 55 +++++- vm/aarch64/aarch64defs/src/lib.rs | 16 +- 12 files changed, 381 insertions(+), 13 deletions(-) create mode 100755 tmk/simple_tmk/src/aarch64/mod.rs diff --git a/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs b/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs index 82af5b79c1..7d90128a24 100644 --- a/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs +++ b/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs @@ -16,7 +16,7 @@ use std::thread; const SHRINKWRAP_REPO: &str = "https://git.gitlab.arm.com/tooling/shrinkwrap.git"; // The guest Linux kernel (with cca/plane driver) hasn't been upstreamed yet, fetch it from our private repo const PLANE0_LINUX_REPO: &str = "https://github.com/jiong-microsoft/OHCL-Linux-Kernel.git"; -const PLANE0_LINUX_BRANCH: &str = "cca-dev"; +const PLANE0_LINUX_BRANCH: &str = "cca-dev-ben"; // A few config information needed when building Linux kernel const CCA_CONFIGS: &[&str] = &["CONFIG_VIRT_DRIVERS", "CONFIG_ARM_CCA_GUEST"]; const NINEP_CONFIGS: &[&str] = &[ diff --git a/openhcl/hcl/src/ioctl.rs b/openhcl/hcl/src/ioctl.rs index f46d919bfa..0878f9fbc4 100755 --- a/openhcl/hcl/src/ioctl.rs +++ b/openhcl/hcl/src/ioctl.rs @@ -376,6 +376,7 @@ pub(crate) mod ioctls { const MSHV_VTL_RSI_SYSREG_READ: u16 = 0x42; const MSHV_VTL_RSI_SYSREG_WRITE: u16 = 0x43; const MSHV_VTL_RSI_SET_MEM_PERM: u16 = 0x44; + const MSHV_VTL_RSI_GET_IPA_STATE: u16 = 0x45; #[repr(C)] #[derive(Copy, Clone)] @@ -617,6 +618,14 @@ pub(crate) mod ioctls { cca::mshv_rsi_sysreg_rw ); + // CCA: Get the RIPAS state of an ipa + ioctl_read!( + hcl_rsi_ipa_state_read, + MSHV_IOCTL, + MSHV_VTL_RSI_GET_IPA_STATE, + cca::mshv_rsi_get_ipa_state + ); + // CCA: Assign the address described by `mshv_rsi_set_mem_perm` // to a plane. // Note: This is a simplification of the memory access configuration. diff --git a/openhcl/hcl/src/ioctl/cca.rs b/openhcl/hcl/src/ioctl/cca.rs index 1c896da47c..07e51f6606 100755 --- a/openhcl/hcl/src/ioctl/cca.rs +++ b/openhcl/hcl/src/ioctl/cca.rs @@ -16,6 +16,7 @@ use crate::ioctl::GetRegError; use crate::ioctl::HvError; use crate::ioctl::SetRegError; use crate::ioctl::ioctls::hcl_realm_config; +use crate::ioctl::ioctls::hcl_rsi_ipa_state_read; use crate::ioctl::ioctls::hcl_rsi_set_mem_perm; use crate::ioctl::ioctls::hcl_rsi_sysreg_read; use crate::ioctl::ioctls::hcl_rsi_sysreg_write; @@ -70,6 +71,15 @@ pub struct mshv_rsi_set_mem_perm { pub top_addr: u64, } +/// CCA: +#[repr(C)] +#[derive(Clone, Copy, Default)] +#[expect(missing_docs)] +pub struct mshv_rsi_get_ipa_state { + pub fipa: u64, + pub state: u64, +} + /// SystemReg is following encoding used by MSR/MRS which is different with /// the encoding RSI is using. The latter doesn't left shift the register /// number. @@ -204,6 +214,15 @@ impl ProcessorRunner<'_, Cca> { .rsi_sysreg_read(vtl, encode_rsi_sysreg(name), value) } + /// Get the ipa ripas state from the RMM + pub fn cca_ipa_state_read( + &self, + vtl: GuestVtl, + state: &mut mshv_rsi_get_ipa_state, + ) -> Result<(), Error> { + self.hcl.rsi_get_ipa_state(vtl, state).map_err(|_| Error::InvalidRegisterValue) + } + /// Update the address of the `plane_run` structure in `mshv_vtl_run.context`. pub fn cca_set_plane_enter(&mut self) { // SAFETY: the run page is exclusively accessed through `&mut self` while @@ -469,6 +488,21 @@ impl MshvVtl { } Ok(()) } + + /// Get the ipa RIPAS state + pub fn rsi_get_ipa_state(&self, vtl: GuestVtl, plane_state: &mut mshv_rsi_get_ipa_state) -> Result<(), HvError> { + let _plane = match vtl { + GuestVtl::Vtl0 => 1, + _ => return Err(HvError::InvalidVtlState) + }; + + unsafe { + hcl_rsi_ipa_state_read(self.file.as_raw_fd(), plane_state) + .map_err(|_| HvError::InvalidVpState)?; + } + + Ok(()) + } } impl Hcl { @@ -501,4 +535,9 @@ impl Hcl { pub fn rsi_set_mem_perm(&self, vtl: GuestVtl, range: MemoryRange) -> Result<(), HvError> { self.mshv_vtl.rsi_set_mem_perm(vtl, &range) } + + /// getting ipa RIPAS state + pub fn rsi_get_ipa_state(&self, vtl: GuestVtl, plane_state: &mut mshv_rsi_get_ipa_state) -> Result<(), HvError> { + self.mshv_vtl.rsi_get_ipa_state(vtl, plane_state) + } } diff --git a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs index af67368d3d..8e1be5f861 100644 --- a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs @@ -17,11 +17,15 @@ use crate::UhCvmVpState; use crate::UhPartitionInner; use crate::processor::InterceptMessageState; use aarch64defs::EsrEl2; +use aarch64defs::FaultStatusCode; +use aarch64defs::HpfarEl2; use aarch64defs::IssDataAbort; +use aarch64defs::IssInstructionAbort; use aarch64defs::SystemReg; use aarch64defs::rsi::cca_rsi_plane_exit; use hcl::GuestVtl; use hcl::ioctl::cca::Cca; +use hcl::ioctl::cca::mshv_rsi_get_ipa_state; use hcl::ioctl::register; use hv1_emulator::hv::ProcessorVtlHv; use hv1_emulator::synic::ProcessorSynic; @@ -49,10 +53,117 @@ enum CcaUnsupportedExit { ExceptionClass { exception_class: u8, esr_el2: u64 }, #[error("CCA data abort with invalid instruction syndrome in ESR_EL2 {0:#x}")] InvalidDataAbortIss(u64), + #[error( + "CCA instruction abort: ESR_EL2 {esr_el2:#x}, ELR_EL2 {elr_el2:#x}, FAR_EL2 {far_el2:#x}, + FIPA {fipa:#x}, FIPA RIPAS state {fipa_state:#x}, IFSC {ifsc:#x}, reason {reason:?}, FNV {far_not_valid}" + )] + InstructionAbort { + esr_el2: u64, + elr_el2: u64, + far_el2: u64, + fipa: u64, + fipa_state: u8, + ifsc: u8, + reason: InstructionAbortReason, + far_not_valid: bool, + }, } const AARCH64_ZERO_REGISTER_INDEX: u8 = 31; +#[derive(Debug, Clone, Copy)] +enum InstructionAbortReason { + AddressSizeFaultLevel0, + AddressSizeFaultLevel1, + AddressSizeFaultLevel2, + AddressSizeFaultLevel3, + TranslationFaultLevel0, + TranslationFaultLevel1, + TranslationFaultLevel2, + TranslationFaultLevel3, + AccessFlagFaultLevel0, + AccessFlagFaultLevel1, + AccessFlagFaultLevel2, + AccessFlagFaultLevel3, + PermissionFaultLevel0, + PermissionFaultLevel1, + PermissionFaultLevel2, + PermissionFaultLevel3, + SynchronousExternalAbort, + SyncTagCheckFault, + SynchronousExternalAbortOnTableWalkLevelNeg1, + SynchronousExternalAbortOnTableWalkLevel0, + SynchronousExternalAbortOnTableWalkLevel1, + SynchronousExternalAbortOnTableWalkLevel2, + SynchronousExternalAbortOnTableWalkLevel3, + EccParity, + EccParityOnTableWalkLevelNeg1, + EccParityOnTableWalkLevel0, + EccParityOnTableWalkLevel1, + EccParityOnTableWalkLevel2, + EccParityOnTableWalkLevel3, + GranuleProtectionFaultLevelNeg1, + GranuleProtectionFaultLevel0, + GranuleProtectionFaultLevel1, + GranuleProtectionFaultLevel2, + GranuleProtectionFaultLevel3, + AddressSizeFaultLevelNeg1, + TranslationFaultLevelNeg1, + TlbConflictAbort, + UnsupportedHardwareUpdateFault, + Unknown, +} + +impl From for InstructionAbortReason { + fn from(value: FaultStatusCode) -> Self { + match value { + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL0 => Self::AddressSizeFaultLevel0, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL1 => Self::AddressSizeFaultLevel1, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL2 => Self::AddressSizeFaultLevel2, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL3 => Self::AddressSizeFaultLevel3, + FaultStatusCode::TRANSLATION_FAULT_LEVEL0 => Self::TranslationFaultLevel0, + FaultStatusCode::TRANSLATION_FAULT_LEVEL1 => Self::TranslationFaultLevel1, + FaultStatusCode::TRANSLATION_FAULT_LEVEL2 => Self::TranslationFaultLevel2, + FaultStatusCode::TRANSLATION_FAULT_LEVEL3 => Self::TranslationFaultLevel3, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL0 => Self::AccessFlagFaultLevel0, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL1 => Self::AccessFlagFaultLevel1, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL2 => Self::AccessFlagFaultLevel2, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL3 => Self::AccessFlagFaultLevel3, + FaultStatusCode::PERMISSION_FAULT_LEVEL0 => Self::PermissionFaultLevel0, + FaultStatusCode::PERMISSION_FAULT_LEVEL1 => Self::PermissionFaultLevel1, + FaultStatusCode::PERMISSION_FAULT_LEVEL2 => Self::PermissionFaultLevel2, + FaultStatusCode::PERMISSION_FAULT_LEVEL3 => Self::PermissionFaultLevel3, + FaultStatusCode::SYNCHRONOUS_EXTERNAL_ABORT => Self::SynchronousExternalAbort, + FaultStatusCode::SYNC_TAG_CHECK_FAULT => Self::SyncTagCheckFault, + FaultStatusCode::SEA_TTW_LEVEL_NEG1 => { + Self::SynchronousExternalAbortOnTableWalkLevelNeg1 + } + FaultStatusCode::SEA_TTW_LEVEL0 => Self::SynchronousExternalAbortOnTableWalkLevel0, + FaultStatusCode::SEA_TTW_LEVEL1 => Self::SynchronousExternalAbortOnTableWalkLevel1, + FaultStatusCode::SEA_TTW_LEVEL2 => Self::SynchronousExternalAbortOnTableWalkLevel2, + FaultStatusCode::SEA_TTW_LEVEL3 => Self::SynchronousExternalAbortOnTableWalkLevel3, + FaultStatusCode::ECC_PARITY => Self::EccParity, + FaultStatusCode::ECC_PARITY_TTW_LEVEL_NEG1 => Self::EccParityOnTableWalkLevelNeg1, + FaultStatusCode::ECC_PARITY_TTW_LEVEL0 => Self::EccParityOnTableWalkLevel0, + FaultStatusCode::ECC_PARITY_TTW_LEVEL1 => Self::EccParityOnTableWalkLevel1, + FaultStatusCode::ECC_PARITY_TTW_LEVEL2 => Self::EccParityOnTableWalkLevel2, + FaultStatusCode::ECC_PARITY_TTW_LEVEL3 => Self::EccParityOnTableWalkLevel3, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL_NEG => { + Self::GranuleProtectionFaultLevelNeg1 + } + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL0 => Self::GranuleProtectionFaultLevel0, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL1 => Self::GranuleProtectionFaultLevel1, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL2 => Self::GranuleProtectionFaultLevel2, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL3 => Self::GranuleProtectionFaultLevel3, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL_NEG1 => Self::AddressSizeFaultLevelNeg1, + FaultStatusCode::TRANSLATION_FAULT_LEVEL_NEG1 => Self::TranslationFaultLevelNeg1, + FaultStatusCode::TLB_CONFLICT_ABORT => Self::TlbConflictAbort, + FaultStatusCode::UNSUPPORTED_HW_UPDATE_FAULT => Self::UnsupportedHardwareUpdateFault, + _ => Self::Unknown, + } + } +} + // For use with Hyper-V synthetic interrupt controller allocated by paravisor. enum UhDirectOverlay { #[expect(unused)] @@ -169,6 +280,14 @@ impl<'a> CcaExit<'a> { self.0.far_el2 } + fn hpfar_el2(&self) -> HpfarEl2 { + self.0.hpfar_el2.into() + } + + fn elr_el2(&self) -> u64 { + self.0.elr_el2 + } + fn gpr_or_zero_register(&self, index: u8) -> Option { match index { AARCH64_ZERO_REGISTER_INDEX => Some(0), @@ -345,7 +464,45 @@ impl BackingPrivate for CcaBacked { } ExceptionClass::InstructionAbort => { // Handle instruction abort - todo!(); + let iss = IssInstructionAbort::from_bits(esr_el2.iss()); + + if iss.fnv() { + tracing::warn!("CCA InstructionAbort: FAR_EL2 is not valid"); + return Err(dev.fatal_error(CcaUnsupportedExit::ExitReason(0).into())); + } + + let far = cca_exit.far_el2(); + let hpfar = cca_exit.hpfar_el2(); + let fipa = (hpfar.fipa() << 12) | (far & 0xfff); + let mut plane_state = mshv_rsi_get_ipa_state{ fipa, state: u64::MAX}; + let _ = this.ipa_state_read(GuestVtl::Vtl0, &mut plane_state).map_err(|_| Error::Hcl); + + let reason = InstructionAbortReason::from(iss.ifsc()); + tracing::warn!( + esr_el2 = cca_exit.0.esr_el2, + elr_el2 = cca_exit.elr_el2(), + far_el2 = cca_exit.far_el2(), + fipa = fipa, + fipa_state = plane_state.state as u8, + ifsc = iss.ifsc().0, + ?reason, + far_not_valid = iss.fnv(), + "CCA instruction abort" + ); + return Err(dev.fatal_error( + CcaUnsupportedExit::InstructionAbort { + esr_el2: cca_exit.0.esr_el2, + elr_el2: cca_exit.elr_el2(), + far_el2: cca_exit.far_el2(), + fipa: fipa, + fipa_state: plane_state.state as u8, + ifsc: iss.ifsc().0, + reason, + far_not_valid: iss.fnv(), + } + .into(), + )); + } ExceptionClass::SimdAccess => { this.runner.cca_plane_no_trap_simd(); @@ -448,6 +605,14 @@ impl UhProcessor<'_, CcaBacked> { self.runner.cca_sysreg_read(vtl, reg, val) } + fn ipa_state_read( + &self, + vtl: GuestVtl, + state: &mut mshv_rsi_get_ipa_state, + ) -> Result<(), Error> { + self.runner.cca_ipa_state_read(vtl, state).map_err(Error::Hcl) + } + fn set_plane_enter(&mut self) { self.runner.cca_set_plane_enter(); } diff --git a/tmk/simple_tmk/src/aarch64/mod.rs b/tmk/simple_tmk/src/aarch64/mod.rs new file mode 100755 index 0000000000..1cef66b082 --- /dev/null +++ b/tmk/simple_tmk/src/aarch64/mod.rs @@ -0,0 +1,70 @@ + +#![cfg(target_arch = "aarch64")] + +#![allow(unsafe_code)] + +use crate::prelude::*; + +core::arch::global_asm! { + ".global instruction_abort_outside_par_entry", + "instruction_abort_outside_par_entry:", + "movz x16, #0x0000", + "movk x16, #0x0000, lsl #16", + "movk x16, #0xffff, lsl #32", + "movk x16, #0x0000, lsl #48", + "br x16", +} + +unsafe extern "C" { + fn instruction_abort_outside_par_entry() -> !; +} + +#[tmk_test(expected_failure)] +fn instruction_abort_outside_par(_: TestContext<'_>) { + log!("instruction_abort_outside_par"); + + unsafe { + instruction_abort_outside_par_entry(); + } +} + +core::arch::global_asm! { + ".global instruction_abort_ripas_empty_entry", + "instruction_abort_ripas_empty_entry:", + "movz x16, #0x0000", + "br x16", +} + +unsafe extern "C" { + fn instruction_abort_ripas_empty_entry() -> !; +} + +#[tmk_test(expected_failure)] +fn instruction_abort_ripas_empty(_: TestContext<'_>) { + log!("instruction_abort_ripas_empty"); + + unsafe { + instruction_abort_ripas_empty_entry(); + } +} + +core::arch::global_asm! { + ".global instruction_abort_permissions_enabled_entry", + "instruction_abort_permissions_enabled_entry:", + "movz x16, #0xf000", + "movk x16, #0x847f, lsl #16", + "br x16", +} + +unsafe extern "C" { + fn instruction_abort_permissions_enabled_entry() -> !; +} + +#[tmk_test(expected_failure)] +fn instruction_abort_permissions_enabled(_: TestContext<'_>) { + log!("instruction_abort_permissions_enabled"); + + unsafe { + instruction_abort_permissions_enabled_entry(); + } +} \ No newline at end of file diff --git a/tmk/simple_tmk/src/main.rs b/tmk/simple_tmk/src/main.rs index bd84cb0b72..b77c902b54 100644 --- a/tmk/simple_tmk/src/main.rs +++ b/tmk/simple_tmk/src/main.rs @@ -11,6 +11,7 @@ mod prelude; mod common; mod x86_64; +mod aarch64; #[cfg(not(minimal_rt))] fn main() { diff --git a/tmk/tmk_core/src/lib.rs b/tmk/tmk_core/src/lib.rs index 048a68fbbc..06825b5f40 100644 --- a/tmk/tmk_core/src/lib.rs +++ b/tmk/tmk_core/src/lib.rs @@ -21,6 +21,9 @@ use core::ptr::null_mut; use core::sync::atomic::AtomicPtr; use core::sync::atomic::Ordering::Relaxed; +/// The test is expected to fail. +pub const TEST_FLAG_EXPECTED_FAILURE: u64 = tmk_protocol::TEST_FLAG_EXPECTED_FAILURE; + /// A TMK test context, passed to each test function. pub struct TestContext<'scope> { /// The BSP VP's scope. @@ -144,6 +147,9 @@ unsafe extern "C" { #[macro_export] macro_rules! define_tmk_test { ($name:expr, $func:ident) => { + $crate::define_tmk_test!($name, $func, 0); + }; + ($name:expr, $func:ident, $flags:expr) => { const _: () = { // Strip the crate name from the module path. const NAME: &[u8] = const { @@ -162,6 +168,7 @@ macro_rules! define_tmk_test { static TEST: $crate::TestDescriptor = $crate::TestDescriptor { name: NAME, entrypoint: $func, + flags: $flags, }; }; }; @@ -176,6 +183,8 @@ pub struct TestDescriptor { pub name: &'static [u8], /// The test entry point. pub entrypoint: for<'scope> fn(TestContext<'scope>), + /// Test flag - expected failure or not + pub flags: u64, } #[cfg_attr(minimal_rt, panic_handler)] diff --git a/tmk/tmk_macros/src/lib.rs b/tmk/tmk_macros/src/lib.rs index 370bf7b3f3..eb318dc00b 100644 --- a/tmk/tmk_macros/src/lib.rs +++ b/tmk/tmk_macros/src/lib.rs @@ -13,12 +13,27 @@ use quote::quote; /// /// This macro is used to define a test in the TMK. #[proc_macro_attribute] -pub fn tmk_test(_attr: TokenStream, item: TokenStream) -> TokenStream { +pub fn tmk_test(attr: TokenStream, item: TokenStream) -> TokenStream { let item = syn::parse_macro_input!(item as syn::ItemFn); let name = item.sig.ident.to_string(); let func = &item.sig.ident; + + let flags = match attr.to_string().as_str() { + "" => quote! { 0 }, + "expected_failure" => quote! { ::tmk_core::TEST_FLAG_EXPECTED_FAILURE }, + attr => { + let msg = format!("unsupported tmk_test option: {attr}"); + return quote! { + compile_error!(#msg); + #item + } + .into_token_stream() + .into(); + } + }; + quote! { - ::tmk_core::define_tmk_test!(#name, #func); + ::tmk_core::define_tmk_test!(#name, #func, #flags); #item } .into_token_stream() diff --git a/tmk/tmk_protocol/src/lib.rs b/tmk/tmk_protocol/src/lib.rs index bd9f98457d..366fef9e4a 100644 --- a/tmk/tmk_protocol/src/lib.rs +++ b/tmk/tmk_protocol/src/lib.rs @@ -11,6 +11,9 @@ use zerocopy::Immutable; use zerocopy::IntoBytes; use zerocopy::TryFromBytes; +/// The test is expected to fail. +pub const TEST_FLAG_EXPECTED_FAILURE: u64 = 1 << 0; + /// Start input from the VMM to the TMK. #[repr(C)] #[derive(Debug, IntoBytes, Immutable)] @@ -31,6 +34,8 @@ pub struct TestDescriptor64 { pub name_len: u64, /// The test entry point. pub entrypoint: u64, + /// Test metadata flags. + pub flags: u64, } /// TMK command. diff --git a/tmk/tmk_vmm/src/load.rs b/tmk/tmk_vmm/src/load.rs index 3a4440b8d3..61bc857800 100644 --- a/tmk/tmk_vmm/src/load.rs +++ b/tmk/tmk_vmm/src/load.rs @@ -176,6 +176,7 @@ struct LoadInfo { pub struct TestInfo { pub name: String, pub index: u64, + pub expected_failure: bool, } /// Enumerate the tests from a TMK binary. @@ -228,6 +229,7 @@ pub fn enumerate_tests(tmk: &File) -> anyhow::Result> { tests.push(TestInfo { name: name.to_string(), index: i as u64, + expected_failure: t.flags & tmk_protocol::TEST_FLAG_EXPECTED_FAILURE != 0, }); } diff --git a/tmk/tmk_vmm/src/run.rs b/tmk/tmk_vmm/src/run.rs index 477547d601..aa3eb2da53 100644 --- a/tmk/tmk_vmm/src/run.rs +++ b/tmk/tmk_vmm/src/run.rs @@ -291,19 +291,40 @@ impl CommonState { vmtime_keeper.stop().await; - match r { - TestResult::Passed => { + match (r, test.expected_failure) { + (TestResult::Passed, false) => { tracing::info!(target: "test", name = test.name, "test passed"); } - TestResult::Failed => { + (TestResult::Passed, true) => { + tracing::error!( + target: "test", + name = test.name, + expected_failure = true, + "test unexpectedly passed" + ); + success = false; + } + (TestResult::Failed, false) => { tracing::error!(target: "test", name = test.name, reason = "explicit failure", "test failed"); success = false; } - TestResult::Faulted { - vp_index, - reason, - regs, - } => { + (TestResult::Failed, true) => { + tracing::info!( + target: "test", + name = test.name, + expected_failure = true, + reason = "explicit failure", + "test passed" + ); + } + ( + TestResult::Faulted { + vp_index, + reason, + regs, + }, + false, + ) => { tracing::error!( target: "test", name = test.name, @@ -314,6 +335,24 @@ impl CommonState { ); success = false; } + ( + TestResult::Faulted { + vp_index, + reason, + regs, + }, + true, + ) => { + tracing::info!( + target: "test", + name = test.name, + expected_failure = true, + vp_index = vp_index.index(), + reason, + regs = format_args!("{:#x?}", regs), + "test passed" + ); + } } } if !success { diff --git a/vm/aarch64/aarch64defs/src/lib.rs b/vm/aarch64/aarch64defs/src/lib.rs index c77fc957da..8c5c64c209 100644 --- a/vm/aarch64/aarch64defs/src/lib.rs +++ b/vm/aarch64/aarch64defs/src/lib.rs @@ -181,6 +181,19 @@ pub struct SctlrEl1 { pub tidcp: bool, } +/// aarch64 HPFAR_EL2 +#[bitfield(u64)] +#[derive(PartialEq, Eq)] +pub struct HpfarEl2 { + #[bits(4)] + pub res0: u8, + #[bits(44)] + pub fipa: u64, + #[bits(15)] + pub res1: u32, + pub ns: bool, +} + open_enum! { pub enum ExceptionClass: u8 { UNKNOWN = 0b000000, @@ -322,7 +335,7 @@ open_enum! { /// Valid only for instruction fault. GRANULE_PROTECTION_FAULT_LEVEL2 = 0b100110, /// Valid only for instruction fault. - GRANULE_PROTECTION_FAULT_LEVE3 = 0b100111, + GRANULE_PROTECTION_FAULT_LEVEL3 = 0b100111, ADDRESS_SIZE_FAULT_LEVEL_NEG1 = 0b101001, TRANSLATION_FAULT_LEVEL_NEG1 = 0b101011, TLB_CONFLICT_ABORT = 0b110000, @@ -339,6 +352,7 @@ impl FaultStatusCode { const fn into_bits(self) -> u32 { self.0 as u32 } + } #[bitfield(u32)] From 4452ec2549f38f1d156e3f4fb3987556e38bf07d Mon Sep 17 00:00:00 2001 From: Flgodd67 Date: Fri, 12 Jun 2026 15:11:44 +0100 Subject: [PATCH 2/5] cca: fixing linting --- openhcl/hcl/src/ioctl/cca.rs | 19 ++++++++++++++---- .../virt_mshv_vtl/src/processor/cca/mod.rs | 20 +++++++++++++------ tmk/simple_tmk/src/aarch64/mod.rs | 19 +++++++++++++++--- tmk/simple_tmk/src/main.rs | 2 +- vm/aarch64/aarch64defs/src/lib.rs | 1 - 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/openhcl/hcl/src/ioctl/cca.rs b/openhcl/hcl/src/ioctl/cca.rs index 07e51f6606..d68c46ed23 100755 --- a/openhcl/hcl/src/ioctl/cca.rs +++ b/openhcl/hcl/src/ioctl/cca.rs @@ -220,7 +220,9 @@ impl ProcessorRunner<'_, Cca> { vtl: GuestVtl, state: &mut mshv_rsi_get_ipa_state, ) -> Result<(), Error> { - self.hcl.rsi_get_ipa_state(vtl, state).map_err(|_| Error::InvalidRegisterValue) + self.hcl + .rsi_get_ipa_state(vtl, state) + .map_err(|_| Error::InvalidRegisterValue) } /// Update the address of the `plane_run` structure in `mshv_vtl_run.context`. @@ -490,12 +492,17 @@ impl MshvVtl { } /// Get the ipa RIPAS state - pub fn rsi_get_ipa_state(&self, vtl: GuestVtl, plane_state: &mut mshv_rsi_get_ipa_state) -> Result<(), HvError> { + pub fn rsi_get_ipa_state( + &self, + vtl: GuestVtl, + plane_state: &mut mshv_rsi_get_ipa_state, + ) -> Result<(), HvError> { let _plane = match vtl { GuestVtl::Vtl0 => 1, - _ => return Err(HvError::InvalidVtlState) + _ => return Err(HvError::InvalidVtlState), }; + // SAFETY: Calling hcl_rsi_ipa_state_read ioctl with the correct arguments. unsafe { hcl_rsi_ipa_state_read(self.file.as_raw_fd(), plane_state) .map_err(|_| HvError::InvalidVpState)?; @@ -537,7 +544,11 @@ impl Hcl { } /// getting ipa RIPAS state - pub fn rsi_get_ipa_state(&self, vtl: GuestVtl, plane_state: &mut mshv_rsi_get_ipa_state) -> Result<(), HvError> { + pub fn rsi_get_ipa_state( + &self, + vtl: GuestVtl, + plane_state: &mut mshv_rsi_get_ipa_state, + ) -> Result<(), HvError> { self.mshv_vtl.rsi_get_ipa_state(vtl, plane_state) } } diff --git a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs index 8e1be5f861..ca9b4b84d0 100644 --- a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs @@ -468,14 +468,21 @@ impl BackingPrivate for CcaBacked { if iss.fnv() { tracing::warn!("CCA InstructionAbort: FAR_EL2 is not valid"); - return Err(dev.fatal_error(CcaUnsupportedExit::ExitReason(0).into())); + return Err( + dev.fatal_error(CcaUnsupportedExit::ExitReason(0).into()) + ); } let far = cca_exit.far_el2(); let hpfar = cca_exit.hpfar_el2(); let fipa = (hpfar.fipa() << 12) | (far & 0xfff); - let mut plane_state = mshv_rsi_get_ipa_state{ fipa, state: u64::MAX}; - let _ = this.ipa_state_read(GuestVtl::Vtl0, &mut plane_state).map_err(|_| Error::Hcl); + let mut plane_state = mshv_rsi_get_ipa_state { + fipa, + state: u64::MAX, + }; + let _ = this + .ipa_state_read(GuestVtl::Vtl0, &mut plane_state) + .map_err(|_| Error::Hcl); let reason = InstructionAbortReason::from(iss.ifsc()); tracing::warn!( @@ -494,7 +501,7 @@ impl BackingPrivate for CcaBacked { esr_el2: cca_exit.0.esr_el2, elr_el2: cca_exit.elr_el2(), far_el2: cca_exit.far_el2(), - fipa: fipa, + fipa, fipa_state: plane_state.state as u8, ifsc: iss.ifsc().0, reason, @@ -502,7 +509,6 @@ impl BackingPrivate for CcaBacked { } .into(), )); - } ExceptionClass::SimdAccess => { this.runner.cca_plane_no_trap_simd(); @@ -610,7 +616,9 @@ impl UhProcessor<'_, CcaBacked> { vtl: GuestVtl, state: &mut mshv_rsi_get_ipa_state, ) -> Result<(), Error> { - self.runner.cca_ipa_state_read(vtl, state).map_err(Error::Hcl) + self.runner + .cca_ipa_state_read(vtl, state) + .map_err(Error::Hcl) } fn set_plane_enter(&mut self) { diff --git a/tmk/simple_tmk/src/aarch64/mod.rs b/tmk/simple_tmk/src/aarch64/mod.rs index 1cef66b082..60ac648a48 100755 --- a/tmk/simple_tmk/src/aarch64/mod.rs +++ b/tmk/simple_tmk/src/aarch64/mod.rs @@ -1,7 +1,11 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. #![cfg(target_arch = "aarch64")] - -#![allow(unsafe_code)] +#![allow( + unsafe_code, + reason = "global_asm! required for AArch64 trampoline code" +)] use crate::prelude::*; @@ -23,6 +27,9 @@ unsafe extern "C" { fn instruction_abort_outside_par(_: TestContext<'_>) { log!("instruction_abort_outside_par"); + // SAFETY: This test intentionally jumps to an assembly entry point that + // triggers an instruction abort. The symbol is defined in this module via + // `global_asm!` and is declared `-> !`, so it is not expected to return. unsafe { instruction_abort_outside_par_entry(); } @@ -43,6 +50,9 @@ unsafe extern "C" { fn instruction_abort_ripas_empty(_: TestContext<'_>) { log!("instruction_abort_ripas_empty"); + // SAFETY: This test intentionally transfers control to an assembly entry + // point that executes from an address chosen to provoke the expected + // instruction abort. The entry point is defined above and never returns. unsafe { instruction_abort_ripas_empty_entry(); } @@ -64,7 +74,10 @@ unsafe extern "C" { fn instruction_abort_permissions_enabled(_: TestContext<'_>) { log!("instruction_abort_permissions_enabled"); + // SAFETY: This test intentionally calls an assembly entry point that jumps + // to an address expected to fault under the configured permissions. The + // entry point is defined in this module and is declared `-> !`. unsafe { instruction_abort_permissions_enabled_entry(); } -} \ No newline at end of file +} diff --git a/tmk/simple_tmk/src/main.rs b/tmk/simple_tmk/src/main.rs index b77c902b54..c8f9f5d7d4 100644 --- a/tmk/simple_tmk/src/main.rs +++ b/tmk/simple_tmk/src/main.rs @@ -9,9 +9,9 @@ mod prelude; +mod aarch64; mod common; mod x86_64; -mod aarch64; #[cfg(not(minimal_rt))] fn main() { diff --git a/vm/aarch64/aarch64defs/src/lib.rs b/vm/aarch64/aarch64defs/src/lib.rs index 8c5c64c209..62827ef2c5 100644 --- a/vm/aarch64/aarch64defs/src/lib.rs +++ b/vm/aarch64/aarch64defs/src/lib.rs @@ -352,7 +352,6 @@ impl FaultStatusCode { const fn into_bits(self) -> u32 { self.0 as u32 } - } #[bitfield(u32)] From ae330b38f3f99512927a1eeb056e3fadac847153 Mon Sep 17 00:00:00 2001 From: Flgodd67 Date: Mon, 15 Jun 2026 14:30:59 +0100 Subject: [PATCH 3/5] cca: changing TestDescriptor flags variable to be a bitfield struct TestFlags64 --- Cargo.lock | 2 ++ tmk/simple_tmk/Cargo.toml | 1 + tmk/simple_tmk/src/aarch64/mod.rs | 1 + tmk/tmk_core/src/lib.rs | 6 ++---- tmk/tmk_macros/src/lib.rs | 6 ++++-- tmk/tmk_protocol/Cargo.toml | 1 + tmk/tmk_protocol/src/lib.rs | 18 ++++++++++++++---- tmk/tmk_vmm/src/load.rs | 2 +- 8 files changed, 26 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7df3e07147..90911576a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7431,6 +7431,7 @@ dependencies = [ "minimal_rt_build", "tmk_core", "tmk_macros", + "tmk_protocol", "x86defs", ] @@ -8017,6 +8018,7 @@ dependencies = [ name = "tmk_protocol" version = "0.0.0" dependencies = [ + "bitfield-struct 0.11.0", "zerocopy", ] diff --git a/tmk/simple_tmk/Cargo.toml b/tmk/simple_tmk/Cargo.toml index bf1d3e4b38..82666302ec 100644 --- a/tmk/simple_tmk/Cargo.toml +++ b/tmk/simple_tmk/Cargo.toml @@ -8,6 +8,7 @@ edition.workspace = true [dependencies] tmk_core.workspace = true +tmk_protocol.workspace = true tmk_macros.workspace = true [target.'cfg(target_arch = "x86_64")'.dependencies] diff --git a/tmk/simple_tmk/src/aarch64/mod.rs b/tmk/simple_tmk/src/aarch64/mod.rs index 60ac648a48..cf10950d6b 100755 --- a/tmk/simple_tmk/src/aarch64/mod.rs +++ b/tmk/simple_tmk/src/aarch64/mod.rs @@ -8,6 +8,7 @@ )] use crate::prelude::*; +use tmk_protocol as _; core::arch::global_asm! { ".global instruction_abort_outside_par_entry", diff --git a/tmk/tmk_core/src/lib.rs b/tmk/tmk_core/src/lib.rs index 06825b5f40..278cd78c50 100644 --- a/tmk/tmk_core/src/lib.rs +++ b/tmk/tmk_core/src/lib.rs @@ -12,6 +12,7 @@ pub mod x86_64; #[cfg(target_arch = "aarch64")] use aarch64 as arch; +use tmk_protocol::TestFlags64; #[cfg(target_arch = "x86_64")] use x86_64 as arch; @@ -21,9 +22,6 @@ use core::ptr::null_mut; use core::sync::atomic::AtomicPtr; use core::sync::atomic::Ordering::Relaxed; -/// The test is expected to fail. -pub const TEST_FLAG_EXPECTED_FAILURE: u64 = tmk_protocol::TEST_FLAG_EXPECTED_FAILURE; - /// A TMK test context, passed to each test function. pub struct TestContext<'scope> { /// The BSP VP's scope. @@ -184,7 +182,7 @@ pub struct TestDescriptor { /// The test entry point. pub entrypoint: for<'scope> fn(TestContext<'scope>), /// Test flag - expected failure or not - pub flags: u64, + pub flags: TestFlags64, } #[cfg_attr(minimal_rt, panic_handler)] diff --git a/tmk/tmk_macros/src/lib.rs b/tmk/tmk_macros/src/lib.rs index eb318dc00b..396f74280c 100644 --- a/tmk/tmk_macros/src/lib.rs +++ b/tmk/tmk_macros/src/lib.rs @@ -19,8 +19,10 @@ pub fn tmk_test(attr: TokenStream, item: TokenStream) -> TokenStream { let func = &item.sig.ident; let flags = match attr.to_string().as_str() { - "" => quote! { 0 }, - "expected_failure" => quote! { ::tmk_core::TEST_FLAG_EXPECTED_FAILURE }, + "" => quote! { ::tmk_protocol::TestFlags64::new() }, + "expected_failure" => { + quote! { ::tmk_protocol::TestFlags64::new().with_expected_failure(true) } + } attr => { let msg = format!("unsupported tmk_test option: {attr}"); return quote! { diff --git a/tmk/tmk_protocol/Cargo.toml b/tmk/tmk_protocol/Cargo.toml index afabe02f02..fa7faefc32 100644 --- a/tmk/tmk_protocol/Cargo.toml +++ b/tmk/tmk_protocol/Cargo.toml @@ -8,6 +8,7 @@ edition.workspace = true [dependencies] zerocopy.workspace = true +bitfield-struct.workspace = true [lints] workspace = true diff --git a/tmk/tmk_protocol/src/lib.rs b/tmk/tmk_protocol/src/lib.rs index 366fef9e4a..eab3d6592a 100644 --- a/tmk/tmk_protocol/src/lib.rs +++ b/tmk/tmk_protocol/src/lib.rs @@ -6,14 +6,13 @@ #![no_std] #![forbid(unsafe_code)] +use bitfield_struct::bitfield; use zerocopy::FromBytes; use zerocopy::Immutable; use zerocopy::IntoBytes; +use zerocopy::KnownLayout; use zerocopy::TryFromBytes; -/// The test is expected to fail. -pub const TEST_FLAG_EXPECTED_FAILURE: u64 = 1 << 0; - /// Start input from the VMM to the TMK. #[repr(C)] #[derive(Debug, IntoBytes, Immutable)] @@ -24,6 +23,17 @@ pub struct StartInput { pub test_index: u64, } +/// Test metadata flags. +#[bitfield(u64)] +#[derive(IntoBytes, Immutable, KnownLayout, FromBytes)] +pub struct TestFlags64 { + #[bits(1)] + pub expected_failure: bool, + + #[bits(63)] + reserved: u64, +} + /// A 64-bit TMK test descriptor. #[repr(C)] #[derive(IntoBytes, FromBytes, Immutable)] @@ -35,7 +45,7 @@ pub struct TestDescriptor64 { /// The test entry point. pub entrypoint: u64, /// Test metadata flags. - pub flags: u64, + pub flags: TestFlags64, } /// TMK command. diff --git a/tmk/tmk_vmm/src/load.rs b/tmk/tmk_vmm/src/load.rs index 61bc857800..23e91ac86f 100644 --- a/tmk/tmk_vmm/src/load.rs +++ b/tmk/tmk_vmm/src/load.rs @@ -229,7 +229,7 @@ pub fn enumerate_tests(tmk: &File) -> anyhow::Result> { tests.push(TestInfo { name: name.to_string(), index: i as u64, - expected_failure: t.flags & tmk_protocol::TEST_FLAG_EXPECTED_FAILURE != 0, + expected_failure: t.flags.expected_failure(), }); } From ce52e9a41180ae3175999307ae1a020508d2b373 Mon Sep 17 00:00:00 2001 From: Flgodd67 Date: Mon, 15 Jun 2026 15:32:52 +0100 Subject: [PATCH 4/5] cca: moving InstructionAbortReason enum to aarch64defs. removing warning and going straight to err in InstructionAbort match in run_vp --- .../virt_mshv_vtl/src/processor/cca/mod.rs | 107 +----------------- vm/aarch64/aarch64defs/src/lib.rs | 93 +++++++++++++++ 2 files changed, 95 insertions(+), 105 deletions(-) diff --git a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs index ca9b4b84d0..a18cd676ae 100644 --- a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs @@ -17,8 +17,8 @@ use crate::UhCvmVpState; use crate::UhPartitionInner; use crate::processor::InterceptMessageState; use aarch64defs::EsrEl2; -use aarch64defs::FaultStatusCode; use aarch64defs::HpfarEl2; +use aarch64defs::InstructionAbortReason; use aarch64defs::IssDataAbort; use aarch64defs::IssInstructionAbort; use aarch64defs::SystemReg; @@ -71,99 +71,6 @@ enum CcaUnsupportedExit { const AARCH64_ZERO_REGISTER_INDEX: u8 = 31; -#[derive(Debug, Clone, Copy)] -enum InstructionAbortReason { - AddressSizeFaultLevel0, - AddressSizeFaultLevel1, - AddressSizeFaultLevel2, - AddressSizeFaultLevel3, - TranslationFaultLevel0, - TranslationFaultLevel1, - TranslationFaultLevel2, - TranslationFaultLevel3, - AccessFlagFaultLevel0, - AccessFlagFaultLevel1, - AccessFlagFaultLevel2, - AccessFlagFaultLevel3, - PermissionFaultLevel0, - PermissionFaultLevel1, - PermissionFaultLevel2, - PermissionFaultLevel3, - SynchronousExternalAbort, - SyncTagCheckFault, - SynchronousExternalAbortOnTableWalkLevelNeg1, - SynchronousExternalAbortOnTableWalkLevel0, - SynchronousExternalAbortOnTableWalkLevel1, - SynchronousExternalAbortOnTableWalkLevel2, - SynchronousExternalAbortOnTableWalkLevel3, - EccParity, - EccParityOnTableWalkLevelNeg1, - EccParityOnTableWalkLevel0, - EccParityOnTableWalkLevel1, - EccParityOnTableWalkLevel2, - EccParityOnTableWalkLevel3, - GranuleProtectionFaultLevelNeg1, - GranuleProtectionFaultLevel0, - GranuleProtectionFaultLevel1, - GranuleProtectionFaultLevel2, - GranuleProtectionFaultLevel3, - AddressSizeFaultLevelNeg1, - TranslationFaultLevelNeg1, - TlbConflictAbort, - UnsupportedHardwareUpdateFault, - Unknown, -} - -impl From for InstructionAbortReason { - fn from(value: FaultStatusCode) -> Self { - match value { - FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL0 => Self::AddressSizeFaultLevel0, - FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL1 => Self::AddressSizeFaultLevel1, - FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL2 => Self::AddressSizeFaultLevel2, - FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL3 => Self::AddressSizeFaultLevel3, - FaultStatusCode::TRANSLATION_FAULT_LEVEL0 => Self::TranslationFaultLevel0, - FaultStatusCode::TRANSLATION_FAULT_LEVEL1 => Self::TranslationFaultLevel1, - FaultStatusCode::TRANSLATION_FAULT_LEVEL2 => Self::TranslationFaultLevel2, - FaultStatusCode::TRANSLATION_FAULT_LEVEL3 => Self::TranslationFaultLevel3, - FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL0 => Self::AccessFlagFaultLevel0, - FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL1 => Self::AccessFlagFaultLevel1, - FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL2 => Self::AccessFlagFaultLevel2, - FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL3 => Self::AccessFlagFaultLevel3, - FaultStatusCode::PERMISSION_FAULT_LEVEL0 => Self::PermissionFaultLevel0, - FaultStatusCode::PERMISSION_FAULT_LEVEL1 => Self::PermissionFaultLevel1, - FaultStatusCode::PERMISSION_FAULT_LEVEL2 => Self::PermissionFaultLevel2, - FaultStatusCode::PERMISSION_FAULT_LEVEL3 => Self::PermissionFaultLevel3, - FaultStatusCode::SYNCHRONOUS_EXTERNAL_ABORT => Self::SynchronousExternalAbort, - FaultStatusCode::SYNC_TAG_CHECK_FAULT => Self::SyncTagCheckFault, - FaultStatusCode::SEA_TTW_LEVEL_NEG1 => { - Self::SynchronousExternalAbortOnTableWalkLevelNeg1 - } - FaultStatusCode::SEA_TTW_LEVEL0 => Self::SynchronousExternalAbortOnTableWalkLevel0, - FaultStatusCode::SEA_TTW_LEVEL1 => Self::SynchronousExternalAbortOnTableWalkLevel1, - FaultStatusCode::SEA_TTW_LEVEL2 => Self::SynchronousExternalAbortOnTableWalkLevel2, - FaultStatusCode::SEA_TTW_LEVEL3 => Self::SynchronousExternalAbortOnTableWalkLevel3, - FaultStatusCode::ECC_PARITY => Self::EccParity, - FaultStatusCode::ECC_PARITY_TTW_LEVEL_NEG1 => Self::EccParityOnTableWalkLevelNeg1, - FaultStatusCode::ECC_PARITY_TTW_LEVEL0 => Self::EccParityOnTableWalkLevel0, - FaultStatusCode::ECC_PARITY_TTW_LEVEL1 => Self::EccParityOnTableWalkLevel1, - FaultStatusCode::ECC_PARITY_TTW_LEVEL2 => Self::EccParityOnTableWalkLevel2, - FaultStatusCode::ECC_PARITY_TTW_LEVEL3 => Self::EccParityOnTableWalkLevel3, - FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL_NEG => { - Self::GranuleProtectionFaultLevelNeg1 - } - FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL0 => Self::GranuleProtectionFaultLevel0, - FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL1 => Self::GranuleProtectionFaultLevel1, - FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL2 => Self::GranuleProtectionFaultLevel2, - FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL3 => Self::GranuleProtectionFaultLevel3, - FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL_NEG1 => Self::AddressSizeFaultLevelNeg1, - FaultStatusCode::TRANSLATION_FAULT_LEVEL_NEG1 => Self::TranslationFaultLevelNeg1, - FaultStatusCode::TLB_CONFLICT_ABORT => Self::TlbConflictAbort, - FaultStatusCode::UNSUPPORTED_HW_UPDATE_FAULT => Self::UnsupportedHardwareUpdateFault, - _ => Self::Unknown, - } - } -} - // For use with Hyper-V synthetic interrupt controller allocated by paravisor. enum UhDirectOverlay { #[expect(unused)] @@ -485,17 +392,7 @@ impl BackingPrivate for CcaBacked { .map_err(|_| Error::Hcl); let reason = InstructionAbortReason::from(iss.ifsc()); - tracing::warn!( - esr_el2 = cca_exit.0.esr_el2, - elr_el2 = cca_exit.elr_el2(), - far_el2 = cca_exit.far_el2(), - fipa = fipa, - fipa_state = plane_state.state as u8, - ifsc = iss.ifsc().0, - ?reason, - far_not_valid = iss.fnv(), - "CCA instruction abort" - ); + return Err(dev.fatal_error( CcaUnsupportedExit::InstructionAbort { esr_el2: cca_exit.0.esr_el2, diff --git a/vm/aarch64/aarch64defs/src/lib.rs b/vm/aarch64/aarch64defs/src/lib.rs index 62827ef2c5..2e5066626f 100644 --- a/vm/aarch64/aarch64defs/src/lib.rs +++ b/vm/aarch64/aarch64defs/src/lib.rs @@ -961,3 +961,96 @@ impl Display for Vendor { } } } + +#[derive(Debug, Clone, Copy)] +pub enum InstructionAbortReason { + AddressSizeFaultLevel0, + AddressSizeFaultLevel1, + AddressSizeFaultLevel2, + AddressSizeFaultLevel3, + TranslationFaultLevel0, + TranslationFaultLevel1, + TranslationFaultLevel2, + TranslationFaultLevel3, + AccessFlagFaultLevel0, + AccessFlagFaultLevel1, + AccessFlagFaultLevel2, + AccessFlagFaultLevel3, + PermissionFaultLevel0, + PermissionFaultLevel1, + PermissionFaultLevel2, + PermissionFaultLevel3, + SynchronousExternalAbort, + SyncTagCheckFault, + SynchronousExternalAbortOnTableWalkLevelNeg1, + SynchronousExternalAbortOnTableWalkLevel0, + SynchronousExternalAbortOnTableWalkLevel1, + SynchronousExternalAbortOnTableWalkLevel2, + SynchronousExternalAbortOnTableWalkLevel3, + EccParity, + EccParityOnTableWalkLevelNeg1, + EccParityOnTableWalkLevel0, + EccParityOnTableWalkLevel1, + EccParityOnTableWalkLevel2, + EccParityOnTableWalkLevel3, + GranuleProtectionFaultLevelNeg1, + GranuleProtectionFaultLevel0, + GranuleProtectionFaultLevel1, + GranuleProtectionFaultLevel2, + GranuleProtectionFaultLevel3, + AddressSizeFaultLevelNeg1, + TranslationFaultLevelNeg1, + TlbConflictAbort, + UnsupportedHardwareUpdateFault, + Unknown, +} + +impl From for InstructionAbortReason { + fn from(value: FaultStatusCode) -> Self { + match value { + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL0 => Self::AddressSizeFaultLevel0, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL1 => Self::AddressSizeFaultLevel1, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL2 => Self::AddressSizeFaultLevel2, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL3 => Self::AddressSizeFaultLevel3, + FaultStatusCode::TRANSLATION_FAULT_LEVEL0 => Self::TranslationFaultLevel0, + FaultStatusCode::TRANSLATION_FAULT_LEVEL1 => Self::TranslationFaultLevel1, + FaultStatusCode::TRANSLATION_FAULT_LEVEL2 => Self::TranslationFaultLevel2, + FaultStatusCode::TRANSLATION_FAULT_LEVEL3 => Self::TranslationFaultLevel3, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL0 => Self::AccessFlagFaultLevel0, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL1 => Self::AccessFlagFaultLevel1, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL2 => Self::AccessFlagFaultLevel2, + FaultStatusCode::ACCESS_FLAG_FAULT_LEVEL3 => Self::AccessFlagFaultLevel3, + FaultStatusCode::PERMISSION_FAULT_LEVEL0 => Self::PermissionFaultLevel0, + FaultStatusCode::PERMISSION_FAULT_LEVEL1 => Self::PermissionFaultLevel1, + FaultStatusCode::PERMISSION_FAULT_LEVEL2 => Self::PermissionFaultLevel2, + FaultStatusCode::PERMISSION_FAULT_LEVEL3 => Self::PermissionFaultLevel3, + FaultStatusCode::SYNCHRONOUS_EXTERNAL_ABORT => Self::SynchronousExternalAbort, + FaultStatusCode::SYNC_TAG_CHECK_FAULT => Self::SyncTagCheckFault, + FaultStatusCode::SEA_TTW_LEVEL_NEG1 => { + Self::SynchronousExternalAbortOnTableWalkLevelNeg1 + } + FaultStatusCode::SEA_TTW_LEVEL0 => Self::SynchronousExternalAbortOnTableWalkLevel0, + FaultStatusCode::SEA_TTW_LEVEL1 => Self::SynchronousExternalAbortOnTableWalkLevel1, + FaultStatusCode::SEA_TTW_LEVEL2 => Self::SynchronousExternalAbortOnTableWalkLevel2, + FaultStatusCode::SEA_TTW_LEVEL3 => Self::SynchronousExternalAbortOnTableWalkLevel3, + FaultStatusCode::ECC_PARITY => Self::EccParity, + FaultStatusCode::ECC_PARITY_TTW_LEVEL_NEG1 => Self::EccParityOnTableWalkLevelNeg1, + FaultStatusCode::ECC_PARITY_TTW_LEVEL0 => Self::EccParityOnTableWalkLevel0, + FaultStatusCode::ECC_PARITY_TTW_LEVEL1 => Self::EccParityOnTableWalkLevel1, + FaultStatusCode::ECC_PARITY_TTW_LEVEL2 => Self::EccParityOnTableWalkLevel2, + FaultStatusCode::ECC_PARITY_TTW_LEVEL3 => Self::EccParityOnTableWalkLevel3, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL_NEG => { + Self::GranuleProtectionFaultLevelNeg1 + } + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL0 => Self::GranuleProtectionFaultLevel0, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL1 => Self::GranuleProtectionFaultLevel1, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL2 => Self::GranuleProtectionFaultLevel2, + FaultStatusCode::GRANULE_PROTECTION_FAULT_LEVEL3 => Self::GranuleProtectionFaultLevel3, + FaultStatusCode::ADDRESS_SIZE_FAULT_LEVEL_NEG1 => Self::AddressSizeFaultLevelNeg1, + FaultStatusCode::TRANSLATION_FAULT_LEVEL_NEG1 => Self::TranslationFaultLevelNeg1, + FaultStatusCode::TLB_CONFLICT_ABORT => Self::TlbConflictAbort, + FaultStatusCode::UNSUPPORTED_HW_UPDATE_FAULT => Self::UnsupportedHardwareUpdateFault, + _ => Self::Unknown, + } + } +} From 1d39dc7d6fb7705fcddeae978a3480f408f8f84c Mon Sep 17 00:00:00 2001 From: Flgodd67 Date: Tue, 16 Jun 2026 16:08:02 +0100 Subject: [PATCH 5/5] cca: adding a linux_only flag for unit tests so that certain test can be ignored if the tmk_vmm doesn't match the os making the aarch64 specific tests linux only using the flag changes to tmk_vmm test macros parsing changes to error reporting in Instruction Abort handling changing hcl_rsi_ipa_state_read from ioctl_read to ioctl_readwrite --- .../src/_jobs/local_install_cca_emu.rs | 2 +- openhcl/hcl/src/ioctl.rs | 2 +- .../virt_mshv_vtl/src/processor/cca/mod.rs | 27 +++++++++--- tmk/simple_tmk/src/aarch64/mod.rs | 6 +-- tmk/tmk_core/src/lib.rs | 5 +-- tmk/tmk_macros/src/lib.rs | 44 ++++++++++++++----- tmk/tmk_protocol/src/lib.rs | 5 ++- tmk/tmk_vmm/src/load.rs | 2 + tmk/tmk_vmm/src/run.rs | 5 +++ 9 files changed, 69 insertions(+), 29 deletions(-) diff --git a/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs b/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs index 7d90128a24..82af5b79c1 100644 --- a/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs +++ b/flowey/flowey_lib_hvlite/src/_jobs/local_install_cca_emu.rs @@ -16,7 +16,7 @@ use std::thread; const SHRINKWRAP_REPO: &str = "https://git.gitlab.arm.com/tooling/shrinkwrap.git"; // The guest Linux kernel (with cca/plane driver) hasn't been upstreamed yet, fetch it from our private repo const PLANE0_LINUX_REPO: &str = "https://github.com/jiong-microsoft/OHCL-Linux-Kernel.git"; -const PLANE0_LINUX_BRANCH: &str = "cca-dev-ben"; +const PLANE0_LINUX_BRANCH: &str = "cca-dev"; // A few config information needed when building Linux kernel const CCA_CONFIGS: &[&str] = &["CONFIG_VIRT_DRIVERS", "CONFIG_ARM_CCA_GUEST"]; const NINEP_CONFIGS: &[&str] = &[ diff --git a/openhcl/hcl/src/ioctl.rs b/openhcl/hcl/src/ioctl.rs index 0878f9fbc4..3043e00fc2 100755 --- a/openhcl/hcl/src/ioctl.rs +++ b/openhcl/hcl/src/ioctl.rs @@ -619,7 +619,7 @@ pub(crate) mod ioctls { ); // CCA: Get the RIPAS state of an ipa - ioctl_read!( + ioctl_readwrite!( hcl_rsi_ipa_state_read, MSHV_IOCTL, MSHV_VTL_RSI_GET_IPA_STATE, diff --git a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs index a18cd676ae..c655190b00 100644 --- a/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs +++ b/openhcl/virt_mshv_vtl/src/processor/cca/mod.rs @@ -375,9 +375,20 @@ impl BackingPrivate for CcaBacked { if iss.fnv() { tracing::warn!("CCA InstructionAbort: FAR_EL2 is not valid"); - return Err( - dev.fatal_error(CcaUnsupportedExit::ExitReason(0).into()) - ); + + return Err(dev.fatal_error( + CcaUnsupportedExit::InstructionAbort { + esr_el2: cca_exit.0.esr_el2, + elr_el2: cca_exit.elr_el2(), + far_el2: cca_exit.far_el2(), + fipa: 0, + fipa_state: u8::MAX, + ifsc: iss.ifsc().0, + reason: InstructionAbortReason::Unknown, + far_not_valid: iss.fnv(), + } + .into(), + )); } let far = cca_exit.far_el2(); @@ -387,9 +398,13 @@ impl BackingPrivate for CcaBacked { fipa, state: u64::MAX, }; - let _ = this - .ipa_state_read(GuestVtl::Vtl0, &mut plane_state) - .map_err(|_| Error::Hcl); + if let Err(e) = this.ipa_state_read(GuestVtl::Vtl0, &mut plane_state) { + tracing::warn!( + error = ?e, + fipa, + "failed to read IPA state; state will be u8::MAX which is unavailable" + ); + } let reason = InstructionAbortReason::from(iss.ifsc()); diff --git a/tmk/simple_tmk/src/aarch64/mod.rs b/tmk/simple_tmk/src/aarch64/mod.rs index cf10950d6b..da8f5ba590 100755 --- a/tmk/simple_tmk/src/aarch64/mod.rs +++ b/tmk/simple_tmk/src/aarch64/mod.rs @@ -24,7 +24,7 @@ unsafe extern "C" { fn instruction_abort_outside_par_entry() -> !; } -#[tmk_test(expected_failure)] +#[tmk_test(expected_failure, linux_only)] fn instruction_abort_outside_par(_: TestContext<'_>) { log!("instruction_abort_outside_par"); @@ -47,7 +47,7 @@ unsafe extern "C" { fn instruction_abort_ripas_empty_entry() -> !; } -#[tmk_test(expected_failure)] +#[tmk_test(expected_failure, linux_only)] fn instruction_abort_ripas_empty(_: TestContext<'_>) { log!("instruction_abort_ripas_empty"); @@ -71,7 +71,7 @@ unsafe extern "C" { fn instruction_abort_permissions_enabled_entry() -> !; } -#[tmk_test(expected_failure)] +#[tmk_test(expected_failure, linux_only)] fn instruction_abort_permissions_enabled(_: TestContext<'_>) { log!("instruction_abort_permissions_enabled"); diff --git a/tmk/tmk_core/src/lib.rs b/tmk/tmk_core/src/lib.rs index 278cd78c50..8d5932b070 100644 --- a/tmk/tmk_core/src/lib.rs +++ b/tmk/tmk_core/src/lib.rs @@ -144,9 +144,6 @@ unsafe extern "C" { #[doc(hidden)] #[macro_export] macro_rules! define_tmk_test { - ($name:expr, $func:ident) => { - $crate::define_tmk_test!($name, $func, 0); - }; ($name:expr, $func:ident, $flags:expr) => { const _: () = { // Strip the crate name from the module path. @@ -181,7 +178,7 @@ pub struct TestDescriptor { pub name: &'static [u8], /// The test entry point. pub entrypoint: for<'scope> fn(TestContext<'scope>), - /// Test flag - expected failure or not + /// Test flags pub flags: TestFlags64, } diff --git a/tmk/tmk_macros/src/lib.rs b/tmk/tmk_macros/src/lib.rs index 396f74280c..938f942cdb 100644 --- a/tmk/tmk_macros/src/lib.rs +++ b/tmk/tmk_macros/src/lib.rs @@ -8,6 +8,7 @@ use proc_macro::TokenStream; use quote::ToTokens; use quote::quote; +use syn::parse::Parser; /// `tmk_test` procedural attribute macro. /// @@ -18,20 +19,39 @@ pub fn tmk_test(attr: TokenStream, item: TokenStream) -> TokenStream { let name = item.sig.ident.to_string(); let func = &item.sig.ident; - let flags = match attr.to_string().as_str() { - "" => quote! { ::tmk_protocol::TestFlags64::new() }, - "expected_failure" => { - quote! { ::tmk_protocol::TestFlags64::new().with_expected_failure(true) } + let mut expected_failure = false; + let mut linux_only = false; + + let parser = syn::meta::parser(|meta| { + if meta.path.is_ident("expected_failure") { + expected_failure = true; + Ok(()) + } else if meta.path.is_ident("linux_only") { + linux_only = true; + Ok(()) + } else { + let ident = meta + .path + .get_ident() + .map_or_else(|| "".to_string(), |exp| exp.to_string()); + + Err(meta.error(format!("unsupported tmk_test option: {ident}"))) } - attr => { - let msg = format!("unsupported tmk_test option: {attr}"); - return quote! { - compile_error!(#msg); - #item - } - .into_token_stream() - .into(); + }); + + if let Err(err) = parser.parse(attr) { + let compile_err = err.to_compile_error(); + return quote! { + #compile_err + #item } + .into(); + } + + let flags = quote! { + ::tmk_protocol::TestFlags64::new() + .with_expected_failure(#expected_failure) + .with_linux_only(#linux_only) }; quote! { diff --git a/tmk/tmk_protocol/src/lib.rs b/tmk/tmk_protocol/src/lib.rs index eab3d6592a..0e84156880 100644 --- a/tmk/tmk_protocol/src/lib.rs +++ b/tmk/tmk_protocol/src/lib.rs @@ -29,8 +29,9 @@ pub struct StartInput { pub struct TestFlags64 { #[bits(1)] pub expected_failure: bool, - - #[bits(63)] + #[bits(1)] + pub linux_only: bool, + #[bits(62)] reserved: u64, } diff --git a/tmk/tmk_vmm/src/load.rs b/tmk/tmk_vmm/src/load.rs index 23e91ac86f..ddb3961de4 100644 --- a/tmk/tmk_vmm/src/load.rs +++ b/tmk/tmk_vmm/src/load.rs @@ -177,6 +177,7 @@ pub struct TestInfo { pub name: String, pub index: u64, pub expected_failure: bool, + pub linux_only: bool, } /// Enumerate the tests from a TMK binary. @@ -230,6 +231,7 @@ pub fn enumerate_tests(tmk: &File) -> anyhow::Result> { name: name.to_string(), index: i as u64, expected_failure: t.flags.expected_failure(), + linux_only: t.flags.linux_only(), }); } diff --git a/tmk/tmk_vmm/src/run.rs b/tmk/tmk_vmm/src/run.rs index aa3eb2da53..790e1c5ffb 100644 --- a/tmk/tmk_vmm/src/run.rs +++ b/tmk/tmk_vmm/src/run.rs @@ -276,6 +276,11 @@ impl CommonState { for test in &tests { tracing::info!(target: "test", name = test.name, "test started"); + if test.linux_only && !cfg!(target_os = "linux") { + tracing::info!(target: "test", name = test.name, "test skipped, incompatible os"); + continue; + } + let mut vmtime_keeper = VmTimeKeeper::new(&self.driver, VmTime::from_100ns(0)); let vmtime_source = vmtime_keeper.builder().build(&self.driver).await.unwrap(); let mut ctx = RunContext {