Re: [PATCH v4 6/8] hw/intc: GICv3 redistributor ITS processing

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 11 Jun 2021 at 00:39, Shashi Mallela <shashi.mallela@linaro.org> wrote:
>
> Have addressed all comments except the ones with responses(inline) below:-
>
> On Jun 8 2021, at 9:57 am, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > + cs->lpivalid = false;
> > + cs->hpplpi.prio = 0xff;
> > + gicv3_redist_update_lpi(cs);
>
> You can avoid doing a full update a lot of the time:
> * if this LPI is worse than the current value in hpplpi
> (where by "worse" I mean lower-priority by the same kind of
> comparison irqbetter() does) then we haven't changed the best-available
> pending LPI, so we don't need to do an update
> * if we set the pending bit to 1 and the LPI is enabled and the priority
> of this LPI is better than the current hpplpi, then we know this LPI
> is now the best, so we can just set hpplpi.prio and .irq without
> doing a full rescan
> * if we didn't actually change the value of the pending bit, we
> don't need to do an update (you get this for free if you take the
> simplification suggestion I make above, which does an early-return
> in the "no change" case)
>
> > Accepted the code simplification,but by not calling gicv3_redist_update_lpi(cs) here ,the scenario of an activated LPI fails because
> this LPI's priority (which could be lower than current hpplpi) is never checked/updated and gicv3_redist_update_noirqset() fails to present a valid active high priority LPI(if applicable) to the cpu,since it is always checking against a stale hpplpi info.

If the LPI is lower priority (higher number) than the current
hpplpi then it would not change the existing hpplpi info in
a full-scan. If the LPI being activated is higher priority
(lower number) than the current hpplpi then that is my point 2 above,
and we set it as the hpplpi without needing the full-scan. And for
the other cases (eg highest-priority LPI being deactivated) we
should fall through to the default "call update_lpi" case.

So I don't really understand why this wouldn't work.

-- PMM