1. Patchew
  2. QEMU
  3. [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code
  4. View patch

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

Nikita Novikov posted 2 patches 2 weeks, 2 days ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
Posted by Nikita Novikov 2 weeks, 2 days ago 
Refactor the APLIC code to consolidate repeated conditions checking
whether an interrupt source is valid, delegated, or inactive.

Signed-off-by: Nikita Novikov <n.novikov@syntacore.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;
     }
 

-- 
2.51.0
Re: [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
Posted by Alistair Francis 2 days, 17 hours ago 
On Wed, Oct 29, 2025 at 5:19 PM Nikita Novikov <n.novikov@syntacore.com> 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.novikov@syntacore.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  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;
>      }
>
>
> --
> 2.51.0
>
>
Re: [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
Posted by Daniel Henrique Barboza 2 weeks, 1 day ago 

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.novikov@syntacore.com>
> ---


Reviewed-by: Daniel Henrique Barboza <dbarboza@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;
>       }
>   
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.