qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] intc/riscv_aplic: Fix target register read when source is inactive
@ 2025-07-24  9:34 Yang Jialong
  2025-07-24 17:39 ` Daniel Henrique Barboza
  2025-07-25  2:25 ` [PATCH v2] " Yang Jialong
  0 siblings, 2 replies; 4+ messages in thread
From: Yang Jialong @ 2025-07-24  9:34 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, Weiwei Li,
	Daniel Henrique Barboza, Liu Zhiwei
  Cc: yangjialong, Yang Jialong, qemu-riscv, qemu-devel

The RISC-V Advanced interrupt Architecture:
4.5.16. Interrupt targets:
If interrupt source i is inactive in this domain, register target[i] is
read-only zero.

Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
---
 hw/intc/riscv_aplic.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 4fa5f75..cfef69f 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -628,7 +628,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;
+    uint32_t irq, word, idc, sm;
     RISCVAPLICState *aplic = opaque;
 
     /* Reads must be 4 byte words */
@@ -696,6 +696,10 @@ 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) {
+            return 0;
+        }
         return aplic->target[irq];
     } else if (!aplic->msimode && (APLIC_IDC_BASE <= addr) &&
             (addr < (APLIC_IDC_BASE + aplic->num_harts * APLIC_IDC_SIZE))) {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] intc/riscv_aplic: Fix target register read when source is inactive
  2025-07-24  9:34 [PATCH v1] intc/riscv_aplic: Fix target register read when source is inactive Yang Jialong
@ 2025-07-24 17:39 ` Daniel Henrique Barboza
  2025-07-25  2:08   ` 回复: " z_bajeer
  2025-07-25  2:25 ` [PATCH v2] " Yang Jialong
  1 sibling, 1 reply; 4+ messages in thread
From: Daniel Henrique Barboza @ 2025-07-24 17:39 UTC (permalink / raw)
  To: Yang Jialong, Palmer Dabbelt, Alistair Francis, Weiwei Li,
	Liu Zhiwei
  Cc: yangjialong, qemu-riscv, qemu-devel



On 7/24/25 6:34 AM, Yang Jialong wrote:
> The RISC-V Advanced interrupt Architecture:
> 4.5.16. Interrupt targets:
> If interrupt source i is inactive in this domain, register target[i] is
> read-only zero.
> 
> Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
> ---
>   hw/intc/riscv_aplic.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index 4fa5f75..cfef69f 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -628,7 +628,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;
> +    uint32_t irq, word, idc, sm;
>       RISCVAPLICState *aplic = opaque;
>   
>       /* Reads must be 4 byte words */
> @@ -696,6 +696,10 @@ 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;

I believe you want '&' here:


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

Otherwise, given that APLIC_SOURCECFG_SM_INACTIVE is 0x0, the only way

sm == APLIC_SOURCECFG_SM_INACTIVE

will happen is aplic->sourcecfg[irq] being 0.


Thanks,

Daniel

> +        if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
> +            return 0;
> +        }
>           return aplic->target[irq];
>       } else if (!aplic->msimode && (APLIC_IDC_BASE <= addr) &&
>               (addr < (APLIC_IDC_BASE + aplic->num_harts * APLIC_IDC_SIZE))) {



^ permalink raw reply	[flat|nested] 4+ messages in thread

* 回复: [PATCH v1] intc/riscv_aplic: Fix target register read when source is inactive
  2025-07-24 17:39 ` Daniel Henrique Barboza
@ 2025-07-25  2:08   ` z_bajeer
  0 siblings, 0 replies; 4+ messages in thread
From: z_bajeer @ 2025-07-25  2:08 UTC (permalink / raw)
  To: Daniel Henrique Barboza, Palmer Dabbelt, Alistair Francis,
	Weiwei Li, Liu Zhiwei
  Cc: yangjialong@rvcore.com, qemu-riscv@nongnu.org,
	qemu-devel@nongnu.org

> On 7/24/25 6:34 AM, Yang Jialong wrote:
> > The RISC-V Advanced interrupt Architecture:
> > 4.5.16. Interrupt targets:
> > If interrupt source i is inactive in this domain, register target[i] is
> > read-only zero.
> > 
> > Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
> > ---
> >   hw/intc/riscv_aplic.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> > index 4fa5f75..cfef69f 100644
> > --- a/hw/intc/riscv_aplic.c
> > +++ b/hw/intc/riscv_aplic.c
> > @@ -628,7 +628,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;
> > +    uint32_t irq, word, idc, sm;
> >       RISCVAPLICState *aplic = opaque;
> >   
> >       /* Reads must be 4 byte words */
> > @@ -696,6 +696,10 @@ 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;
> 
> I believe you want '&' here:
> 

... Yes. Thanks. I will submit a newer one.

> 
> sm = aplic->sourcecfg[irq] & APLIC_SOURCECFG_SM_MASK;
> 
> Otherwise, given that APLIC_SOURCECFG_SM_INACTIVE is 0x0, the only way
> 
> sm == APLIC_SOURCECFG_SM_INACTIVE
> 
> will happen is aplic->sourcecfg[irq] being 0.
> 
> 
> Thanks,
> 
> Daniel
> 
> > +        if (sm == APLIC_SOURCECFG_SM_INACTIVE) {
> > +            return 0;
> > +        }
> >           return aplic->target[irq];
> >       } else if (!aplic->msimode && (APLIC_IDC_BASE <= addr) &&
> >               (addr < (APLIC_IDC_BASE + aplic->num_harts * APLIC_IDC_SIZE))) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] intc/riscv_aplic: Fix target register read when source is inactive
  2025-07-24  9:34 [PATCH v1] intc/riscv_aplic: Fix target register read when source is inactive Yang Jialong
  2025-07-24 17:39 ` Daniel Henrique Barboza
@ 2025-07-25  2:25 ` Yang Jialong
  1 sibling, 0 replies; 4+ messages in thread
From: Yang Jialong @ 2025-07-25  2:25 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, Weiwei Li,
	Daniel Henrique Barboza, Liu Zhiwei
  Cc: yangjialong, Yang Jialong, qemu-riscv, qemu-devel

The RISC-V Advanced interrupt Architecture:
4.5.16. Interrupt targets:
If interrupt source i is inactive in this domain, register target[i] is
read-only zero.

Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
---
 hw/intc/riscv_aplic.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

v1 --> v2:
- Use '&' replace the error '*'.

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 4fa5f7597b..a1d9fa5085 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -628,7 +628,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;
+    uint32_t irq, word, idc, sm;
     RISCVAPLICState *aplic = opaque;
 
     /* Reads must be 4 byte words */
@@ -696,6 +696,10 @@ 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) {
+            return 0;
+        }
         return aplic->target[irq];
     } else if (!aplic->msimode && (APLIC_IDC_BASE <= addr) &&
             (addr < (APLIC_IDC_BASE + aplic->num_harts * APLIC_IDC_SIZE))) {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-25  2:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-24  9:34 [PATCH v1] intc/riscv_aplic: Fix target register read when source is inactive Yang Jialong
2025-07-24 17:39 ` Daniel Henrique Barboza
2025-07-25  2:08   ` 回复: " z_bajeer
2025-07-25  2:25 ` [PATCH v2] " Yang Jialong

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).