* [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code
@ 2025-10-29 7:17 Nikita Novikov
2025-10-29 7:17 ` [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i] Nikita Novikov
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Nikita Novikov @ 2025-10-29 7:17 UTC (permalink / raw)
To: qemu-devel
Cc: Nikita Novikov, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
During the debug of APLIC I faced with the problem with target[i]
registers of source i. According to RISC-V AIA spec v1.0 ratificated on
the 12-th of March, 2025, the section 4.5.2 ("Source configurations")
says, than 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. Currently we checks the source status
only using sm == INACTIVE, but we do not check whether the source is
delegated.
This series of patches expands current active source checking and reduces
code the associated code dublication.
Signed-off-by: Nikita Novikov <n.novikov@syntacore.com>
---
Nikita Novikov (2):
hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]
hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
hw/intc/riscv_aplic.c | 66 ++++++++++++++++++++++-----------------------------
1 file changed, 29 insertions(+), 37 deletions(-)
---
base-commit: d1bf06e8cc8197a2b18dbde820d957a51899d374
change-id: 20251028-n-novikov-aplic_aia_ro-1baa353cd672
Best regards,
--
Nikita Novikov <n.novikov@syntacore.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]
2025-10-29 7:17 [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Nikita Novikov
@ 2025-10-29 7:17 ` Nikita Novikov
2025-10-30 13:19 ` Daniel Henrique Barboza
2025-11-12 0:55 ` Alistair Francis
2025-10-29 7:17 ` [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks Nikita Novikov
2025-11-12 1:06 ` [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Alistair Francis
2 siblings, 2 replies; 8+ messages in thread
From: Nikita Novikov @ 2025-10-29 7:17 UTC (permalink / raw)
To: qemu-devel
Cc: Nikita Novikov, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
2025-10-29 7:17 [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Nikita Novikov
2025-10-29 7:17 ` [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i] Nikita Novikov
@ 2025-10-29 7:17 ` Nikita Novikov
2025-10-30 13:19 ` Daniel Henrique Barboza
2025-11-12 0:57 ` Alistair Francis
2025-11-12 1:06 ` [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Alistair Francis
2 siblings, 2 replies; 8+ messages in thread
From: Nikita Novikov @ 2025-10-29 7:17 UTC (permalink / raw)
To: qemu-devel
Cc: Nikita Novikov, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]
2025-10-29 7:17 ` [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i] Nikita Novikov
@ 2025-10-30 13:19 ` Daniel Henrique Barboza
2025-11-12 0:55 ` Alistair Francis
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Henrique Barboza @ 2025-10-30 13:19 UTC (permalink / raw)
To: Nikita Novikov, qemu-devel
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Liu Zhiwei
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 {
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
2025-10-29 7:17 ` [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks Nikita Novikov
@ 2025-10-30 13:19 ` Daniel Henrique Barboza
2025-11-12 0:57 ` Alistair Francis
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Henrique Barboza @ 2025-10-30 13:19 UTC (permalink / raw)
To: Nikita Novikov, qemu-devel
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Liu Zhiwei
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;
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]
2025-10-29 7:17 ` [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i] Nikita Novikov
2025-10-30 13:19 ` Daniel Henrique Barboza
@ 2025-11-12 0:55 ` Alistair Francis
1 sibling, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2025-11-12 0:55 UTC (permalink / raw)
To: Nikita Novikov
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
2025-10-29 7:17 ` [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks Nikita Novikov
2025-10-30 13:19 ` Daniel Henrique Barboza
@ 2025-11-12 0:57 ` Alistair Francis
1 sibling, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2025-11-12 0:57 UTC (permalink / raw)
To: Nikita Novikov
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code
2025-10-29 7:17 [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Nikita Novikov
2025-10-29 7:17 ` [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i] Nikita Novikov
2025-10-29 7:17 ` [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks Nikita Novikov
@ 2025-11-12 1:06 ` Alistair Francis
2 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2025-11-12 1:06 UTC (permalink / raw)
To: Nikita Novikov
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
On Wed, Oct 29, 2025 at 5:20 PM Nikita Novikov <n.novikov@syntacore.com> wrote:
>
> During the debug of APLIC I faced with the problem with target[i]
> registers of source i. According to RISC-V AIA spec v1.0 ratificated on
> the 12-th of March, 2025, the section 4.5.2 ("Source configurations")
> says, than 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. Currently we checks the source status
> only using sm == INACTIVE, but we do not check whether the source is
> delegated.
>
> This series of patches expands current active source checking and reduces
> code the associated code dublication.
>
> Signed-off-by: Nikita Novikov <n.novikov@syntacore.com>
> ---
> Nikita Novikov (2):
> hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i]
> hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> hw/intc/riscv_aplic.c | 66 ++++++++++++++++++++++-----------------------------
> 1 file changed, 29 insertions(+), 37 deletions(-)
> ---
> base-commit: d1bf06e8cc8197a2b18dbde820d957a51899d374
> change-id: 20251028-n-novikov-aplic_aia_ro-1baa353cd672
>
> Best regards,
> --
> Nikita Novikov <n.novikov@syntacore.com>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-11-12 1:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 7:17 [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Nikita Novikov
2025-10-29 7:17 ` [PATCH 1/2] hw/intc/riscv_aplic: Expand inactive source handling for AIA target[i] Nikita Novikov
2025-10-30 13:19 ` Daniel Henrique Barboza
2025-11-12 0:55 ` Alistair Francis
2025-10-29 7:17 ` [PATCH 2/2] hw/intc/riscv_aplic: Factor out source_active() and remove duplicate checks Nikita Novikov
2025-10-30 13:19 ` Daniel Henrique Barboza
2025-11-12 0:57 ` Alistair Francis
2025-11-12 1:06 ` [PATCH 0/2] hw/intc/riscv_aplic: Expand AIA target[i] source handling and refactor related code Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.