On Wed, Nov 21, 2012 at 1:15 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> As SGIs and PPIs are treated as edge-triggered, they must be cleared
> as soon as they are queued into the list registers. Otherwise, we end-up
> with spurious interrupts.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm/kvm/vgic.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
> index 581faf2..b83de57 100644
> --- a/arch/arm/kvm/vgic.c
> +++ b/arch/arm/kvm/vgic.c
> @@ -797,8 +797,10 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
> clear_bit(c, &sources);
> }
>
> - if (!sources)
> + if (!sources) {
> clear_bit(i, pending);
> + kvm_vgic_vcpu_clear_pending_irq(vcpu, i);
> + }
the (!sources) happens _only_ if we were able to queue all of them
onto LRs right?
I might just add a comment to say that, since I get bogged up on that
condition every time I read that code.
I'll apply this one now, since it's a fix, but I'm thinking when
reading this that part of why the code is so confusing (at least to
me) is that I'm always in doubt if we're clearing/setting interrupts
on the interface side or at the input to the gic itself. This relates
to my comment in the following patch about a static wrapper, so maybe
we could come up with some naming to make things more clear here,
like:
vgic_dist_irq_set
vgic_dist_irq_clear
vgic_cpu_irq_set
vgic_cpu_irq_clear
I don't know if it helps a lot...?
>
> dist->irq_sgi_sources[vcpu_id][i] = sources;
> }
> @@ -811,6 +813,7 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
> }
>
> clear_bit(i, pending);
> + kvm_vgic_vcpu_clear_pending_irq(vcpu, i);
> }
>
>
> --
> 1.7.12
>
thanks!
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
