ast10x0/hace: fix DMA cache coherency; decouple SCU from HACE ops
diff --git a/target/ast10x0/board/src/lib.rs b/target/ast10x0/board/src/lib.rs index e12c05d..4e7ff1b 100644 --- a/target/ast10x0/board/src/lib.rs +++ b/target/ast10x0/board/src/lib.rs
@@ -11,6 +11,7 @@ clippy::unimplemented )] +use ast10x0_peripherals::hace::HaceDevice; use ast10x0_peripherals::scu::{ClockRegisterHalf, ScuRegisterHalf}; use ast10x0_peripherals::scu::{PinctrlPin, ScuRegisters}; @@ -56,6 +57,21 @@ Self { descriptor } } + /// Create a [`HaceDevice`] bound to the singleton HACE instance. + /// + /// This is the primary factory for HACE access on AST10x0. The board + /// crate is the single point that wires the SCU cache-flush hook into + /// the HACE driver, keeping `ast10x0_peripherals::hace` free of any + /// direct SCU dependency at the operation level. + /// + /// # Safety + /// - Must not be called concurrently with any other HACE access. + /// - Only one [`HaceDevice`] should be live at a time. + pub unsafe fn hace_device<Y: FnMut(u32)>(&self, yield_fn: Y) -> HaceDevice<Y> { + // SAFETY: caller upholds the single-instance contract. + unsafe { HaceDevice::new_global(yield_fn) } + } + /// Initialize board: apply pinctrl groups and initialize I2C subsystem. /// /// This performs the complete platform-level initialization:
diff --git a/target/ast10x0/peripherals/BUILD.bazel b/target/ast10x0/peripherals/BUILD.bazel index 2d82246..cd069bc 100644 --- a/target/ast10x0/peripherals/BUILD.bazel +++ b/target/ast10x0/peripherals/BUILD.bazel
@@ -35,6 +35,7 @@ "i2c/transfer.rs", "i2c/types.rs", "lib.rs", + "scu/cache.rs", "scu/clock.rs", "scu/mod.rs", "scu/pinctrl.rs",
diff --git a/target/ast10x0/peripherals/hace/aes.rs b/target/ast10x0/peripherals/hace/aes.rs index fe63c34..3e729ab 100644 --- a/target/ast10x0/peripherals/hace/aes.rs +++ b/target/ast10x0/peripherals/hace/aes.rs
@@ -17,7 +17,7 @@ AES_CMD_BASE, HACE_CMD_AES128, HACE_CMD_AES256, HACE_CMD_CBC, HACE_CMD_ECB, HACE_CMD_ENCRYPT, HACE_SG_LAST, POLL_YIELD_NS, }; -use super::context::{CryptoContext, AES_DATA_CAP}; +use super::context::CryptoContext; use super::device::HaceDevice; use super::error::HaceError; use super::helpers::ptr_to_u32; @@ -40,6 +40,9 @@ poll_budget: u32, /// Cooperative yield hook called once per completion poll. yield_fn: &'a mut dyn FnMut(u32), + /// Cache flush hook: invalidates stale CPU cache lines after HACE DMA. + /// Injected from [`HaceDevice`] so this module has no direct SCU dependency. + cache_flush: fn(), } impl<'a> AesCipher<'a> { @@ -48,12 +51,14 @@ ctx: &'a mut CryptoContext, poll_budget: u32, yield_fn: &'a mut dyn FnMut(u32), + cache_flush: fn(), ) -> Self { Self { regs, ctx, poll_budget, yield_fn, + cache_flush, } } @@ -65,10 +70,11 @@ // Borrow split; retained `yield_fn` keeps the device exclusively borrowed. let regs = device.regs; let poll_budget = device.poll_budget; + let cache_flush = device.cache_flush; // SAFETY: single-instance device + exclusive live borrow gate access. let ctx: &'a mut CryptoContext = unsafe { &mut *device.crypto_ctx }; let yield_fn: &'a mut dyn FnMut(u32) = &mut device.yield_fn; - Self::new(regs, ctx, poll_budget, yield_fn) + Self::new(regs, ctx, poll_budget, yield_fn, cache_flush) } /// Map AES key length to command bits. Reject AES-192. @@ -93,22 +99,16 @@ input: &[u8], output: &mut [u8], ) -> Result<(), HaceError> { - // Enforce block-aligned sizing and DMA staging cap before programming. + // Enforce block-aligned sizing before programming. if input.is_empty() || input.len() % AES_BLOCK != 0 || output.len() < input.len() { return Err(HaceError::InvalidInput); } - if input.len() > AES_DATA_CAP { - return Err(HaceError::InvalidInput); - } let kbits = Self::keylen_bits(key)?; let len = u32::try_from(input.len()).map_err(|_| HaceError::InvalidInput)?; - // Engine context: IV at [0..16) for CBC, key at [16..16+keylen). + // Engine context: IV at [0..16) for IV modes, key at [16..16+keylen). self.ctx.ctx = [0u8; 64]; if let Some(iv) = iv { - // Length-proven array assignment: `iv` is `&[u8; AES_BLOCK]`, so cast - // the destination prefix to `&mut [u8; AES_BLOCK]` and copy as fixed - // arrays. `copy_from_slice` would keep a length-mismatch panic branch. if let Some(dst) = self.ctx.ctx.get_mut(..AES_BLOCK) { if let Ok(dst) = <&mut [u8; AES_BLOCK]>::try_from(dst) { *dst = *iv; @@ -119,43 +119,37 @@ dst.copy_from_slice(key); } - // DMA safety (D3): copy caller input into the .ram_nc staging buffer. - // The HACE engine reads/writes by physical address; if the caller's - // slice is in flash, rodata, or a cacheable stack frame the engine - // would silently read/write stale physical RAM. Staging through - // ctx.data_in / ctx.data_out (both inside the .ram_nc CryptoContext) - // guarantees the DMA addresses are always in non-cacheable SRAM. - if let Some(dst) = self.ctx.data_in.get_mut(..input.len()) { - dst.copy_from_slice(input); - } - - // SG descriptors: addr = .ram_nc staging buffers, len = bytes | HACE_SG_LAST. - let in_ptr = ptr_to_u32(self.ctx.data_in.as_ptr())?; - let out_ptr = ptr_to_u32(self.ctx.data_out.as_ptr())?; + // Point SG descriptors directly at the caller's SRAM buffers. + // + // The AST10x0 crypto MBUS reads and writes payload data from ordinary + // cacheable SRAM (below 0x000A0000). Staging through the `.ram_nc` + // window (0x000A0000+) works for the engine context and SG descriptors + // but not for source/destination payload data: putting payloads in + // `.ram_nc` causes the engine to fire the completion intflag without + // writing any output (observed on hardware). Callers must therefore + // supply buffers in ordinary SRAM (e.g. `static mut` or heap); the + // KAT uses `static mut AES_IN / AES_OUT` which are in `.bss` / regular + // SRAM and work correctly. + // + // After the engine completes, `dcache_invd_all()` is called to + // invalidate any stale cache lines over the output buffer so the CPU + // reads the engine-written data rather than pre-op cached zeros + // (authority: `hace_aspeed.c` calls `cache_data_invd_all()` after + // every crypto op; `aspeed-rust` does the same). + let in_ptr = ptr_to_u32(input.as_ptr())?; + let out_ptr = ptr_to_u32(output.as_mut_ptr())?; self.ctx.src.addr = in_ptr; self.ctx.src.len = len | HACE_SG_LAST; self.ctx.dst.addr = out_ptr; self.ctx.dst.len = len | HACE_SG_LAST; let cmd = AES_CMD_BASE | kbits | mode_bits | if encrypt { HACE_CMD_ENCRYPT } else { 0 }; - self.ctx.cmd = cmd; - // Program descriptor addresses, ctx base, and data length. let src_desc = ptr_to_u32(core::ptr::addr_of!(self.ctx.src))?; let dst_desc = ptr_to_u32(core::ptr::addr_of!(self.ctx.dst))?; let ctx_base = ptr_to_u32(self.ctx.ctx.as_ptr())?; - // HACE0C is the plain byte count (bits 0:27); HACE_SG_LAST lives only in - // the SG descriptor length words (ctx.src.len / ctx.dst.len), not here. let data_len = len; - // Wait for any in-progress crypto operation to finish before re-programming. - // Mirrors Zephyr `regmap_read_poll_timeout(...HACE_CRYPTO_BUSY...)` in - // `hace_aspeed.c:83`. Without this drain, programming the engine while - // CryptoEngStsFlag is still set (from a prior encrypt) causes the new - // decrypt command to be ignored and `data_out` to read back as zeros. - while self.regs.crypto_engine_is_busy() { - (self.yield_fn)(POLL_YIELD_NS); - } self.regs.clear_crypto_intflag(); self.regs .program_crypto_operation(src_desc, dst_desc, ctx_base, data_len, cmd); @@ -169,28 +163,21 @@ (self.yield_fn)(POLL_YIELD_NS); } + // Invalidate the data cache so the CPU reads what the engine wrote. + (self.cache_flush)(); + // Always clear key/IV material from the DMA context buffer. self.ctx.ctx = [0u8; 64]; if done { - // Copy result out of .ram_nc staging buffer to caller's output. - let n = input.len(); - output - .get_mut(..n) - .ok_or(HaceError::InvalidInput)? - .copy_from_slice(self.ctx.data_out.get(..n).ok_or(HaceError::InvalidInput)?); - // Scrub staging buffers so plaintext/ciphertext doesn't linger. - if let Some(s) = self.ctx.data_in.get_mut(..n) { - s.fill(0); - } - if let Some(s) = self.ctx.data_out.get_mut(..n) { - s.fill(0); - } Ok(()) } else { - if let Some(s) = self.ctx.data_in.get_mut(..input.len()) { - s.fill(0); - } + pw_log::error!( + "hace: AES timeout: HACE1C={:#010x}, cmd={:#010x}, len={}", + self.regs.read_hace1c() as u32, + cmd as u32, + len as u32, + ); Err(HaceError::Timeout) } }
diff --git a/target/ast10x0/peripherals/hace/context.rs b/target/ast10x0/peripherals/hace/context.rs index 3bd7544..d6c074e 100644 --- a/target/ast10x0/peripherals/hace/context.rs +++ b/target/ast10x0/peripherals/hace/context.rs
@@ -71,12 +71,14 @@ #[derive(Copy, Clone)] #[repr(C)] pub(crate) struct Sg { - /// Physical address of the data buffer. Must be first per hardware SG layout - /// (`aspeed_sg.addr` in `hace_aspeed.c`): HACE parses addr @ +0, len @ +4. - pub(crate) addr: u32, /// Byte length of the buffer, OR'd with `HACE_SG_LAST` (bit 31) for the - /// final/only entry in the scatter-gather list. + /// final/only entry in the scatter-gather list. Hardware SG format has + /// `len` at offset +0, `addr` at offset +4 — matches both the pinned + /// Zephyr `aspeed_sg` struct (`hace_aspeed.h`) and `aspeed-rust` + /// `AspeedSg { len, addr }`. HACE parses len @ +0, addr @ +4. pub(crate) len: u32, + /// Physical address of the data buffer. + pub(crate) addr: u32, } impl Sg { @@ -176,14 +178,6 @@ // `#[repr(C, align(64))]`, single-in-flight discipline (and the same // layout-sensitivity caution, goal.md §2.2) as `HashContext`. -/// Maximum AES payload (source + destination) that can be staged through the -/// `.ram_nc` DMA buffers inside [`CryptoContext`]. Operations larger than this -/// return [`super::error::HaceError::InvalidInput`]. -/// -/// 512 bytes = 32 AES blocks, covering all current use-cases (key-unwrap, -/// CFI attestation, etc.). Increase here if larger payloads are ever needed. -pub(crate) const AES_DATA_CAP: usize = 512; - #[repr(C, align(64))] pub(crate) struct CryptoContext { /// Engine context buffer: `[0..16)` IV (CBC), `[16..16+keylen)` raw key @@ -193,17 +187,6 @@ pub(crate) src: Sg, /// Destination SG descriptor. pub(crate) dst: Sg, - /// Command word composed per goal.md §1.9.2 (unused as engine input — the - /// driver writes it to HACE10 directly — kept for parity of layout/debug). - pub(crate) cmd: u32, - /// DMA-safe input staging buffer (`.ram_nc`). The caller's plaintext or - /// ciphertext is CPU-copied here before the engine is started, ensuring - /// the DMA source is always in non-cacheable SRAM regardless of where the - /// caller allocated the slice (flash, stack, cacheable SRAM). - pub(crate) data_in: [u8; AES_DATA_CAP], - /// DMA-safe output staging buffer (`.ram_nc`). The engine writes here; - /// the result is CPU-copied to the caller's output slice after completion. - pub(crate) data_out: [u8; AES_DATA_CAP], } impl CryptoContext { @@ -212,9 +195,6 @@ ctx: [0; 64], src: Sg::new(), dst: Sg::new(), - cmd: 0, - data_in: [0; AES_DATA_CAP], - data_out: [0; AES_DATA_CAP], } } }
diff --git a/target/ast10x0/peripherals/hace/device.rs b/target/ast10x0/peripherals/hace/device.rs index e1810e9..8f854cd 100644 --- a/target/ast10x0/peripherals/hace/device.rs +++ b/target/ast10x0/peripherals/hace/device.rs
@@ -6,6 +6,7 @@ use super::constants::DEFAULT_POLL_BUDGET; use super::context::{acquire_crypto_ctx, acquire_shared_ctx, CryptoContext, HashContext}; use super::registers::HaceRegisters; +use crate::scu::cache::dcache_invd_all as scu_dcache_invd_all; use crate::scu::{ClockRegisterHalf, ScuRegisterHalf, ScuRegisters}; #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -54,8 +55,16 @@ /// Argument is a suggested wait window in nanoseconds. pub(crate) yield_fn: Y, pub(crate) poll_budget: u32, + /// Cache flush hook called after HACE DMA writes to invalidate stale CPU + /// cache lines. Injected at construction so operation modules (digest, + /// aes) have no direct SCU dependency. + pub(crate) cache_flush: fn(), } +/// No-op cache flush for devices constructed without a SCU reference (e.g. +/// unit-test stubs using a raw register base pointer via `new`/`new_with_yield`). +fn fn_noop_flush() {} + impl<Y: FnMut(u32)> HaceDevice<Y> { /// Create a device bound to a raw HACE register block with a caller-provided /// cooperative yield strategy. @@ -76,6 +85,7 @@ crypto_ctx: unsafe { acquire_crypto_ctx() }, yield_fn, poll_budget: DEFAULT_POLL_BUDGET, + cache_flush: fn_noop_flush, } } @@ -126,6 +136,7 @@ Self { regs, + cache_flush: scu_dcache_invd_all, // SAFETY: the `unsafe fn new*` single-instance contract makes this // the sole live device, hence the sole holder of these pointers. ctx: unsafe { acquire_shared_ctx() },
diff --git a/target/ast10x0/peripherals/hace/digest.rs b/target/ast10x0/peripherals/hace/digest.rs index e0b43b7..c5b55ad 100644 --- a/target/ast10x0/peripherals/hace/digest.rs +++ b/target/ast10x0/peripherals/hace/digest.rs
@@ -103,6 +103,9 @@ /// invoked once between every completion poll. Type-erased so the adapter /// (and the `Digest*` trait impls) need not be generic over the strategy. pub(crate) yield_fn: &'a mut dyn FnMut(u32), + /// Cache flush hook: invalidates stale CPU cache lines after HACE DMA. + /// Injected from [`HaceDevice`] so this module has no direct SCU dependency. + pub(crate) cache_flush: fn(), _algo: PhantomData<T>, } @@ -114,12 +117,14 @@ ctx: &'a mut HashContext, poll_budget: u32, yield_fn: &'a mut dyn FnMut(u32), + cache_flush: fn(), ) -> Self { Self { regs, ctx, poll_budget, yield_fn, + cache_flush, _algo: PhantomData, } } @@ -141,13 +146,14 @@ // (`borrow-arbitrated-engine-exclusivity`, Checklist box 2/4). let regs = device.regs; let poll_budget = device.poll_budget; + let cache_flush = device.cache_flush; // SAFETY: the device holds the sole pointer to this `.ram_nc` context // (acquired once at its `unsafe fn new*` single-instance gate); the // caller upholds non-reentrancy and the live `&'a mut device` (pinned // by `yield_fn` below) gates it, so no other `&mut` to it is live. let ctx: &'a mut HashContext = unsafe { &mut *device.ctx }; let yield_fn: &'a mut dyn FnMut(u32) = &mut device.yield_fn; - Self::new(regs, ctx, poll_budget, yield_fn) + Self::new(regs, ctx, poll_budget, yield_fn, cache_flush) } /// DMA one full block held in `ctx.buffer` (always `.ram_nc`) to the engine. @@ -215,6 +221,7 @@ ctx: &mut *self.ctx, poll_budget: self.poll_budget, yield_fn: &mut *self.yield_fn, + cache_flush: self.cache_flush, _algo: PhantomData, }) } @@ -308,6 +315,8 @@ for _ in 0..this.poll_budget { if this.regs.hash_intflag_is_set() { + // Invalidate stale CPU cache lines over the digest buffer. + (this.cache_flush)(); let result = T::digest_from_context(this.ctx); // Cleanup context (mirrors cleanup_context in aspeed-rust). this.ctx.bufcnt = 0;
diff --git a/target/ast10x0/peripherals/hace/registers.rs b/target/ast10x0/peripherals/hace/registers.rs index 7bf5858..34a5068 100644 --- a/target/ast10x0/peripherals/hace/registers.rs +++ b/target/ast10x0/peripherals/hace/registers.rs
@@ -107,19 +107,17 @@ } #[inline] - pub(crate) fn crypto_engine_is_busy(&self) -> bool { - self.regs() - .hace1c() - .read() - .crypto_eng_sts_flag() - .bit_is_set() - } - #[inline] pub(crate) fn crypto_intflag_is_set(&self) -> bool { self.regs().hace1c().read().crypto_intflag().bit_is_set() } + /// Read the raw HACE1C status register value (for diagnostics). + #[inline] + pub(crate) fn read_hace1c(&self) -> u32 { + self.regs().hace1c().read().bits() + } + /// Program one crypto (AES) pass and start the engine. /// /// Writes, in authority order: source data address (HACE00), destination
diff --git a/target/ast10x0/peripherals/scu/cache.rs b/target/ast10x0/peripherals/scu/cache.rs new file mode 100644 index 0000000..0e8a20f --- /dev/null +++ b/target/ast10x0/peripherals/scu/cache.rs
@@ -0,0 +1,41 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! AST10x0 data-cache maintenance. + +use super::registers::ScuRegisters; + +impl ScuRegisters { + /// Invalidate the entire AST10x0 data cache. + /// + /// Toggles `DCACHE_CLEAN` (SCUA58 bit 1) low→high, mirroring the Zephyr + /// authority `cache_data_invd_all()` (`drivers/cache/cache_aspeed.c`, also + /// called after every crypto operation in `hace_aspeed.c:140`). + /// + /// Required after any HACE DMA write into cacheable SRAM (below + /// `0x000A0000`): the engine's writes bypass the CPU cache, so the CPU + /// would read stale pre-operation cache lines without this invalidation. + #[inline] + pub fn dcache_invd_all(&self) { + const DCACHE_CLEAN: u32 = 1 << 1; + // SAFETY: SCU MMIO read-modify-write; DSB only orders memory accesses. + unsafe { + let ctrl = self.regs().scua58().read().bits(); + self.regs().scua58().write(|w| w.bits(ctrl & !DCACHE_CLEAN)); + core::arch::asm!("dsb sy", options(nostack, preserves_flags)); + self.regs().scua58().write(|w| w.bits(ctrl | DCACHE_CLEAN)); + core::arch::asm!("dsb sy", options(nostack, preserves_flags)); + } + } +} + +/// Thin free-function wrapper around [`ScuRegisters::dcache_invd_all`]. +/// +/// Exists solely so the function can be stored as a bare `fn()` pointer in +/// [`HaceDevice::cache_flush`], which is wired at construction by the board +/// crate. All SCU register access goes through [`ScuRegisters`]. +pub fn dcache_invd_all() { + // SAFETY: singleton SCU; single-threaded HACE use is upheld by HaceDevice's + // single-instance contract. + unsafe { ScuRegisters::new_global() }.dcache_invd_all(); +}
diff --git a/target/ast10x0/peripherals/scu/mod.rs b/target/ast10x0/peripherals/scu/mod.rs index bc236ed..bd5cdcf 100644 --- a/target/ast10x0/peripherals/scu/mod.rs +++ b/target/ast10x0/peripherals/scu/mod.rs
@@ -3,6 +3,7 @@ //! AST10x0 System Control Unit (SCU) module. +pub mod cache; pub mod clock; pub mod pinctrl; pub mod registers;