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

[Pavel Fedin <p.fedin@samsung.com>]

 Hello!

> It's a bit weird to just sned this as a new patch without replying to my
> mail from yesterday with feedback

 Sorry. But changes are actually minimal, and i remember that i replied to you with the promise of
testing your suggestion. So, done, works fine. :)

> I think you're missing a potential change to the irq_pending_on_cpu
> field here, which you have to compute by calling vgic_update_state()
> like we do elsewhere when we change status bits

 I have just checked this. vgic_update_state() never resets this bit. This bit is reset only in
__kvm_vgic_flush_hwstate() and only if we have consumed completely everything. I have followed
through the code and looks like it's perfectly safe to have this bit set while nothing is actually
pendng. Following __kvm_vgic_flush_hwstate(), having this bit cleared is actually a shorthand for
"no interrupt is pending at all". If it is set without any interrupt actually being pending (this
ends up in pa_percpu and pa_shared being all zeroes), all three for_each_set_bit() loops will just
not do anything, and we still get to "epilog:" label, just after a bit longer check. And, since we
are here, the guest has already been disturbed.

> different from the incorrect approach I suggested yesterday where we
> always just clear the bit for that vcpu).

 Yes, it is extremely bad idea to clear it because this bit summarizes all interrupts for this vcpu,
and clearing it means that we are going to lose everything.
 An alternate would be: clear the bit, THEN call vgic_update_state() which would set it back if
necessary. But does this extra bit of complexity worth anything, given one paragraph above?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia