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>
---
v4:
- Replace is_nmi with has_superprio to not a mix NMI and superpriority.
- Update the comment in irqbetter().
- Extract gicv3_get_priority() to avoid code repeat.
---
v3:
- Add missing brace
---
hw/intc/arm_gicv3.c | 71 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 63 insertions(+), 8 deletions(-)
diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 0b8f79a122..1d16a53b23 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -21,7 +21,8 @@
#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 has_superprio)
{
/* Return true if this IRQ at this priority should take
* precedence over the current recorded highest priority
@@ -33,11 +34,24 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
if (prio < cs->hppi.prio) {
return true;
}
+
+ /*
+ * Current highest prioirity pending interrupt is an IRQ without
+ * superpriority, the new IRQ with superpriority has same priority
+ * should signal to the CPU as it have the priority higher than
+ * the labelled 0x80 or 0x00.
+ */
+ if (prio == cs->hppi.prio && !cs->hppi.superprio && has_superprio) {
+ 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.
+ * signal the one with the lowest interrupt number if they don't have
+ * superpriority.
*/
- if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
+ if (prio == cs->hppi.prio && !cs->hppi.superprio &&
+ !has_superprio && irq <= cs->hppi.irq) {
return true;
}
return false;
@@ -129,6 +143,35 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
return pend;
}
+static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
+ uint32_t superprio, uint8_t *prio, int irq)
+{
+ bool has_superprio = false;
+
+ if (superprio) {
+ has_superprio = true;
+
+ /* DS = 0 & Non-secure NMI */
+ if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
+ ((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
+ (!is_redist && gicv3_gicd_group_test(cs->gic, irq)))) {
+ *prio = 0x80;
+ } else {
+ *prio = 0x0;
+ }
+ } else {
+ has_superprio = false;
+
+ if (is_redist) {
+ *prio = cs->gicr_ipriorityr[irq];
+ } else {
+ *prio = cs->gic->gicd_ipriority[irq];
+ }
+ }
+
+ return has_superprio;
+}
+
/* Update the interrupt status after state in a redistributor
* or CPU interface has changed, but don't tell the CPU i/f.
*/
@@ -141,6 +184,8 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
uint8_t prio;
int i;
uint32_t pend;
+ uint32_t superprio = 0;
+ bool has_superprio = false;
/* Find out which redistributor interrupts are eligible to be
* signaled to the CPU interface.
@@ -152,10 +197,13 @@ 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);
+ has_superprio = gicv3_get_priority(cs, true, superprio, &prio, i);
+
+ if (irqbetter(cs, i, prio, has_superprio)) {
cs->hppi.irq = i;
cs->hppi.prio = prio;
+ cs->hppi.superprio = has_superprio;
seenbetter = true;
}
}
@@ -168,7 +216,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;
@@ -213,6 +261,8 @@ static void gicv3_update_noirqset(GICv3State *s, int start, int len)
int i;
uint8_t prio;
uint32_t pend = 0;
+ uint32_t superprio = 0;
+ bool has_superprio = false;
assert(start >= GIC_INTERNAL);
assert(len > 0);
@@ -240,10 +290,15 @@ 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);
+ superprio = superprio & (1 << (i & 0x1f));
+ has_superprio = gicv3_get_priority(cs, false, superprio, &prio, i);
+
+ if (irqbetter(cs, i, prio, has_superprio)) {
cs->hppi.irq = i;
cs->hppi.prio = prio;
+ cs->hppi.superprio = has_superprio;
cs->seenbetter = true;
}
}
--
2.34.1On 2/29/24 03:10, 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>
> ---
> v4:
> - Replace is_nmi with has_superprio to not a mix NMI and superpriority.
> - Update the comment in irqbetter().
> - Extract gicv3_get_priority() to avoid code repeat.
> ---
> v3:
> - Add missing brace
> ---
> hw/intc/arm_gicv3.c | 71 ++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 63 insertions(+), 8 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
> index 0b8f79a122..1d16a53b23 100644
> --- a/hw/intc/arm_gicv3.c
> +++ b/hw/intc/arm_gicv3.c
> @@ -21,7 +21,8 @@
> #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 has_superprio)
> {
> /* Return true if this IRQ at this priority should take
> * precedence over the current recorded highest priority
> @@ -33,11 +34,24 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
> if (prio < cs->hppi.prio) {
> return true;
> }
> +
> + /*
> + * Current highest prioirity pending interrupt is an IRQ without
> + * superpriority, the new IRQ with superpriority has same priority
> + * should signal to the CPU as it have the priority higher than
> + * the labelled 0x80 or 0x00.
> + */
> + if (prio == cs->hppi.prio && !cs->hppi.superprio && has_superprio) {
> + 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.
> + * signal the one with the lowest interrupt number if they don't have
> + * superpriority.
> */
> - if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
> + if (prio == cs->hppi.prio && !cs->hppi.superprio &&
> + !has_superprio && irq <= cs->hppi.irq) {
> return true;
> }
> return false;
> @@ -129,6 +143,35 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
> return pend;
> }
>
> +static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
> + uint32_t superprio, uint8_t *prio, int irq)
> +{
> + bool has_superprio = false;
> +
> + if (superprio) {
> + has_superprio = true;
> +
> + /* DS = 0 & Non-secure NMI */
> + if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
> + ((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
> + (!is_redist && gicv3_gicd_group_test(cs->gic, irq)))) {
> + *prio = 0x80;
> + } else {
> + *prio = 0x0;
> + }
> + } else {
> + has_superprio = false;
> +
> + if (is_redist) {
> + *prio = cs->gicr_ipriorityr[irq];
> + } else {
> + *prio = cs->gic->gicd_ipriority[irq];
> + }
> + }
> +
> + return has_superprio;
> +}
Did you not like the idea to map {priority, !superpriority} into a single value?
It would eliminate the change in irqbetter(), which is a bit more complex than it needs to be.
> @@ -152,10 +197,13 @@ 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);
> + has_superprio = gicv3_get_priority(cs, true, superprio, &prio, i);
It would allow moving the read of gicr_isuperprio into gicv3_get_priority(), alongside the
read of gicr_ipriorityr.
Is there a bug here not handling is_redist for GCIR_INMI*?
> @@ -168,7 +216,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)) {
Always passing false here is incorrect -- again missing the redistributor nmi bit?
r~
On 2024/3/1 7:50, Richard Henderson wrote:
> On 2/29/24 03:10, 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>
>> ---
>> v4:
>> - Replace is_nmi with has_superprio to not a mix NMI and superpriority.
>> - Update the comment in irqbetter().
>> - Extract gicv3_get_priority() to avoid code repeat.
>> ---
>> v3:
>> - Add missing brace
>> ---
>> hw/intc/arm_gicv3.c | 71 ++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 63 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
>> index 0b8f79a122..1d16a53b23 100644
>> --- a/hw/intc/arm_gicv3.c
>> +++ b/hw/intc/arm_gicv3.c
>> @@ -21,7 +21,8 @@
>> #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 has_superprio)
>> {
>> /* Return true if this IRQ at this priority should take
>> * precedence over the current recorded highest priority
>> @@ -33,11 +34,24 @@ static bool irqbetter(GICv3CPUState *cs, int irq,
>> uint8_t prio)
>> if (prio < cs->hppi.prio) {
>> return true;
>> }
>> +
>> + /*
>> + * Current highest prioirity pending interrupt is an IRQ without
>> + * superpriority, the new IRQ with superpriority has same priority
>> + * should signal to the CPU as it have the priority higher than
>> + * the labelled 0x80 or 0x00.
>> + */
>> + if (prio == cs->hppi.prio && !cs->hppi.superprio && has_superprio) {
>> + 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.
>> + * signal the one with the lowest interrupt number if they don't
>> have
>> + * superpriority.
>> */
>> - if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
>> + if (prio == cs->hppi.prio && !cs->hppi.superprio &&
>> + !has_superprio && irq <= cs->hppi.irq) {
>> return true;
>> }
>> return false;
>> @@ -129,6 +143,35 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
>> return pend;
>> }
>> +static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
>> + uint32_t superprio, uint8_t *prio, int
>> irq)
>> +{
>> + bool has_superprio = false;
>> +
>> + if (superprio) {
>> + has_superprio = true;
>> +
>> + /* DS = 0 & Non-secure NMI */
>> + if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
>> + ((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
>> + (!is_redist && gicv3_gicd_group_test(cs->gic, irq)))) {
>> + *prio = 0x80;
>> + } else {
>> + *prio = 0x0;
>> + }
>> + } else {
>> + has_superprio = false;
>> +
>> + if (is_redist) {
>> + *prio = cs->gicr_ipriorityr[irq];
>> + } else {
>> + *prio = cs->gic->gicd_ipriority[irq];
>> + }
>> + }
>> +
>> + return has_superprio;
>> +}
>
> Did you not like the idea to map {priority, !superpriority} into a
> single value?
>
> It would eliminate the change in irqbetter(), which is a bit more
> complex than it needs to be.
>
>> @@ -152,10 +197,13 @@ 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);
>> + has_superprio = gicv3_get_priority(cs, true, superprio,
>> &prio, i);
>
> It would allow moving the read of gicr_isuperprio into
> gicv3_get_priority(), alongside the read of gicr_ipriorityr.
>
> Is there a bug here not handling is_redist for GCIR_INMI*?
There seems no bug. Since prio = cs->gicr_ipriorityr[i] or prio =
s->gicd_ipriority[i] are separately used originally,
>
>> @@ -168,7 +216,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)) {
>
> Always passing false here is incorrect -- again missing the
> redistributor nmi bit?
>
>
> r~
On 2024/3/1 7:50, Richard Henderson wrote:
> On 2/29/24 03:10, 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>
>> ---
>> v4:
>> - Replace is_nmi with has_superprio to not a mix NMI and superpriority.
>> - Update the comment in irqbetter().
>> - Extract gicv3_get_priority() to avoid code repeat.
>> ---
>> v3:
>> - Add missing brace
>> ---
>> hw/intc/arm_gicv3.c | 71 ++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 63 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
>> index 0b8f79a122..1d16a53b23 100644
>> --- a/hw/intc/arm_gicv3.c
>> +++ b/hw/intc/arm_gicv3.c
>> @@ -21,7 +21,8 @@
>> #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 has_superprio)
>> {
>> /* Return true if this IRQ at this priority should take
>> * precedence over the current recorded highest priority
>> @@ -33,11 +34,24 @@ static bool irqbetter(GICv3CPUState *cs, int irq,
>> uint8_t prio)
>> if (prio < cs->hppi.prio) {
>> return true;
>> }
>> +
>> + /*
>> + * Current highest prioirity pending interrupt is an IRQ without
>> + * superpriority, the new IRQ with superpriority has same priority
>> + * should signal to the CPU as it have the priority higher than
>> + * the labelled 0x80 or 0x00.
>> + */
>> + if (prio == cs->hppi.prio && !cs->hppi.superprio && has_superprio) {
>> + 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.
>> + * signal the one with the lowest interrupt number if they don't
>> have
>> + * superpriority.
>> */
>> - if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
>> + if (prio == cs->hppi.prio && !cs->hppi.superprio &&
>> + !has_superprio && irq <= cs->hppi.irq) {
>> return true;
>> }
>> return false;
>> @@ -129,6 +143,35 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
>> return pend;
>> }
>> +static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
>> + uint32_t superprio, uint8_t *prio, int
>> irq)
>> +{
>> + bool has_superprio = false;
>> +
>> + if (superprio) {
>> + has_superprio = true;
>> +
>> + /* DS = 0 & Non-secure NMI */
>> + if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
>> + ((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
>> + (!is_redist && gicv3_gicd_group_test(cs->gic, irq)))) {
>> + *prio = 0x80;
>> + } else {
>> + *prio = 0x0;
>> + }
>> + } else {
>> + has_superprio = false;
>> +
>> + if (is_redist) {
>> + *prio = cs->gicr_ipriorityr[irq];
>> + } else {
>> + *prio = cs->gic->gicd_ipriority[irq];
>> + }
>> + }
>> +
>> + return has_superprio;
>> +}
>
> Did you not like the idea to map {priority, !superpriority} into a
> single value?
>
> It would eliminate the change in irqbetter(), which is a bit more
> complex than it needs to be.
I will try to change to implement this mapping scheme.
>
>> @@ -152,10 +197,13 @@ 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);
>> + has_superprio = gicv3_get_priority(cs, true, superprio,
>> &prio, i);
>
> It would allow moving the read of gicr_isuperprio into
> gicv3_get_priority(), alongside the read of gicr_ipriorityr.
>
> Is there a bug here not handling is_redist for GCIR_INMI*?
>
>> @@ -168,7 +216,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)) {
>
> Always passing false here is incorrect -- again missing the
> redistributor nmi bit?
>
>
> r~
On 2024/3/4 20:18, Jinjie Ruan wrote:
>
>
> On 2024/3/1 7:50, Richard Henderson wrote:
>> On 2/29/24 03:10, 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>
>>> ---
>>> v4:
>>> - Replace is_nmi with has_superprio to not a mix NMI and superpriority.
>>> - Update the comment in irqbetter().
>>> - Extract gicv3_get_priority() to avoid code repeat.
>>> ---
>>> v3:
>>> - Add missing brace
>>> ---
>>> hw/intc/arm_gicv3.c | 71 ++++++++++++++++++++++++++++++++++++++++-----
>>> 1 file changed, 63 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
>>> index 0b8f79a122..1d16a53b23 100644
>>> --- a/hw/intc/arm_gicv3.c
>>> +++ b/hw/intc/arm_gicv3.c
>>> @@ -21,7 +21,8 @@
>>> #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 has_superprio)
>>> {
>>> /* Return true if this IRQ at this priority should take
>>> * precedence over the current recorded highest priority
>>> @@ -33,11 +34,24 @@ static bool irqbetter(GICv3CPUState *cs, int irq,
>>> uint8_t prio)
>>> if (prio < cs->hppi.prio) {
>>> return true;
>>> }
>>> +
>>> + /*
>>> + * Current highest prioirity pending interrupt is an IRQ without
>>> + * superpriority, the new IRQ with superpriority has same priority
>>> + * should signal to the CPU as it have the priority higher than
>>> + * the labelled 0x80 or 0x00.
>>> + */
>>> + if (prio == cs->hppi.prio && !cs->hppi.superprio && has_superprio) {
>>> + 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.
>>> + * signal the one with the lowest interrupt number if they don't
>>> have
>>> + * superpriority.
>>> */
>>> - if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
>>> + if (prio == cs->hppi.prio && !cs->hppi.superprio &&
>>> + !has_superprio && irq <= cs->hppi.irq) {
>>> return true;
>>> }
>>> return false;
>>> @@ -129,6 +143,35 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
>>> return pend;
>>> }
>>> +static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
>>> + uint32_t superprio, uint8_t *prio, int
>>> irq)
>>> +{
>>> + bool has_superprio = false;
>>> +
>>> + if (superprio) {
>>> + has_superprio = true;
>>> +
>>> + /* DS = 0 & Non-secure NMI */
>>> + if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
>>> + ((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
>>> + (!is_redist && gicv3_gicd_group_test(cs->gic, irq)))) {
>>> + *prio = 0x80;
>>> + } else {
>>> + *prio = 0x0;
>>> + }
>>> + } else {
>>> + has_superprio = false;
>>> +
>>> + if (is_redist) {
>>> + *prio = cs->gicr_ipriorityr[irq];
>>> + } else {
>>> + *prio = cs->gic->gicd_ipriority[irq];
>>> + }
>>> + }
>>> +
>>> + return has_superprio;
>>> +}
>>
>> Did you not like the idea to map {priority, !superpriority} into a
>> single value?
>>
>> It would eliminate the change in irqbetter(), which is a bit more
>> complex than it needs to be.
>
> I will try to change to implement this mapping scheme.
I try to implement this mapping scheme, but it seems more complex, more
change on a larger scale, although it would eliminate the change in
irqbetter(). The subpriority and group priority, eight-bit segmentation
is used, the same is true of the semantics of BPR. And currently, the
invalid values or initial value of all priorities in the QEMU are 0xff,
and all priority mask operations are 0xff.
Especially, what should hppi.prio save? the original priority value or
the mapped priority value. If hppi.prio save the mapped priority value,
all of the above things need to be updated. Otherwise,if hppi.prio save
the original priority value，it will be more simpler, but it also need
to convert from mapped priority to original priority when save
cs->hppi.prio and convert to mapped priority when comparing priority in
irqbetter(), it is also not more compact than existing implementation.
@Peter
>
>>
>>> @@ -152,10 +197,13 @@ 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);
>>> + has_superprio = gicv3_get_priority(cs, true, superprio,
>>> &prio, i);
>>
>> It would allow moving the read of gicr_isuperprio into
>> gicv3_get_priority(), alongside the read of gicr_ipriorityr.
>>
>> Is there a bug here not handling is_redist for GCIR_INMI*?
>>
>>> @@ -168,7 +216,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)) {
>>
>> Always passing false here is incorrect -- again missing the
>> redistributor nmi bit?
>>
>>
>> r~
© 2016 - 2024 Red Hat, Inc.