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

[Shashi Mallela <shashi.mallela@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]



On Jun 11 2021, at 4:30 am, Peter Maydell <peter.maydell@linaro.org> wrote:
> 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

Have got this working as per comments.Please ignore my last comment.

[-- Attachment #2: Type: text/html, Size: 2723 bytes --]