[dif/alert_handler] Add alert configuration DIF. Previously, the only way to configure an alert within the alert handler was to build a rather large struct that would configure all alerts within a class. (Re)Configuring a single alert had the side of effect of reconfiguring the class too. This commit adds a DIF to configure a single alert, i.e., enable it, set its class, and lock it. This partially addresses #9899. Signed-off-by: Timothy Trippel <ttrippel@google.com>
diff --git a/sw/device/lib/dif/dif_alert_handler.c b/sw/device/lib/dif/dif_alert_handler.c index c967d6f..c2b5566 100644 --- a/sw/device/lib/dif/dif_alert_handler.c +++ b/sw/device/lib/dif/dif_alert_handler.c
@@ -20,9 +20,90 @@ "Expected seven local alerts!"); /** + * We use this to convert the class enum to the integer value that is assigned + * to each class in auto-generated register header file. We do this to make this + * code robust against changes to the class values in the auto-generated + * register header file. + */ +OT_WARN_UNUSED_RESULT +static bool class_to_uint32(dif_alert_handler_class_t alert_class, + uint32_t *classification) { + switch (alert_class) { + case kDifAlertHandlerClassA: + *classification = + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSA; + break; + case kDifAlertHandlerClassB: + *classification = + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSB; + break; + case kDifAlertHandlerClassC: + *classification = + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSC; + break; + case kDifAlertHandlerClassD: + *classification = + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSD; + break; + default: + return false; + } + return true; +} + +/** + * NOTE: the alert ID corresponds directly to the multireg index for each CSR. + * (I.e., alert N has enable multireg N). + */ +dif_result_t dif_alert_handler_configure_alert( + const dif_alert_handler_t *alert_handler, dif_alert_handler_alert_t alert, + dif_alert_handler_class_t alert_class, dif_toggle_t enabled, + dif_toggle_t locked) { + if (alert_handler == NULL || alert >= ALERT_HANDLER_PARAM_N_ALERTS || + !dif_is_valid_toggle(enabled) || !dif_is_valid_toggle(locked)) { + return kDifBadArg; + } + uint32_t classification; + if (!class_to_uint32(alert_class, &classification)) { + return kDifBadArg; + } + + // Check if configuration is locked. + ptrdiff_t regwen_offset = + ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + alert * sizeof(uint32_t); + if (!mmio_region_read32(alert_handler->base_addr, regwen_offset)) { + return kDifLocked; + } + + // Enable the alert. + ptrdiff_t enable_reg_offset = + ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t); + uint32_t enable_reg = bitfield_bit32_write( + 0, ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true); + mmio_region_write32_shadowed(alert_handler->base_addr, enable_reg_offset, + enable_reg); + + // Classify the alert. + ptrdiff_t class_reg_offset = ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + + alert * sizeof(uint32_t); + uint32_t class_reg = bitfield_field32_write( + 0, ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_FIELD, classification); + mmio_region_write32_shadowed(alert_handler->base_addr, class_reg_offset, + class_reg); + + // Lock the configuration. + if (locked == kDifToggleEnabled) { + mmio_region_write32(alert_handler->base_addr, regwen_offset, 0); + } + + return kDifOk; +} + +/** * Classifies alerts for a single alert class. Returns `false` if any of the * provided configuration is invalid. */ +// TODO(#9899): support locking the alert class configuration. OT_WARN_UNUSED_RESULT static bool classify_alerts(const dif_alert_handler_t *alert_handler, const dif_alert_handler_class_config_t *class) { @@ -31,67 +112,11 @@ } for (int i = 0; i < class->alerts_len; ++i) { - if (class->alerts[i] >= ALERT_HANDLER_PARAM_N_ALERTS) { + if (dif_alert_handler_configure_alert(alert_handler, class->alerts[i], + class->alert_class, kDifToggleEnabled, + kDifToggleDisabled) != kDifOk) { return false; } - - // Enable the alert. - // NOTE: the value in alerts[i] corresponds directly to the multireg index. - // (I.e., alert N has enable multireg N). - ptrdiff_t enable_reg_offset = ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + - class->alerts[i] * sizeof(uint32_t); - uint32_t enable_reg = - mmio_region_read32(alert_handler->base_addr, enable_reg_offset); - // TODO: we would like to use the generated macro for the ENABLE BIT OFFSET - // below for the alert with the given index/ID, not just assume they are - // the same across all regs in the multireg. However, making this assumption - // for now. - enable_reg = bitfield_bit32_write( - enable_reg, ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true); - mmio_region_write32_shadowed(alert_handler->base_addr, enable_reg_offset, - enable_reg); - - // Determine alert classification. - uint32_t classification; - switch (class->alert_class) { - case kDifAlertHandlerClassA: - classification = - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSA; - break; - case kDifAlertHandlerClassB: - classification = - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSB; - break; - case kDifAlertHandlerClassC: - classification = - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSC; - break; - case kDifAlertHandlerClassD: - classification = - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSD; - break; - default: - return false; - } - - // Classify the alert. - // NOTE: the value in alerts[i] corresponds directly to the multireg index. - // (I.e., alert N has enable multireg N). - ptrdiff_t class_reg_offset = - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + - class->alerts[i] * sizeof(uint32_t); - uint32_t class_reg = - mmio_region_read32(alert_handler->base_addr, class_reg_offset); - // TODO: we would like to use the generated macro for the BITFIELD - // below for the alert with the given index/ID, not just assume they are - // the same across all regs in the multireg. - class_reg = bitfield_field32_write( - class_reg, ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_FIELD, - classification); - mmio_region_write32_shadowed(alert_handler->base_addr, class_reg_offset, - class_reg); - - // TODO: support locking the alert class configuration. } return true;
diff --git a/sw/device/lib/dif/dif_alert_handler.h b/sw/device/lib/dif/dif_alert_handler.h index b7a18db..ad51bff 100644 --- a/sw/device/lib/dif/dif_alert_handler.h +++ b/sw/device/lib/dif/dif_alert_handler.h
@@ -329,6 +329,24 @@ dif_alert_handler_config_t config); /** + * Configures an alert in the alert handler. + * + * This operation is lock-protected. + * + * @param handler An alert handler handle. + * @param alert The alert to be configured. + * @param alert_class The class to assign the alert to. + * @param enabled The enablement state to configure the alert in. + * @param locked The locked state to configure the alert in. + * @return The result of the operation. + */ +OT_WARN_UNUSED_RESULT +dif_result_t dif_alert_handler_configure_alert( + const dif_alert_handler_t *alert_handler, dif_alert_handler_alert_t alert, + dif_alert_handler_class_t alert_class, dif_toggle_t enabled, + dif_toggle_t locked); + +/** * Locks out alert handler ping timer configuration. * * Once locked, `dif_alert_handler_configure()` will return
diff --git a/sw/device/lib/dif/dif_alert_handler_unittest.cc b/sw/device/lib/dif/dif_alert_handler_unittest.cc index 0f40295..78882eb 100644 --- a/sw/device/lib/dif/dif_alert_handler_unittest.cc +++ b/sw/device/lib/dif/dif_alert_handler_unittest.cc
@@ -30,6 +30,102 @@ dif_alert_handler_t alert_handler_ = {.base_addr = dev().region()}; }; +class AlertConfigTest : public AlertHandlerTest, + public testing::WithParamInterface< + std::tuple<int, dif_alert_handler_class_t>> {}; + +TEST_F(AlertConfigTest, BadArgs) { + EXPECT_EQ(dif_alert_handler_configure_alert( + &alert_handler_, ALERT_HANDLER_PARAM_N_ALERTS, + kDifAlertHandlerClassA, kDifToggleEnabled, kDifToggleDisabled), + kDifBadArg); + + EXPECT_EQ( + dif_alert_handler_configure_alert( + &alert_handler_, 0, + static_cast<dif_alert_handler_class_t>(ALERT_HANDLER_PARAM_N_CLASSES), + kDifToggleEnabled, kDifToggleDisabled), + kDifBadArg); + + EXPECT_EQ(dif_alert_handler_configure_alert( + &alert_handler_, 0, kDifAlertHandlerClassA, + static_cast<dif_toggle_t>(2), kDifToggleDisabled), + kDifBadArg); + + EXPECT_EQ(dif_alert_handler_configure_alert( + &alert_handler_, 0, kDifAlertHandlerClassA, kDifToggleEnabled, + static_cast<dif_toggle_t>(2)), + kDifBadArg); +} + +TEST_F(AlertConfigTest, Locked) { + EXPECT_READ32(ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET, 0); + EXPECT_EQ(dif_alert_handler_configure_alert( + &alert_handler_, 0, kDifAlertHandlerClassA, kDifToggleEnabled, + kDifToggleDisabled), + kDifLocked); + + EXPECT_READ32(ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + + (ALERT_HANDLER_PARAM_N_ALERTS - 1) * sizeof(uint32_t), + 0); + EXPECT_EQ(dif_alert_handler_configure_alert( + &alert_handler_, ALERT_HANDLER_PARAM_N_ALERTS - 1, + kDifAlertHandlerClassD, kDifToggleEnabled, kDifToggleEnabled), + kDifLocked); +} + +TEST_P(AlertConfigTest, EnableOnly) { + dif_alert_handler_alert_t alert = std::get<0>(GetParam()); + dif_alert_handler_class_t alert_class = std::get<1>(GetParam()); + + EXPECT_READ32( + ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + alert * sizeof(uint32_t), + ALERT_HANDLER_ALERT_REGWEN_0_REG_RESVAL); + EXPECT_WRITE32_SHADOWED( + ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t), + {{ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true}}); + EXPECT_WRITE32_SHADOWED( + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + + alert * sizeof(uint32_t), + {{ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_OFFSET, alert_class}}); + + EXPECT_EQ( + dif_alert_handler_configure_alert(&alert_handler_, alert, alert_class, + kDifToggleEnabled, kDifToggleDisabled), + kDifOk); +} + +TEST_P(AlertConfigTest, EnableAndLock) { + dif_alert_handler_alert_t alert = std::get<0>(GetParam()); + dif_alert_handler_class_t alert_class = std::get<1>(GetParam()); + + EXPECT_READ32( + ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + alert * sizeof(uint32_t), + ALERT_HANDLER_ALERT_REGWEN_0_REG_RESVAL); + EXPECT_WRITE32_SHADOWED( + ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t), + {{ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true}}); + EXPECT_WRITE32_SHADOWED( + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + + alert * sizeof(uint32_t), + {{ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_OFFSET, alert_class}}); + EXPECT_WRITE32( + ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + alert * sizeof(uint32_t), 0); + + EXPECT_EQ( + dif_alert_handler_configure_alert(&alert_handler_, alert, alert_class, + kDifToggleEnabled, kDifToggleEnabled), + kDifOk); +} + +INSTANTIATE_TEST_SUITE_P( + AllAlertsAndClasses, AlertConfigTest, + testing::Combine(testing::Range(0, ALERT_HANDLER_PARAM_N_ALERTS), + testing::Values(kDifAlertHandlerClassA, + kDifAlertHandlerClassB, + kDifAlertHandlerClassC, + kDifAlertHandlerClassD))); + class ConfigTest : public AlertHandlerTest { // We provide our own dev_ member variable in this fixture, in order to // support IgnoreMmioCalls(). @@ -179,10 +275,13 @@ // Unfortunately, we can't use EXPECT_MASK for these reads/writes, since the // target registers are shadowed. for (auto alert : alerts_a) { + // The various alerts should be unlocked. + ptrdiff_t alert_regwen_offset = + ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + alert * sizeof(uint32_t); + EXPECT_READ32(alert_regwen_offset, ALERT_HANDLER_ALERT_REGWEN_0_REG_RESVAL); // The various alerts should be enabled. ptrdiff_t alert_enable_reg_offset = ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t); - EXPECT_READ32(alert_enable_reg_offset, 0); EXPECT_WRITE32_SHADOWED( alert_enable_reg_offset, {{ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true}}); @@ -190,7 +289,6 @@ ptrdiff_t alert_class_reg_offset = ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t); - EXPECT_READ32(alert_class_reg_offset, 0); EXPECT_WRITE32_SHADOWED( alert_class_reg_offset, ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSA); @@ -236,10 +334,13 @@ // Unfortunately, we can't use EXPECT_MASK for these reads/writes, since the // target registers are shadowed. for (auto alert : alerts_b) { + // The various alerts should be unlocked. + ptrdiff_t alert_regwen_offset = + ALERT_HANDLER_ALERT_REGWEN_0_REG_OFFSET + alert * sizeof(uint32_t); + EXPECT_READ32(alert_regwen_offset, ALERT_HANDLER_ALERT_REGWEN_0_REG_RESVAL); // The various alerts should be enabled. ptrdiff_t alert_enable_reg_offset = ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t); - EXPECT_READ32(alert_enable_reg_offset, 0); EXPECT_WRITE32_SHADOWED( alert_enable_reg_offset, {{ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true}}); @@ -247,7 +348,6 @@ ptrdiff_t alert_class_reg_offset = ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + alert * sizeof(uint32_t); - EXPECT_READ32(alert_class_reg_offset, 0); EXPECT_WRITE32_SHADOWED( alert_class_reg_offset, ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_VALUE_CLASSB);