[PATCH v5 21/22] KVM: arm64: vgic-its: Fix pending table sync

[Christoffer Dall]

On Fri, Apr 14, 2017 at 12:15:33PM +0200, Eric Auger wrote:
> In its_sync_lpi_pending_table() we currently ignore the
> target_vcpu of the LPIs. We sync the pending bit found in
> the vcpu pending table even if the LPI is not targeting it.
> 
> Also in vgic_its_cmd_handle_invall() we are supposed to
> read the config table data for the LPIs associated to the
> collection ID. At the moment we refresh all LPI config
> information.
> 
> This patch passes a vpcu to vgic_copy_lpi_list() so that
> this latter returns a snapshot of the LPIs targeting this
> CPU and only those.
> 
> Signed-off-by: Eric Auger <eric.auger at redhat.com>
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 86dfc6c..be848be 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -252,13 +252,13 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
>  }
>  
>  /*
> - * Create a snapshot of the current LPI list, so that we can enumerate all
> - * LPIs without holding any lock.
> - * Returns the array length and puts the kmalloc'ed array into intid_ptr.
> + * Create a snapshot of the current LPIs targeting @vcpu, so that we can
> + * enumerate those LPIs without holding any lock.
> + * Returns their number and puts the kmalloc'ed array into intid_ptr.
>   */
> -static int vgic_copy_lpi_list(struct kvm *kvm, u32 **intid_ptr)
> +static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  {
> -	struct vgic_dist *dist = &kvm->arch.vgic;
> +	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>  	struct vgic_irq *irq;
>  	u32 *intids;
>  	int irq_count = dist->lpi_list_count, i = 0;
> @@ -277,14 +277,14 @@ static int vgic_copy_lpi_list(struct kvm *kvm, u32 **intid_ptr)
>  	spin_lock(&dist->lpi_list_lock);
>  	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
>  		/* We don't need to "get" the IRQ, as we hold the list lock. */
> -		intids[i] = irq->intid;
> -		if (++i == irq_count)
> -			break;
> +		if (irq->target_vcpu != vcpu)
> +			continue;
> +		intids[i++] = irq->intid;

were we checking the ++i == irq_count condition for no good reason
before since we can just drop it now?

>  	}
>  	spin_unlock(&dist->lpi_list_lock);
>  
>  	*intid_ptr = intids;
> -	return irq_count;
> +	return i;
>  }
>  
>  /*
> @@ -333,7 +333,7 @@ static u32 max_lpis_propbaser(u64 propbaser)
>  }
>  
>  /*
> - * Scan the whole LPI pending table and sync the pending bit in there
> + * Sync the pending table pending bit of LPIs targeting @vcpu
>   * with our own data structures. This relies on the LPI being
>   * mapped before.
>   */
> @@ -346,7 +346,7 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu *vcpu)
>  	u32 *intids;
>  	int nr_irqs, i;
>  
> -	nr_irqs = vgic_copy_lpi_list(vcpu->kvm, &intids);
> +	nr_irqs = vgic_copy_lpi_list(vcpu, &intids);
>  	if (nr_irqs < 0)
>  		return nr_irqs;
>  
> @@ -1027,7 +1027,7 @@ static int vgic_its_cmd_handle_invall(struct kvm *kvm, struct vgic_its *its,
>  
>  	vcpu = kvm_get_vcpu(kvm, collection->target_addr);
>  
> -	irq_count = vgic_copy_lpi_list(kvm, &intids);
> +	irq_count = vgic_copy_lpi_list(vcpu, &intids);
>  	if (irq_count < 0)
>  		return irq_count;
>  
> -- 
> 2.5.5
> 

Assuming that it's ok to remove the irq_count check above, the rest of
this patch looks good to me.

Thanks,
-Christoffer