From efb5ebe9e752272371820a8544093d7e529bd098 Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Wed, 10 Jun 2026 14:10:23 -0600 Subject: [PATCH 1/2] :bug: Fix dynamic handling of top-level initialization Problem: - The cached state of IRQs which are statically enabled is not properly initialized. Solution: - For IRQs which have been statically enabled, make sure the cached register state is also set to enabled. --- include/interrupt/dynamic_controller.hpp | 27 +++++++++++++++++++++--- include/interrupt/policies.hpp | 4 +++- test/interrupt/dynamic_controller.cpp | 25 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/include/interrupt/dynamic_controller.hpp b/include/interrupt/dynamic_controller.hpp index e1e71db2..7b9f33e6 100644 --- a/include/interrupt/dynamic_controller.hpp +++ b/include/interrupt/dynamic_controller.hpp @@ -413,6 +413,25 @@ struct dynamic_controller { by_registers); } + // set: mark cached state for irqs which are already enabled + template static auto set_internal_state() -> void { + constexpr auto by_registers = + stdx::gather_by::template from_irq>( + EnabledIrqs{}); + stdx::for_each( + [](stdx::tuple) -> void { + using reg_t = + decltype(detail::register_for< + typename I::enable_field_t>::template fn()); + if constexpr (not is_no_register_v) { + auto &value = cached_enable; + ((value |= mask_for()), ..., + (value |= mask_for())); + } + }, + by_registers); + } + public: using hal_t = Hal; struct mutex_t; @@ -434,9 +453,10 @@ struct dynamic_controller { using potential_enable_irqs_t = boost::mp11::mp_copy_if, detail::has_enable_field>; - using enable_irqs_t = boost::mp11::mp_set_difference< - potential_enable_irqs_t, - stdx::tuple>; + using active_irqs_t = stdx::tuple; + using enable_irqs_t = + boost::mp11::mp_set_difference; using active_resources_t = Policy::template active_resources_t; @@ -445,6 +465,7 @@ struct dynamic_controller { conc::call_in_critical_section([] { reset_internal_state(); + set_internal_state(); update(enable_irqs_t{}); }); } diff --git a/include/interrupt/policies.hpp b/include/interrupt/policies.hpp index 4a1ec1ab..a8fb50c3 100644 --- a/include/interrupt/policies.hpp +++ b/include/interrupt/policies.hpp @@ -149,6 +149,8 @@ using no_irqs_policy = irqs_policy<>; struct default_policy : all_resources_policy, all_flows_policy, - all_irqs_policy {}; + all_irqs_policy { + constexpr static auto dynamic_enable_top_level = false; +}; } // namespace dynamic_init } // namespace interrupt diff --git a/test/interrupt/dynamic_controller.cpp b/test/interrupt/dynamic_controller.cpp index c5334f18..86c9a5f1 100644 --- a/test/interrupt/dynamic_controller.cpp +++ b/test/interrupt/dynamic_controller.cpp @@ -101,6 +101,31 @@ TEST_CASE("dynamic init enables all fields", "[dynamic controller]") { CHECK(*v == EN_TOP::mask); } +TEST_CASE("dynamic init correctly handles top-level fields after manager init", + "[dynamic controller]") { + using namespace groov::literals; + reset_dynamic_state(); + + // manager init statically enables top level IRQs + using M = interrupt::manager, test_nexus>; + groov::test::set_value("enable_top"_r, EN_TOP::mask); + M::init(); + + auto v = groov::test::get_value("enable_top"_r); + REQUIRE(v); + CHECK(*v == EN_TOP::mask); + + M::dynamic_t::disable<"shared">(); + v = groov::test::get_value("enable_top"_r); + REQUIRE(v); + CHECK(*v == 0); + + M::dynamic_t::enable<"shared">(); + v = groov::test::get_value("enable_top"_r); + REQUIRE(v); + CHECK(*v == EN_TOP::mask); +} + namespace { using noflows_config_t = interrupt::root, From 33ae5b84501edef636e98dcbd7dfeb737fa1ff89 Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Mon, 15 Jun 2026 14:24:20 -0600 Subject: [PATCH 2/2] :art: Interrupt: rename `cached` to `shadow` Problem: - Interrupt register values are "cached", but it isn't really a cache: it's written through immediately and serves to avoid RMW. "Cache" implies things that don't occur; a better way to describe this is a shadow value. Solution: - Rename `cached_*` to `shadow_*` for the dynamic interrupt manager. --- include/interrupt/dynamic_controller.hpp | 32 ++++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/include/interrupt/dynamic_controller.hpp b/include/interrupt/dynamic_controller.hpp index 7b9f33e6..754a9fb6 100644 --- a/include/interrupt/dynamic_controller.hpp +++ b/include/interrupt/dynamic_controller.hpp @@ -292,12 +292,12 @@ struct dynamic_controller { using irqs_t = detail::collect_t; - // cached values for each enable register + // shadow values for each enable register template using register_t = typename Hal::template register_datatype_t; template - constinit static inline register_t cached_enable{}; + constinit static inline register_t shadow_enable{}; // which IRQs are potentially affected by a change? template @@ -339,20 +339,20 @@ struct dynamic_controller { } } - constexpr static auto force_write = [](auto reg, auto &cached_value, + constexpr static auto force_write = [](auto reg, auto &shadow_value, auto new_value) { - cached_value = new_value; - Hal::write(reg, cached_value); + shadow_value = new_value; + Hal::write(reg, shadow_value); }; - constexpr static auto write_if_changed = [](auto reg, auto &cached_value, + constexpr static auto write_if_changed = [](auto reg, auto &shadow_value, auto new_value) { - if (new_value != std::exchange(cached_value, new_value)) { - Hal::write(reg, cached_value); + if (new_value != std::exchange(shadow_value, new_value)) { + Hal::write(reg, shadow_value); } }; - // update cached values and write the changed registers + // update shadow values and write the changed registers template typename L, typename... Irqs> static auto update(L) -> void { @@ -374,9 +374,9 @@ struct dynamic_controller { (compute_register(value), ..., compute_register(value)); - auto &cached_value = cached_enable; - auto new_value = (cached_value & ~mask) | value; - WriteFn(r, cached_value, new_value); + auto &shadow_value = shadow_enable; + auto new_value = (shadow_value & ~mask) | value; + WriteFn(r, shadow_value, new_value); } }, by_registers); @@ -391,7 +391,7 @@ struct dynamic_controller { } // reset: mark resources, flows and all named irqs enabled - // and clear all cached state + // and clear all shadow state template static auto reset_internal_state() -> void { resource_enables = resources_t{ActiveResources{}}; @@ -407,13 +407,13 @@ struct dynamic_controller { decltype(detail::register_for< typename I::enable_field_t>::template fn()); if constexpr (not is_no_register_v) { - cached_enable = {}; + shadow_enable = {}; } }, by_registers); } - // set: mark cached state for irqs which are already enabled + // set: mark shadow state for irqs which are already enabled template static auto set_internal_state() -> void { constexpr auto by_registers = stdx::gather_by::template from_irq>( @@ -424,7 +424,7 @@ struct dynamic_controller { decltype(detail::register_for< typename I::enable_field_t>::template fn()); if constexpr (not is_no_register_v) { - auto &value = cached_enable; + auto &value = shadow_enable; ((value |= mask_for()), ..., (value |= mask_for())); }