On Fri, 11 Jun 2021 at 00:39, Shashi Mallela <shashi.mall...@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.mayd...@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
