From ddcbc21b1e9eaf133e758621ee8b09cdbdd7e301 Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Sun, 2 Aug 2026 22:09:18 +0200 Subject: [PATCH] earlgrey: Properly ack flash operation done interrupt Delay ack'ing of the interrupt with the PLIC until the handling routine has run, and has had the opportunity to address the situation with the IP, to cause interrupt request to be deasserted. Before this change, the PLIC would re-latch the interrupt immediately after it being ack'ed, as the interrupt request was still active at the flash IP. As a result, next flash operation would erroneously be considered done immediately. --- hal/blocking/flash/BUILD.bazel | 1 + hal/blocking/flash/flash.rs | 7 ++- target/earlgrey/firmware/hwe/BUILD.bazel | 2 +- target/earlgrey/firmware/hwe/flash_server.rs | 26 ++------- .../earlgrey/firmware/transport/BUILD.bazel | 1 + .../firmware/transport/flash_server.rs | 26 ++------- target/earlgrey/tests/bootinfo/BUILD.bazel | 2 +- .../earlgrey/tests/bootinfo/flash_server.rs | 26 ++------- target/earlgrey/tests/eflash/BUILD.bazel | 2 +- target/earlgrey/tests/eflash/flash_server.rs | 26 ++------- target/earlgrey/tests/spi_flash/BUILD.bazel | 2 +- .../tests/spi_flash/combined_flash_server.rs | 27 ++------- target/earlgrey/tests/usbdfu/BUILD.bazel | 2 +- target/earlgrey/tests/usbdfu/flash_server.rs | 26 ++------- util/blocking/BUILD.bazel | 24 ++++++++ util/blocking/lib.rs | 57 +++++++++++++++++++ util/types/BUILD.bazel | 1 + util/types/lib.rs | 9 --- 18 files changed, 123 insertions(+), 144 deletions(-) create mode 100644 util/blocking/BUILD.bazel create mode 100644 util/blocking/lib.rs diff --git a/hal/blocking/flash/BUILD.bazel b/hal/blocking/flash/BUILD.bazel index 6962a9c3d..15f97f6d9 100644 --- a/hal/blocking/flash/BUILD.bazel +++ b/hal/blocking/flash/BUILD.bazel @@ -27,6 +27,7 @@ rust_library( visibility = ["//visibility:public"], deps = [ ":driver", + "//util/blocking", "//util/io", "//util/types", ], diff --git a/hal/blocking/flash/flash.rs b/hal/blocking/flash/flash.rs index 072e83c7d..140fe4d20 100644 --- a/hal/blocking/flash/flash.rs +++ b/hal/blocking/flash/flash.rs @@ -8,8 +8,9 @@ use core::{cmp::min, num::NonZero}; pub use hal_flash_driver::FlashAddress; use hal_flash_driver::FlashDriver; +use util_blocking::Blocking; use util_io::RandomRead; -use util_types::{Blocking, PowerOf2Usize}; +use util_types::PowerOf2Usize; /// High-level flash interface. /// @@ -157,7 +158,7 @@ impl Flash for BlockingFlash Result<(), Self::Error> { self.driver.start_erase(start_addr, size)?; - self.blocking.wait_for_notification(); + let _token = self.blocking.wait_for_notification(); self.driver.complete_op() } /// Programs data into flash. @@ -179,7 +180,7 @@ impl Flash for BlockingFlash Result<(), ErrorCode> { let mut driver = @@ -47,7 +28,10 @@ fn flash_server() -> Result<(), ErrorCode> { } let flash = BlockingFlash { driver, - blocking: FlashCtrlInterrupt, + blocking: BlockingInterrupt { + handle: handle::FLASH_INTERRUPTS, + signals: signals::FLASH_CTRL_OP_DONE, + }, }; let mut flash_server = FlashIpcServer::new(flash); let mut buf = [0u8; 2064]; diff --git a/target/earlgrey/firmware/transport/BUILD.bazel b/target/earlgrey/firmware/transport/BUILD.bazel index c89487a3f..4140f5834 100644 --- a/target/earlgrey/firmware/transport/BUILD.bazel +++ b/target/earlgrey/firmware/transport/BUILD.bazel @@ -96,6 +96,7 @@ rust_process( "//target/earlgrey/registers:flash_ctrl_core", "//target/earlgrey/registers:spi_host", "//target/earlgrey/util", + "//util/blocking", "//util/error", "//util/ipc", "//util/types", diff --git a/target/earlgrey/firmware/transport/flash_server.rs b/target/earlgrey/firmware/transport/flash_server.rs index 4dfef1b55..5ce8d9028 100644 --- a/target/earlgrey/firmware/transport/flash_server.rs +++ b/target/earlgrey/firmware/transport/flash_server.rs @@ -18,8 +18,8 @@ use hal_flash::{BlockingFlash, FlashAddress}; use services_flash_server::FlashIpcServer; use spi_flash::SpiFlash; use spi_host::SpiHost0; +use util_blocking::BlockingInterrupt; use util_ipc::IpcHandle; -use util_types::Blocking; #[derive(Zfmt)] #[zfmt(format = "SPI Host init failed: {code:08x}")] @@ -33,25 +33,6 @@ struct SpiFlashInitFailed { code: u32, } -struct FlashCtrlInterrupt; - -impl Blocking for FlashCtrlInterrupt { - fn wait_for_notification(&self) { - loop { - if let Ok(w) = syscall::object_wait( - handle::FLASH_INTERRUPTS, - signals::FLASH_CTRL_OP_DONE, - Instant::MAX, - ) { - if w.pending_signals.contains(signals::FLASH_CTRL_OP_DONE) { - break; - } - } - } - let _ = syscall::interrupt_ack(handle::FLASH_INTERRUPTS, signals::FLASH_CTRL_OP_DONE); - } -} - fn flash_server() -> Result<(), ErrorCode> { let mut eflash_driver = EmbeddedFlash::new_with_interrupts(unsafe { flash_ctrl_core::FlashCtrl::new() }); @@ -62,7 +43,10 @@ fn flash_server() -> Result<(), ErrorCode> { } let eflash = BlockingFlash { driver: eflash_driver, - blocking: FlashCtrlInterrupt, + blocking: BlockingInterrupt { + handle: handle::FLASH_INTERRUPTS, + signals: signals::FLASH_CTRL_OP_DONE, + }, }; let mut eflash_server = FlashIpcServer::new(eflash); diff --git a/target/earlgrey/tests/bootinfo/BUILD.bazel b/target/earlgrey/tests/bootinfo/BUILD.bazel index 33dfe52f2..9399c18a7 100644 --- a/target/earlgrey/tests/bootinfo/BUILD.bazel +++ b/target/earlgrey/tests/bootinfo/BUILD.bazel @@ -57,10 +57,10 @@ rust_app( "//target/earlgrey/drivers:eflash_driver", "//target/earlgrey/registers:flash_ctrl_core", "//target/earlgrey/util", + "//util/blocking", "//util/error", "//util/ipc", "//util/panic", - "//util/types", "@pigweed//pw_kernel/userspace", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", diff --git a/target/earlgrey/tests/bootinfo/flash_server.rs b/target/earlgrey/tests/bootinfo/flash_server.rs index 93dfbd41e..249446013 100644 --- a/target/earlgrey/tests/bootinfo/flash_server.rs +++ b/target/earlgrey/tests/bootinfo/flash_server.rs @@ -13,28 +13,9 @@ use earlgrey_util::flash::EarlgreyFlashAddress; use eflash_driver::{EmbeddedFlash, Permission}; use hal_flash::{BlockingFlash, FlashAddress}; use services_flash_server::FlashIpcServer; +use util_blocking::BlockingInterrupt; use util_error::ErrorCode; use util_ipc::IpcHandle; -use util_types::Blocking; - -struct FlashCtrlInterrupt; - -impl Blocking for FlashCtrlInterrupt { - fn wait_for_notification(&self) { - loop { - if let Ok(w) = syscall::object_wait( - handle::FLASH_INTERRUPTS, - signals::FLASH_CTRL_OP_DONE, - Instant::MAX, - ) { - if w.pending_signals.contains(signals::FLASH_CTRL_OP_DONE) { - break; - } - } - } - let _ = syscall::interrupt_ack(handle::FLASH_INTERRUPTS, signals::FLASH_CTRL_OP_DONE); - } -} fn flash_server() -> Result<(), ErrorCode> { let mut driver = @@ -46,7 +27,10 @@ fn flash_server() -> Result<(), ErrorCode> { } let flash = BlockingFlash { driver, - blocking: FlashCtrlInterrupt, + blocking: BlockingInterrupt { + handle: handle::FLASH_INTERRUPTS, + signals: signals::FLASH_CTRL_OP_DONE, + }, }; let mut flash_server = FlashIpcServer::new(flash); let mut buf = [0u8; 2064]; diff --git a/target/earlgrey/tests/eflash/BUILD.bazel b/target/earlgrey/tests/eflash/BUILD.bazel index eb6a4e779..a28759b9f 100644 --- a/target/earlgrey/tests/eflash/BUILD.bazel +++ b/target/earlgrey/tests/eflash/BUILD.bazel @@ -27,10 +27,10 @@ rust_app( "//target/earlgrey/drivers:eflash_driver", "//target/earlgrey/registers:flash_ctrl_core", "//target/earlgrey/util", + "//util/blocking", "//util/error", "//util/ipc", "//util/panic", - "//util/types", "@pigweed//pw_kernel/userspace", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", diff --git a/target/earlgrey/tests/eflash/flash_server.rs b/target/earlgrey/tests/eflash/flash_server.rs index d1c53cb13..aab588c8b 100644 --- a/target/earlgrey/tests/eflash/flash_server.rs +++ b/target/earlgrey/tests/eflash/flash_server.rs @@ -13,28 +13,9 @@ use earlgrey_util::EarlgreyFlashAddress; use eflash_driver::{EmbeddedFlash, Permission}; use hal_flash::{BlockingFlash, FlashAddress}; use services_flash_server::FlashIpcServer; +use util_blocking::BlockingInterrupt; use util_error::ErrorCode; use util_ipc::IpcHandle; -use util_types::Blocking; - -struct FlashCtrlInterrupt; - -impl Blocking for FlashCtrlInterrupt { - fn wait_for_notification(&self) { - loop { - if let Ok(w) = syscall::object_wait( - handle::FLASH_INTERRUPTS, - signals::FLASH_CTRL_OP_DONE, - Instant::MAX, - ) { - if w.pending_signals.contains(signals::FLASH_CTRL_OP_DONE) { - break; - } - } - } - let _ = syscall::interrupt_ack(handle::FLASH_INTERRUPTS, signals::FLASH_CTRL_OP_DONE); - } -} fn flash_server() -> Result<(), ErrorCode> { pw_log::info!("flash_server: initializing driver"); @@ -47,7 +28,10 @@ fn flash_server() -> Result<(), ErrorCode> { } let flash = BlockingFlash { driver, - blocking: FlashCtrlInterrupt, + blocking: BlockingInterrupt { + handle: handle::FLASH_INTERRUPTS, + signals: signals::FLASH_CTRL_OP_DONE, + }, }; let mut flash_server = FlashIpcServer::new(flash); let mut buf = [0u8; 2064]; diff --git a/target/earlgrey/tests/spi_flash/BUILD.bazel b/target/earlgrey/tests/spi_flash/BUILD.bazel index 114bc229c..164bf3dea 100644 --- a/target/earlgrey/tests/spi_flash/BUILD.bazel +++ b/target/earlgrey/tests/spi_flash/BUILD.bazel @@ -31,10 +31,10 @@ rust_app( "//target/earlgrey/registers:flash_ctrl_core", "//target/earlgrey/registers:spi_host", "//target/earlgrey/util", + "//util/blocking", "//util/error", "//util/ipc", "//util/panic", - "//util/types", "@pigweed//pw_kernel/userspace", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", diff --git a/target/earlgrey/tests/spi_flash/combined_flash_server.rs b/target/earlgrey/tests/spi_flash/combined_flash_server.rs index 2388f343f..259619832 100644 --- a/target/earlgrey/tests/spi_flash/combined_flash_server.rs +++ b/target/earlgrey/tests/spi_flash/combined_flash_server.rs @@ -15,30 +15,10 @@ use spi_flash::SpiFlash; use spi_host::SpiHost0; use userspace::time::Instant; use userspace::{entry, syscall}; +use util_blocking::BlockingInterrupt; use util_error::ErrorCode; use util_ipc::IpcHandle; use util_panic as _; -use util_types::Blocking; - -// EFlash Interrupt Blocker -struct FlashCtrlInterrupt; - -impl Blocking for FlashCtrlInterrupt { - fn wait_for_notification(&self) { - loop { - if let Ok(w) = syscall::object_wait( - handle::FLASH_INTERRUPTS, - signals::FLASH_CTRL_OP_DONE, - Instant::MAX, - ) { - if w.pending_signals.contains(signals::FLASH_CTRL_OP_DONE) { - break; - } - } - } - let _ = syscall::interrupt_ack(handle::FLASH_INTERRUPTS, signals::FLASH_CTRL_OP_DONE); - } -} fn run_server() -> Result<(), ErrorCode> { // 1. Initialize EFlash driver. @@ -55,7 +35,10 @@ fn run_server() -> Result<(), ErrorCode> { let eflash = BlockingFlash { driver: eflash_driver, - blocking: FlashCtrlInterrupt, + blocking: BlockingInterrupt { + handle: handle::FLASH_INTERRUPTS, + signals: signals::FLASH_CTRL_OP_DONE, + }, }; let mut eflash_server = FlashIpcServer::new(eflash); diff --git a/target/earlgrey/tests/usbdfu/BUILD.bazel b/target/earlgrey/tests/usbdfu/BUILD.bazel index 1b686aa0a..13d598a80 100644 --- a/target/earlgrey/tests/usbdfu/BUILD.bazel +++ b/target/earlgrey/tests/usbdfu/BUILD.bazel @@ -61,10 +61,10 @@ rust_app( "//target/earlgrey/drivers:eflash_driver", "//target/earlgrey/registers:flash_ctrl_core", "//target/earlgrey/util", + "//util/blocking", "//util/error", "//util/ipc", "//util/panic", - "//util/types", "@pigweed//pw_kernel/userspace", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", diff --git a/target/earlgrey/tests/usbdfu/flash_server.rs b/target/earlgrey/tests/usbdfu/flash_server.rs index e4cf9859a..0b556557c 100644 --- a/target/earlgrey/tests/usbdfu/flash_server.rs +++ b/target/earlgrey/tests/usbdfu/flash_server.rs @@ -13,28 +13,9 @@ use earlgrey_util::flash::EarlgreyFlashAddress; use eflash_driver::{EmbeddedFlash, Permission}; use hal_flash::{BlockingFlash, FlashAddress}; use services_flash_server::FlashIpcServer; +use util_blocking::BlockingInterrupt; use util_error::ErrorCode; use util_ipc::IpcHandle; -use util_types::Blocking; - -struct FlashCtrlInterrupt; - -impl Blocking for FlashCtrlInterrupt { - fn wait_for_notification(&self) { - loop { - if let Ok(w) = syscall::object_wait( - handle::FLASH_INTERRUPTS, - signals::FLASH_CTRL_OP_DONE, - Instant::MAX, - ) { - if w.pending_signals.contains(signals::FLASH_CTRL_OP_DONE) { - break; - } - } - } - let _ = syscall::interrupt_ack(handle::FLASH_INTERRUPTS, signals::FLASH_CTRL_OP_DONE); - } -} fn flash_server() -> Result<(), ErrorCode> { let mut driver = @@ -46,7 +27,10 @@ fn flash_server() -> Result<(), ErrorCode> { } let flash = BlockingFlash { driver, - blocking: FlashCtrlInterrupt, + blocking: BlockingInterrupt { + handle: handle::FLASH_INTERRUPTS, + signals: signals::FLASH_CTRL_OP_DONE, + }, }; let mut flash_server = FlashIpcServer::new(flash); let mut buf = [0u8; 2056]; diff --git a/util/blocking/BUILD.bazel b/util/blocking/BUILD.bazel new file mode 100644 index 000000000..a26532d4d --- /dev/null +++ b/util/blocking/BUILD.bazel @@ -0,0 +1,24 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@rules_rust//rust:defs.bzl", "rust_library") + +rust_library( + name = "blocking", + srcs = [ + "lib.rs", + ], + crate_name = "util_blocking", + edition = "2024", + visibility = ["//visibility:public"], + deps = [ + "@pigweed//pw_status/rust:pw_status", + ] + select({ + "@platforms//os:none": [ + "@pigweed//pw_kernel/userspace", + ], + "//conditions:default": [ + "@pigweed//pw_time/rust:pw_time", + ], + }), +) diff --git a/util/blocking/lib.rs b/util/blocking/lib.rs new file mode 100644 index 000000000..542f3c559 --- /dev/null +++ b/util/blocking/lib.rs @@ -0,0 +1,57 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +#![no_std] + +use userspace::syscall; +use userspace::time::Instant; + +/// A trait for blocking on notifications. +/// +/// This trait is typically implemented by mechanisms that need to wait for +/// an event or notification from another part of the system (e.g., an interrupt). +pub trait Blocking { + /// Waits until a notification is received. + fn wait_for_notification(&self) -> impl Drop; +} + +/// A struct for blocking on interrupts. +/// +/// This struct allows threads to block until a particular interrupt occurs. +/// The interrupt will be ack'ed (thereby allowing new interrupt requests to be +/// latched) when the struct returned from `wait_for_notification()` is dropped. +pub struct BlockingInterrupt { + pub handle: u32, + pub signals: syscall::Signals, +} + +impl Blocking for BlockingInterrupt { + fn wait_for_notification(&self) -> impl Drop { + loop { + if let Ok(w) = syscall::object_wait( + self.handle, + self.signals, + Instant::MAX, + ) { + if w.pending_signals.contains(self.signals) { + return InterruptAckToken { + handle: self.handle, + signals: w.pending_signals, + }; + } + } + } + } +} + +/// A struct for ack'ing an interrupt with the PLIC, when the struct is dropped. +struct InterruptAckToken { + pub handle: u32, + pub signals: syscall::Signals, +} + +impl Drop for InterruptAckToken { + fn drop(&mut self) { + let _ = syscall::interrupt_ack(self.handle, self.signals); + } +} diff --git a/util/types/BUILD.bazel b/util/types/BUILD.bazel index ef0b9045b..ce33f960d 100644 --- a/util/types/BUILD.bazel +++ b/util/types/BUILD.bazel @@ -15,6 +15,7 @@ rust_library( edition = "2024", visibility = ["//visibility:public"], deps = [ + "@pigweed//pw_kernel/userspace", "@pigweed//pw_time/rust:pw_time", "@rust_crates//:zerocopy", ], diff --git a/util/types/lib.rs b/util/types/lib.rs index c6c8dedb7..3eabec3d3 100644 --- a/util/types/lib.rs +++ b/util/types/lib.rs @@ -13,12 +13,3 @@ pub use opcode::Opcode; pub use power_of_2::PowerOf2Usize; pub use time::MultiplyDuration; pub use time::Nanoseconds; - -/// A trait for blocking on notifications. -/// -/// This trait is typically implemented by mechanisms that need to wait for -/// an event or notification from another part of the system (e.g., an interrupt). -pub trait Blocking { - /// Waits until a notification is received. - fn wait_for_notification(&self); -}