Hi,
On 07/03/18 12:40, Marc Zyngier wrote:
> The vgic code is trying to be clever when injecting GICv2 SGIs,
> and will happily populate LRs with the same interrupt number if
> they come from multiple vcpus (after all, they are distinct
> interrupt sources).
>
> Unfortunately, this is against the letter of the architecture,
> and the GICv2 architecture spec says "Each valid interrupt stored
> in the List registers must have a unique VirtualID for that
> virtual CPU interface.". GICv3 has similar (although slightly
> ambiguous) restrictions.
Ah, good catch. I was silently assuming that this "unique interrupt"
restriction was including the source ID, but fair enough.
> This results in guests locking up when using GICv2-on-GICv3, for
> example. The obvious fix is to stop trying so hard, and inject
> a single vcpu per SGI per guest entry. After all, pending SGIs
> with multiple source vcpus are pretty rare, and are mostly seen
> in scenario where the physical CPUs are severely overcomitted.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 0919e84c0fc1 ("KVM: arm/arm64: vgic-new: Add IRQ sync/flush framework")
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
Thanks!
Andre.
> ---
> virt/kvm/arm/vgic/vgic.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index c7c5ef190afa..1f7ff175f47b 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -720,18 +720,9 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
> list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
> spin_lock(&irq->irq_lock);
>
> - if (unlikely(vgic_target_oracle(irq) != vcpu))
> - goto next;
> -
> - /*
> - * If we get an SGI with multiple sources, try to get
> - * them in all at once.
> - */
> - do {
> + if (likely(vgic_target_oracle(irq) == vcpu))
> vgic_populate_lr(vcpu, irq, count++);
> - } while (irq->source && count < kvm_vgic_global_state.nr_lr);
>
> -next:
> spin_unlock(&irq->irq_lock);
>
> if (count == kvm_vgic_global_state.nr_lr) {
>
