Re: [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks

[Daniel Henrique Barboza]

On 10/29/25 4:17 AM, Nikita Novikov wrote:

Refactor the APLIC code to consolidate repeated conditions checking
whether an interrupt source is valid, delegated, or inactive.

Signed-off-by: Nikita Novikov <n.novi...@syntacore.com>
---

Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>

  hw/intc/riscv_aplic.c | 44 +++++++-------------------------------------
  1 file changed, 7 insertions(+), 37 deletions(-)

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 
8c3b16074cd3ca1bc3004cfaaa13f34b8860bd48..ccfbc9b4656f3e2a69eb5bcd1cee9e5762020351
 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -216,22 +216,13 @@ static inline bool 
riscv_aplic_source_active(RISCVAPLICState *aplic,
  static bool riscv_aplic_irq_rectified_val(RISCVAPLICState *aplic,
                                            uint32_t irq)
  {
-    uint32_t sourcecfg, sm, raw_input, irq_inverted;
+    uint32_t sm, raw_input, irq_inverted;

- if (!irq || aplic->num_irqs <= irq) {

-        return false;
-    }
-
-    sourcecfg = aplic->sourcecfg[irq];
-    if (sourcecfg & APLIC_SOURCECFG_D) {
-        return false;
-    }
-
-    sm = sourcecfg & APLIC_SOURCECFG_SM_MASK;
-    if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
+    if (!riscv_aplic_source_active(aplic, irq)) {
          return false;
      }

+ sm = aplic->sourcecfg[irq] & APLIC_SOURCECFG_SM_MASK;

      raw_input = (aplic->state[irq] & APLIC_ISTATE_INPUT) ? 1 : 0;
      irq_inverted = (sm == APLIC_SOURCECFG_SM_LEVEL_LOW ||
                      sm == APLIC_SOURCECFG_SM_EDGE_FALL) ? 1 : 0;
@@ -284,22 +275,13 @@ static void riscv_aplic_set_pending_raw(RISCVAPLICState 
*aplic,
  static void riscv_aplic_set_pending(RISCVAPLICState *aplic,
                                      uint32_t irq, bool pending)
  {
-    uint32_t sourcecfg, sm;
+    uint32_t sm;

- if ((irq <= 0) || (aplic->num_irqs <= irq)) {

-        return;
-    }
-
-    sourcecfg = aplic->sourcecfg[irq];
-    if (sourcecfg & APLIC_SOURCECFG_D) {
-        return;
-    }
-
-    sm = sourcecfg & APLIC_SOURCECFG_SM_MASK;
-    if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
+    if (!riscv_aplic_source_active(aplic, irq)) {
          return;
      }

+ sm = aplic->sourcecfg[irq] & APLIC_SOURCECFG_SM_MASK;

      if ((sm == APLIC_SOURCECFG_SM_LEVEL_HIGH) ||
          (sm == APLIC_SOURCECFG_SM_LEVEL_LOW)) {
          if (!aplic->msimode) {
@@ -370,19 +352,7 @@ static void riscv_aplic_set_enabled_raw(RISCVAPLICState 
*aplic,
  static void riscv_aplic_set_enabled(RISCVAPLICState *aplic,
                                      uint32_t irq, bool enabled)
  {
-    uint32_t sourcecfg, sm;
-
-    if ((irq <= 0) || (aplic->num_irqs <= irq)) {
-        return;
-    }
-
-    sourcecfg = aplic->sourcecfg[irq];
-    if (sourcecfg & APLIC_SOURCECFG_D) {
-        return;
-    }
-
-    sm = sourcecfg & APLIC_SOURCECFG_SM_MASK;
-    if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
+    if (!riscv_aplic_source_active(aplic, irq)) {
          return;
      }