hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code

[PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]

Posted by Nikita Novikov 2 weeks, 2 days ago

According to the RISC-V AIA v1.0, section 4.5.2 ("Source configurations"),
register target[i] shall be read-only zero when interrupt source i is inactive
in this domain. A source is inactive if it is delegated to a child domain or
its source mode is INACTIVE.

The previous implementation only checked SM == INACTIVE. This patch adds
full compliance:
- Return zero on read if D == 1 or SM == INACTIVE
- Ignore writes in both cases

Fixes: b6f1244678 ("intc/riscv_aplic: Fix target register read when source is inactive")
Signed-off-by: Nikita Novikov <n.novikov@syntacore.com>
---
 hw/intc/riscv_aplic.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index a2041e702245211ba3bcf4334301d7c86272e36f..8c3b16074cd3ca1bc3004cfaaa13f34b8860bd48 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -193,6 +193,26 @@ void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr)
 #endif
 }
 
+/*
+ * APLIC target[i] must be read-only zero if the source i is inactive
+ * in this domain (delegated or SM == INACTIVE)
+ */
+static inline bool riscv_aplic_source_active(RISCVAPLICState *aplic,
+                                             uint32_t irq)
+{
+    uint32_t sc, sm;
+
+    if ((irq == 0) || (aplic->num_irqs <= irq)) {
+        return false;
+    }
+    sc = aplic->sourcecfg[irq];
+    if (sc & APLIC_SOURCECFG_D) {
+        return false;
+    }
+    sm = sc & APLIC_SOURCECFG_SM_MASK;
+    return sm != APLIC_SOURCECFG_SM_INACTIVE;
+}
+
 static bool riscv_aplic_irq_rectified_val(RISCVAPLICState *aplic,
                                           uint32_t irq)
 {
@@ -635,7 +655,7 @@ static void riscv_aplic_request(void *opaque, int irq, int level)
 
 static uint64_t riscv_aplic_read(void *opaque, hwaddr addr, unsigned size)
 {
-    uint32_t irq, word, idc, sm;
+    uint32_t irq, word, idc;
     RISCVAPLICState *aplic = opaque;
 
     /* Reads must be 4 byte words */
@@ -703,8 +723,7 @@ static uint64_t riscv_aplic_read(void *opaque, hwaddr addr, unsigned size)
     } else if ((APLIC_TARGET_BASE <= addr) &&
             (addr < (APLIC_TARGET_BASE + (aplic->num_irqs - 1) * 4))) {
         irq = ((addr - APLIC_TARGET_BASE) >> 2) + 1;
-        sm = aplic->sourcecfg[irq] & APLIC_SOURCECFG_SM_MASK;
-        if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
+        if (!riscv_aplic_source_active(aplic, irq)) {
             return 0;
         }
         return aplic->target[irq];
@@ -841,6 +860,9 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
     } else if ((APLIC_TARGET_BASE <= addr) &&
             (addr < (APLIC_TARGET_BASE + (aplic->num_irqs - 1) * 4))) {
         irq = ((addr - APLIC_TARGET_BASE) >> 2) + 1;
+        if (!riscv_aplic_source_active(aplic, irq)) {
+            return;
+        }
         if (aplic->msimode) {
             aplic->target[irq] = value;
         } else {

-- 
2.51.0

Re: [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]

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:
>
> According to the RISC-V AIA v1.0, section 4.5.2 ("Source configurations"),
> register target[i] shall be read-only zero when interrupt source i is inactive
> in this domain. A source is inactive if it is delegated to a child domain or
> its source mode is INACTIVE.
>
> The previous implementation only checked SM == INACTIVE. This patch adds
> full compliance:
> - Return zero on read if D == 1 or SM == INACTIVE
> - Ignore writes in both cases
>
> Fixes: b6f1244678 ("intc/riscv_aplic: Fix target register read when source is inactive")
> Signed-off-by: Nikita Novikov <n.novikov@syntacore.com>

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

Alistair

> ---
>  hw/intc/riscv_aplic.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index a2041e702245211ba3bcf4334301d7c86272e36f..8c3b16074cd3ca1bc3004cfaaa13f34b8860bd48 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -193,6 +193,26 @@ void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr)
>  #endif
>  }
>
> +/*
> + * APLIC target[i] must be read-only zero if the source i is inactive
> + * in this domain (delegated or SM == INACTIVE)
> + */
> +static inline bool riscv_aplic_source_active(RISCVAPLICState *aplic,
> +                                             uint32_t irq)
> +{
> +    uint32_t sc, sm;
> +
> +    if ((irq == 0) || (aplic->num_irqs <= irq)) {
> +        return false;
> +    }
> +    sc = aplic->sourcecfg[irq];
> +    if (sc & APLIC_SOURCECFG_D) {
> +        return false;
> +    }
> +    sm = sc & APLIC_SOURCECFG_SM_MASK;
> +    return sm != APLIC_SOURCECFG_SM_INACTIVE;
> +}
> +
>  static bool riscv_aplic_irq_rectified_val(RISCVAPLICState *aplic,
>                                            uint32_t irq)
>  {
> @@ -635,7 +655,7 @@ static void riscv_aplic_request(void *opaque, int irq, int level)
>
>  static uint64_t riscv_aplic_read(void *opaque, hwaddr addr, unsigned size)
>  {
> -    uint32_t irq, word, idc, sm;
> +    uint32_t irq, word, idc;
>      RISCVAPLICState *aplic = opaque;
>
>      /* Reads must be 4 byte words */
> @@ -703,8 +723,7 @@ static uint64_t riscv_aplic_read(void *opaque, hwaddr addr, unsigned size)
>      } else if ((APLIC_TARGET_BASE <= addr) &&
>              (addr < (APLIC_TARGET_BASE + (aplic->num_irqs - 1) * 4))) {
>          irq = ((addr - APLIC_TARGET_BASE) >> 2) + 1;
> -        sm = aplic->sourcecfg[irq] & APLIC_SOURCECFG_SM_MASK;
> -        if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
> +        if (!riscv_aplic_source_active(aplic, irq)) {
>              return 0;
>          }
>          return aplic->target[irq];
> @@ -841,6 +860,9 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>      } else if ((APLIC_TARGET_BASE <= addr) &&
>              (addr < (APLIC_TARGET_BASE + (aplic->num_irqs - 1) * 4))) {
>          irq = ((addr - APLIC_TARGET_BASE) >> 2) + 1;
> +        if (!riscv_aplic_source_active(aplic, irq)) {
> +            return;
> +        }
>          if (aplic->msimode) {
>              aplic->target[irq] = value;
>          } else {
>
> --
> 2.51.0
>
>

Re: [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]

Posted by Daniel Henrique Barboza 2 weeks, 1 day ago


On 10/29/25 4:17 AM, Nikita Novikov wrote:
> According to the RISC-V AIA v1.0, section 4.5.2 ("Source configurations"),
> register target[i] shall be read-only zero when interrupt source i is inactive
> in this domain. A source is inactive if it is delegated to a child domain or
> its source mode is INACTIVE.
> 
> The previous implementation only checked SM == INACTIVE. This patch adds
> full compliance:
> - Return zero on read if D == 1 or SM == INACTIVE
> - Ignore writes in both cases
> 
> Fixes: b6f1244678 ("intc/riscv_aplic: Fix target register read when source is inactive")
> Signed-off-by: Nikita Novikov <n.novikov@syntacore.com>
> ---

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

>   hw/intc/riscv_aplic.c | 28 +++++++++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index a2041e702245211ba3bcf4334301d7c86272e36f..8c3b16074cd3ca1bc3004cfaaa13f34b8860bd48 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -193,6 +193,26 @@ void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr)
>   #endif
>   }
>   
> +/*
> + * APLIC target[i] must be read-only zero if the source i is inactive
> + * in this domain (delegated or SM == INACTIVE)
> + */
> +static inline bool riscv_aplic_source_active(RISCVAPLICState *aplic,
> +                                             uint32_t irq)
> +{
> +    uint32_t sc, sm;
> +
> +    if ((irq == 0) || (aplic->num_irqs <= irq)) {
> +        return false;
> +    }
> +    sc = aplic->sourcecfg[irq];
> +    if (sc & APLIC_SOURCECFG_D) {
> +        return false;
> +    }
> +    sm = sc & APLIC_SOURCECFG_SM_MASK;
> +    return sm != APLIC_SOURCECFG_SM_INACTIVE;
> +}
> +
>   static bool riscv_aplic_irq_rectified_val(RISCVAPLICState *aplic,
>                                             uint32_t irq)
>   {
> @@ -635,7 +655,7 @@ static void riscv_aplic_request(void *opaque, int irq, int level)
>   
>   static uint64_t riscv_aplic_read(void *opaque, hwaddr addr, unsigned size)
>   {
> -    uint32_t irq, word, idc, sm;
> +    uint32_t irq, word, idc;
>       RISCVAPLICState *aplic = opaque;
>   
>       /* Reads must be 4 byte words */
> @@ -703,8 +723,7 @@ static uint64_t riscv_aplic_read(void *opaque, hwaddr addr, unsigned size)
>       } else if ((APLIC_TARGET_BASE <= addr) &&
>               (addr < (APLIC_TARGET_BASE + (aplic->num_irqs - 1) * 4))) {
>           irq = ((addr - APLIC_TARGET_BASE) >> 2) + 1;
> -        sm = aplic->sourcecfg[irq] & APLIC_SOURCECFG_SM_MASK;
> -        if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
> +        if (!riscv_aplic_source_active(aplic, irq)) {
>               return 0;
>           }
>           return aplic->target[irq];
> @@ -841,6 +860,9 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>       } else if ((APLIC_TARGET_BASE <= addr) &&
>               (addr < (APLIC_TARGET_BASE + (aplic->num_irqs - 1) * 4))) {
>           irq = ((addr - APLIC_TARGET_BASE) >> 2) + 1;
> +        if (!riscv_aplic_source_active(aplic, irq)) {
> +            return;
> +        }
>           if (aplic->msimode) {
>               aplic->target[irq] = value;
>           } else {
>

[PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]
[PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks