Hi,
On 30/04/2017 23:10, Christoffer Dall wrote:
> 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@xxxxxxxxxx>
>> ---
>> 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?
I didn't get why we had that check.
Thanks
Eric
>
>> }
>> 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
>
