qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Enhance maximum priority support of PLIC
@ 2022-09-30 12:32 Jim Shu
  2022-09-30 12:32 ` [PATCH v2 1/2] hw/intc: sifive_plic: fix hard-coded max priority level Jim Shu
  2022-09-30 12:32 ` [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field Jim Shu
  0 siblings, 2 replies; 6+ messages in thread
From: Jim Shu @ 2022-09-30 12:32 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: Alistair Francis, Bin Meng, Palmer Dabbelt, chigot, Jim Shu

This patchset fixes hard-coded maximum priority of interrupt priority
register and also changes this register to WARL field to align the PLIC
spec.

Changelog:

v2:
  * change interrupt priority register to WARL field.

Jim Shu (2):
  hw/intc: sifive_plic: fix hard-coded max priority level
  hw/intc: sifive_plic: change interrupt priority register to WARL field

 hw/intc/sifive_plic.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

-- 
2.17.1



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

* [PATCH v2 1/2] hw/intc: sifive_plic: fix hard-coded max priority level
  2022-09-30 12:32 [PATCH v2 0/2] Enhance maximum priority support of PLIC Jim Shu
@ 2022-09-30 12:32 ` Jim Shu
  2022-09-30 12:32 ` [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field Jim Shu
  1 sibling, 0 replies; 6+ messages in thread
From: Jim Shu @ 2022-09-30 12:32 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: Alistair Francis, Bin Meng, Palmer Dabbelt, chigot, Jim Shu,
	Emmanuel Blot

The maximum priority level is hard-coded when writing to interrupt
priority register. However, when writing to priority threshold register,
the maximum priority level is from num_priorities Property which is
configured by platform.

Also change interrupt priority register to use num_priorities Property
in maximum priority level.

Signed-off-by: Emmanuel Blot <emmanuel.blot@sifive.com>
Signed-off-by: Jim Shu <jim.shu@sifive.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
---
 hw/intc/sifive_plic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index af4ae3630e..f864efa761 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,8 +180,10 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
     if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
         uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
 
-        plic->source_priority[irq] = value & 7;
-        sifive_plic_update(plic);
+        if (value <= plic->num_priorities) {
+            plic->source_priority[irq] = value;
+            sifive_plic_update(plic);
+        }
     } else if (addr_between(addr, plic->pending_base,
                             plic->num_sources >> 3)) {
         qemu_log_mask(LOG_GUEST_ERROR,
-- 
2.17.1



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

* [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
  2022-09-30 12:32 [PATCH v2 0/2] Enhance maximum priority support of PLIC Jim Shu
  2022-09-30 12:32 ` [PATCH v2 1/2] hw/intc: sifive_plic: fix hard-coded max priority level Jim Shu
@ 2022-09-30 12:32 ` Jim Shu
  2022-09-30 12:58   ` Clément Chigot
  1 sibling, 1 reply; 6+ messages in thread
From: Jim Shu @ 2022-09-30 12:32 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: Alistair Francis, Bin Meng, Palmer Dabbelt, chigot, Jim Shu

PLIC spec [1] requires interrupt source priority registers are WARL
field and the number of supported priority is power-of-2 to simplify SW
discovery.

Existing QEMU RISC-V machine (e.g. shakti_c) don't strictly follow PLIC
spec, whose number of supported priority is not power-of-2. Just change
each bit of interrupt priority register to WARL field when the number of
supported priority is power-of-2.

[1] https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities

Signed-off-by: Jim Shu <jim.shu@sifive.com>
---
 hw/intc/sifive_plic.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index f864efa761..218ccff8bd 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
     if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
         uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
 
-        if (value <= plic->num_priorities) {
+        if ((plic->num_priorities + 1) & (plic->num_priorities)) {
+            /*
+             * if "num_priorities + 1" is power-of-2, make each register bit of
+             * interrupt priority WARL (Write-Any-Read-Legal). Just filter
+             * out the access to unsupported priority bits.
+             */
+            plic->source_priority[irq] = value % (plic->num_priorities + 1);
+            sifive_plic_update(plic);
+        } else if (value <= plic->num_priorities) {
             plic->source_priority[irq] = value;
             sifive_plic_update(plic);
         }
@@ -207,7 +215,16 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
         uint32_t contextid = (addr & (plic->context_stride - 1));
 
         if (contextid == 0) {
-            if (value <= plic->num_priorities) {
+            if ((plic->num_priorities + 1) & (plic->num_priorities)) {
+                /*
+                 * if "num_priorities + 1" is power-of-2, each register bit of
+                 * interrupt priority is WARL (Write-Any-Read-Legal). Just
+                 * filter out the access to unsupported priority bits.
+                 */
+                plic->target_priority[addrid] = value %
+                                                (plic->num_priorities + 1);
+                sifive_plic_update(plic);
+            } else if (value <= plic->num_priorities) {
                 plic->target_priority[addrid] = value;
                 sifive_plic_update(plic);
             }
-- 
2.17.1



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

* Re: [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
  2022-09-30 12:32 ` [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field Jim Shu
@ 2022-09-30 12:58   ` Clément Chigot
  2022-09-30 13:09     ` Jim Shu
  0 siblings, 1 reply; 6+ messages in thread
From: Clément Chigot @ 2022-09-30 12:58 UTC (permalink / raw)
  To: Jim Shu
  Cc: qemu-devel, qemu-riscv, Alistair Francis, Bin Meng, Palmer Dabbelt

Hi Jim,

On Fri, Sep 30, 2022 at 2:32 PM Jim Shu <jim.shu@sifive.com> wrote:
>
> PLIC spec [1] requires interrupt source priority registers are WARL
> field and the number of supported priority is power-of-2 to simplify SW
> discovery.
>
> Existing QEMU RISC-V machine (e.g. shakti_c) don't strictly follow PLIC
> spec, whose number of supported priority is not power-of-2. Just change
> each bit of interrupt priority register to WARL field when the number of
> supported priority is power-of-2.
>
> [1] https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities
>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> ---
>  hw/intc/sifive_plic.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index f864efa761..218ccff8bd 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
>      if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
>          uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
>
> -        if (value <= plic->num_priorities) {
> +        if ((plic->num_priorities + 1) & (plic->num_priorities)) {

That's the opposite. If n is a power of 2, n & (n-1) == 0 (eg 8 & 7 ==
 0, 9 & 8 == 8).
Note that n must be positive too. But I'm not sure it matters here.
I'll let you decide.

> +            /*
> +             * if "num_priorities + 1" is power-of-2, make each register bit of
> +             * interrupt priority WARL (Write-Any-Read-Legal). Just filter
> +             * out the access to unsupported priority bits.
> +             */
> +            plic->source_priority[irq] = value % (plic->num_priorities + 1);
> +            sifive_plic_update(plic);
> +        } else if (value <= plic->num_priorities) {
>              plic->source_priority[irq] = value;
>              sifive_plic_update(plic);
>          }
> @@ -207,7 +215,16 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
>          uint32_t contextid = (addr & (plic->context_stride - 1));
>
>          if (contextid == 0) {
> -            if (value <= plic->num_priorities) {
> +            if ((plic->num_priorities + 1) & (plic->num_priorities)) {

Same.

> +                /*
> +                 * if "num_priorities + 1" is power-of-2, each register bit of
> +                 * interrupt priority is WARL (Write-Any-Read-Legal). Just
> +                 * filter out the access to unsupported priority bits.
> +                 */
> +                plic->target_priority[addrid] = value %
> +                                                (plic->num_priorities + 1);
> +                sifive_plic_update(plic);
> +            } else if (value <= plic->num_priorities) {
>                  plic->target_priority[addrid] = value;
>                  sifive_plic_update(plic);
>              }
> --
> 2.17.1

Clément


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

* Re: [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
  2022-09-30 12:58   ` Clément Chigot
@ 2022-09-30 13:09     ` Jim Shu
  2022-10-03  4:13       ` Jim Shu
  0 siblings, 1 reply; 6+ messages in thread
From: Jim Shu @ 2022-09-30 13:09 UTC (permalink / raw)
  To: Clément Chigot
  Cc: qemu-devel, qemu-riscv, Alistair Francis, Bin Meng, Palmer Dabbelt

hi Clément,

Thank you very much.
I'll fix it in the next version patch.

Thanks,
Jim Shu



On Fri, Sep 30, 2022 at 8:58 PM Clément Chigot <chigot@adacore.com> wrote:
>
> Hi Jim,
>
> On Fri, Sep 30, 2022 at 2:32 PM Jim Shu <jim.shu@sifive.com> wrote:
> >
> > PLIC spec [1] requires interrupt source priority registers are WARL
> > field and the number of supported priority is power-of-2 to simplify SW
> > discovery.
> >
> > Existing QEMU RISC-V machine (e.g. shakti_c) don't strictly follow PLIC
> > spec, whose number of supported priority is not power-of-2. Just change
> > each bit of interrupt priority register to WARL field when the number of
> > supported priority is power-of-2.
> >
> > [1] https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-priorities
> >
> > Signed-off-by: Jim Shu <jim.shu@sifive.com>
> > ---
> >  hw/intc/sifive_plic.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> > index f864efa761..218ccff8bd 100644
> > --- a/hw/intc/sifive_plic.c
> > +++ b/hw/intc/sifive_plic.c
> > @@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
> >      if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
> >          uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
> >
> > -        if (value <= plic->num_priorities) {
> > +        if ((plic->num_priorities + 1) & (plic->num_priorities)) {
>
> That's the opposite. If n is a power of 2, n & (n-1) == 0 (eg 8 & 7 ==
>  0, 9 & 8 == 8).
> Note that n must be positive too. But I'm not sure it matters here.
> I'll let you decide.
>
> > +            /*
> > +             * if "num_priorities + 1" is power-of-2, make each register bit of
> > +             * interrupt priority WARL (Write-Any-Read-Legal). Just filter
> > +             * out the access to unsupported priority bits.
> > +             */
> > +            plic->source_priority[irq] = value % (plic->num_priorities + 1);
> > +            sifive_plic_update(plic);
> > +        } else if (value <= plic->num_priorities) {
> >              plic->source_priority[irq] = value;
> >              sifive_plic_update(plic);
> >          }
> > @@ -207,7 +215,16 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
> >          uint32_t contextid = (addr & (plic->context_stride - 1));
> >
> >          if (contextid == 0) {
> > -            if (value <= plic->num_priorities) {
> > +            if ((plic->num_priorities + 1) & (plic->num_priorities)) {
>
> Same.
>
> > +                /*
> > +                 * if "num_priorities + 1" is power-of-2, each register bit of
> > +                 * interrupt priority is WARL (Write-Any-Read-Legal). Just
> > +                 * filter out the access to unsupported priority bits.
> > +                 */
> > +                plic->target_priority[addrid] = value %
> > +                                                (plic->num_priorities + 1);
> > +                sifive_plic_update(plic);
> > +            } else if (value <= plic->num_priorities) {
> >                  plic->target_priority[addrid] = value;
> >                  sifive_plic_update(plic);
> >              }
> > --
> > 2.17.1
>
> Clément


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

* Re: [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field
  2022-09-30 13:09     ` Jim Shu
@ 2022-10-03  4:13       ` Jim Shu
  0 siblings, 0 replies; 6+ messages in thread
From: Jim Shu @ 2022-10-03  4:13 UTC (permalink / raw)
  To: Clément Chigot
  Cc: qemu-devel, qemu-riscv, Alistair Francis, Bin Meng, Palmer Dabbelt

Hi Clément,

> > > @@ -180,7 +180,15 @@ static void sifive_plic_write(void *opaque, hwaddr addr, uint64_t value,
> > >      if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) {
> > >          uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
> > >
> > > -        if (value <= plic->num_priorities) {
> > > +        if ((plic->num_priorities + 1) & (plic->num_priorities)) {
> >
> > That's the opposite. If n is a power of 2, n & (n-1) == 0 (eg 8 & 7 ==
> >  0, 9 & 8 == 8).
> > Note that n must be positive too. But I'm not sure it matters here.
> > I'll let you decide.
> >

num_priorities is a uint32_t variable so that n is always positive.


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

end of thread, other threads:[~2022-10-03  4:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-30 12:32 [PATCH v2 0/2] Enhance maximum priority support of PLIC Jim Shu
2022-09-30 12:32 ` [PATCH v2 1/2] hw/intc: sifive_plic: fix hard-coded max priority level Jim Shu
2022-09-30 12:32 ` [PATCH v2 2/2] hw/intc: sifive_plic: change interrupt priority register to WARL field Jim Shu
2022-09-30 12:58   ` Clément Chigot
2022-09-30 13:09     ` Jim Shu
2022-10-03  4:13       ` Jim Shu

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