* [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:07 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).