Re: [PATCH v2] KVM: arm/arm64: BUG FIX: Do not inject spurious interrupts

[Christoffer Dall <christoffer.dall@linaro.org>]

On Fri, Oct 09, 2015 at 05:41:11PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > I reworked the commit message and applied this patch.
> 
>  During testing i discovered a problem with this patch and vITS series by Andre.
>  The problem is that compute_pending_for_cpu() does not know anything about LPIs. Therefore, we can
> reset this bit even if some LPIs (and only LPIs) are pending. This causes LPI loss.

I haven't looked at the ITS series in detail yet so I cannot commetn on
this.

>  This is the confirmation of that clearing irq_pending_on_cpu anywhere else than
> __kvm_vgic_flush_hwstate() is a bad idea. I would suggest to stick back to v1 of the patch (without
> clearing this bit). We can add a clarifying description to the commit message like this:
> 
> --- cut ---
> In some situations level-sensitive IRQ disappears before it has been
> processed. This is normal, and in this situation we lose this IRQ, the same
> as real HW does. The aim of this patch is to handle this situation more
> correctly. However, dist->irq_pending_on_cpu stays set until the vCPU
> itself rechecks its status. Therefore, this bit does not guarantee that
> something is pending at the given moment, it should be treated as attention
> flag, saying that something has happened on this vCPU, and it could have
> been even gone since that, but wakeup and status recheck is needed.
> --- cut ---

I really don't want to have an inconsistent state in our data
structures, this whole thing is plenty fragile as it is.

> 
>  Would you be happy with this? An alternative would be to add a check for pending LPIs, but wouldn't
> it just be too complex for a simple problem?
> 

My concern at this point is to try to keep this thing stable.

It is really up to whoever adds support for LPIs to make sure it's done
correctly.  So I think this is for Andre to work out in his ITS series.

This patch fixes an issue with the current code in the correct way as
far as I can tell.

Thanks,
-Christoffer