[RFC PATCH v3 18/21] hw/intc/arm_gicv3: Implement NMI interrupt prioirty

Jinjie Ruan via posted 21 patches 9 months ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[RFC PATCH v3 18/21] hw/intc/arm_gicv3: Implement NMI interrupt prioirty
Posted by Jinjie Ruan via 9 months ago
If GICD_CTLR_DS bit is zero and the NMI is non-secure, the NMI prioirty
is higher than 0x80, otherwise it is higher than 0x0. And save NMI
super prioirty information in hppi.superprio to deliver NMI exception.
Since both GICR and GICD can deliver NMI, it is both necessary to check
whether the pending irq is NMI in gicv3_redist_update_noirqset and
gicv3_update_noirqset. And In irqbetter(), only a non-NMI with the same
priority and a smaller interrupt number can be preempted but not NMI.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v3:
- Add missing brace
---
 hw/intc/arm_gicv3.c | 63 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 7 deletions(-)

diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 0b8f79a122..75999edd19 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -21,7 +21,7 @@
 #include "hw/intc/arm_gicv3.h"
 #include "gicv3_internal.h"
 
-static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
+static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio, bool is_nmi)
 {
     /* Return true if this IRQ at this priority should take
      * precedence over the current recorded highest priority
@@ -33,11 +33,21 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
     if (prio < cs->hppi.prio) {
         return true;
     }
+
+    /*
+     * Current highest prioirity pending interrupt is not a NMI
+     * and the new IRQ is a NMI with same priority.
+     */
+    if (prio == cs->hppi.prio && !cs->hppi.superprio && is_nmi) {
+        return true;
+    }
+
     /* If multiple pending interrupts have the same priority then it is an
      * IMPDEF choice which of them to signal to the CPU. We choose to
      * signal the one with the lowest interrupt number.
      */
-    if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
+    if (prio == cs->hppi.prio && !cs->hppi.superprio &&
+        !is_nmi && irq <= cs->hppi.irq) {
         return true;
     }
     return false;
@@ -141,6 +151,8 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
     uint8_t prio;
     int i;
     uint32_t pend;
+    bool is_nmi = 0;
+    uint32_t superprio = 0;
 
     /* Find out which redistributor interrupts are eligible to be
      * signaled to the CPU interface.
@@ -152,10 +164,27 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
             if (!(pend & (1 << i))) {
                 continue;
             }
-            prio = cs->gicr_ipriorityr[i];
-            if (irqbetter(cs, i, prio)) {
+            superprio = extract32(cs->gicr_isuperprio, i, 1);
+
+            /* NMI */
+            if (superprio) {
+                is_nmi = 1;
+
+                /* DS = 0 & Non-secure NMI */
+                if ((!(cs->gic->gicd_ctlr & GICD_CTLR_DS)) &&
+                    extract32(cs->gicr_igroupr0, i, 1)) {
+                    prio = 0x80;
+                } else {
+                    prio = 0x0;
+                }
+            } else {
+               is_nmi = 0;
+               prio = cs->gicr_ipriorityr[i];
+            }
+            if (irqbetter(cs, i, prio, is_nmi)) {
                 cs->hppi.irq = i;
                 cs->hppi.prio = prio;
+                cs->hppi.superprio = is_nmi;
                 seenbetter = true;
             }
         }
@@ -168,7 +197,7 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
     if ((cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) && cs->gic->lpi_enable &&
         (cs->gic->gicd_ctlr & GICD_CTLR_EN_GRP1NS) &&
         (cs->hpplpi.prio != 0xff)) {
-        if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio)) {
+        if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio, false)) {
             cs->hppi.irq = cs->hpplpi.irq;
             cs->hppi.prio = cs->hpplpi.prio;
             cs->hppi.grp = cs->hpplpi.grp;
@@ -212,7 +241,9 @@ static void gicv3_update_noirqset(GICv3State *s, int start, int len)
 {
     int i;
     uint8_t prio;
+    bool is_nmi = 0;
     uint32_t pend = 0;
+    uint32_t superprio = 0;
 
     assert(start >= GIC_INTERNAL);
     assert(len > 0);
@@ -240,10 +271,28 @@ static void gicv3_update_noirqset(GICv3State *s, int start, int len)
              */
             continue;
         }
-        prio = s->gicd_ipriority[i];
-        if (irqbetter(cs, i, prio)) {
+
+        superprio = *gic_bmp_ptr32(s->superprio, i);
+        /* NMI */
+        if (superprio & (1 << (i & 0x1f))) {
+            is_nmi = 1;
+
+            /* DS = 0 & Non-secure NMI */
+            if ((!(s->gicd_ctlr & GICD_CTLR_DS)) &&
+                gicv3_gicd_group_test(s, i)) {
+                    prio = 0x80;
+            } else {
+                    prio = 0x0;
+            }
+        } else {
+            is_nmi = 0;
+            prio = s->gicd_ipriority[i];
+        }
+
+        if (irqbetter(cs, i, prio, is_nmi)) {
             cs->hppi.irq = i;
             cs->hppi.prio = prio;
+            cs->hppi.superprio = is_nmi;
             cs->seenbetter = true;
         }
     }
-- 
2.34.1
Re: [RFC PATCH v3 18/21] hw/intc/arm_gicv3: Implement NMI interrupt prioirty
Posted by Richard Henderson 9 months ago
On 2/23/24 00:32, Jinjie Ruan via wrote:
> If GICD_CTLR_DS bit is zero and the NMI is non-secure, the NMI prioirty
> is higher than 0x80, otherwise it is higher than 0x0. And save NMI
> super prioirty information in hppi.superprio to deliver NMI exception.
> Since both GICR and GICD can deliver NMI, it is both necessary to check
> whether the pending irq is NMI in gicv3_redist_update_noirqset and
> gicv3_update_noirqset. And In irqbetter(), only a non-NMI with the same
> priority and a smaller interrupt number can be preempted but not NMI.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v3:
> - Add missing brace
> ---
>   hw/intc/arm_gicv3.c | 63 ++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 56 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
> index 0b8f79a122..75999edd19 100644
> --- a/hw/intc/arm_gicv3.c
> +++ b/hw/intc/arm_gicv3.c
> @@ -21,7 +21,7 @@
>   #include "hw/intc/arm_gicv3.h"
>   #include "gicv3_internal.h"
>   
> -static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
> +static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio, bool is_nmi)
>   {
>       /* Return true if this IRQ at this priority should take
>        * precedence over the current recorded highest priority
> @@ -33,11 +33,21 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
>       if (prio < cs->hppi.prio) {
>           return true;
>       }
> +
> +    /*
> +     * Current highest prioirity pending interrupt is not a NMI
> +     * and the new IRQ is a NMI with same priority.
> +     */
> +    if (prio == cs->hppi.prio && !cs->hppi.superprio && is_nmi) {

It would be best to not mix terminology -- superpriority or nmi but not a mix.  It's 
unfortunate that the manual does so...

It is very tempting expand prio to more bits so that all of the rest of this Just Works.
Because of...

> +            if (superprio) {
> +                is_nmi = 1;
> +
> +                /* DS = 0 & Non-secure NMI */
> +                if ((!(cs->gic->gicd_ctlr & GICD_CTLR_DS)) &&
> +                    extract32(cs->gicr_igroupr0, i, 1)) {
> +                    prio = 0x80;
> +                } else {
> +                    prio = 0x0;
> +                }
> +            } else {
> +               is_nmi = 0;
> +               prio = cs->gicr_ipriorityr[i];

... the need to check GICD_CTLR_DS for interpreting superpriority within the continuum, 
per section 4.8.1 (NMI prioritization), it would seem that we could map

     Secure NMI         -> 0
     Non-secure NMI     -> 0x100
     prio 0x00 .. 0xff  -> prio * 2 + 1

which matches the ordering in Figure 4-6.

> @@ -240,10 +271,28 @@ static void gicv3_update_noirqset(GICv3State *s, int start, int len)
>                */
>               continue;
>           }
> -        prio = s->gicd_ipriority[i];
> -        if (irqbetter(cs, i, prio)) {
> +
> +        superprio = *gic_bmp_ptr32(s->superprio, i);
> +        /* NMI */
> +        if (superprio & (1 << (i & 0x1f))) {
> +            is_nmi = 1;
> +
> +            /* DS = 0 & Non-secure NMI */
> +            if ((!(s->gicd_ctlr & GICD_CTLR_DS)) &&
> +                gicv3_gicd_group_test(s, i)) {
> +                    prio = 0x80;
> +            } else {
> +                    prio = 0x0;
> +            }
> +        } else {
> +            is_nmi = 0;
> +            prio = s->gicd_ipriority[i];
> +        }

In any case, let's not have two copies of this resolution.


r~