Re: [RFC PATCH v6 19/23] hw/intc/arm_gicv3: Implement NMI interrupt prioirty

[Richard Henderson]

On 3/4/24 21:03, Jinjie Ruan via wrote:
> -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,22 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t 
> prio)
>       if (prio < cs->hppi.prio) {
>           return true;
>       }
> +
> +    /*
> +     * The same priority IRQ with superpriority 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;

This should be reordered for clarity:

    if (prio != cs->hppi.prio) {
        return prio < cs->hppi.prio;
    }

    if (has_superprio != cs->hppi.superprio) {
        return has_superprio;
    }

    return irq <= cs->hppa.irq;

So that we do not have to keep incorporating previous tests.



> @@ -129,6 +141,43 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
>       return pend;
>   }
>   
> +static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
> +                               uint8_t *prio, int irq)
> +{
> +    bool has_superprio = false;
> +    uint32_t superprio = 0x0;
> +
> +    if (is_redist) {
> +        superprio = extract32(cs->gicr_isuperprio, irq, 1);
> +    } else {
> +        superprio = *gic_bmp_ptr32(cs->gic->superprio, irq);
> +        superprio = superprio & (1 << (irq & 0x1f));
> +    }
> +
> +    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;
> +}

has_superprio == superprio -- you can eliminate has_superprio,
or even leverage the code path:

    if (superprio) {
        ...
        return true;
    }

    if (is_redist)
    ...
    return false;

> @@ -168,9 +219,10 @@ 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)) {

New argument should be hpplpi.superprio.

There are several places where we're setting irq and prio which might need to clear 
superprio, e.g. update_for_one_lpi.  But also anywhere else that deals with PendingIrq.

> +        cs->hppi.superprio = 0x0;

false, not 0x0.


r~