Hi Marc,
On 27/10/2017 16:28, Marc Zyngier wrote:
> Since when updating the properties one LPI at a time, there is no
Since we update the properties one LPI at a time, ... ?
> need to perform an INV each time we read one. Instead, we rely
> on the final VINVALL that gets sent to the ITS to do the work.
The commit message is not crystal clear for me.
I understand in case of vgic_its_cmd_handle_invall you want to avoid
doing an invalidation for each physical irq but rather do an
its_invall_vpe at the end. So you add a new @needs_inv arg to
update_lpi_config to tell whether the invalidation should be done or not.
Besides
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
Thanks
Eric
>
> Acked-by: Christoffer Dall <cdall@xxxxxxxxxx>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> virt/kvm/arm/vgic/vgic-its.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 2e77c7c83942..eb72eb027060 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -38,7 +38,7 @@ static int vgic_its_save_tables_v0(struct vgic_its *its);
> static int vgic_its_restore_tables_v0(struct vgic_its *its);
> static int vgic_its_commit_v0(struct vgic_its *its);
> static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
> - struct kvm_vcpu *filter_vcpu);
> + struct kvm_vcpu *filter_vcpu, bool needs_inv);
>
> /*
> * Creates a new (reference to a) struct vgic_irq for a given LPI.
> @@ -106,7 +106,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
> * However we only have those structs for mapped IRQs, so we read in
> * the respective config data from memory here upon mapping the LPI.
> */
> - ret = update_lpi_config(kvm, irq, NULL);
> + ret = update_lpi_config(kvm, irq, NULL, false);
> if (ret)
> return ERR_PTR(ret);
>
> @@ -273,7 +273,7 @@ static struct its_collection *find_collection(struct vgic_its *its, int coll_id)
> * VCPU. Unconditionally applies if filter_vcpu is NULL.
> */
> static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
> - struct kvm_vcpu *filter_vcpu)
> + struct kvm_vcpu *filter_vcpu, bool needs_inv)
> {
> u64 propbase = GICR_PROPBASER_ADDRESS(kvm->arch.vgic.propbaser);
> u8 prop;
> @@ -297,7 +297,7 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
> }
>
> if (irq->hw)
> - return its_prop_update_vlpi(irq->host_irq, prop, true);
> + return its_prop_update_vlpi(irq->host_irq, prop, needs_inv);
>
> return 0;
> }
> @@ -1096,7 +1096,7 @@ static int vgic_its_cmd_handle_inv(struct kvm *kvm, struct vgic_its *its,
> if (!ite)
> return E_ITS_INV_UNMAPPED_INTERRUPT;
>
> - return update_lpi_config(kvm, ite->irq, NULL);
> + return update_lpi_config(kvm, ite->irq, NULL, true);
> }
>
> /*
> @@ -1131,12 +1131,15 @@ static int vgic_its_cmd_handle_invall(struct kvm *kvm, struct vgic_its *its,
> irq = vgic_get_irq(kvm, NULL, intids[i]);
> if (!irq)
> continue;
> - update_lpi_config(kvm, irq, vcpu);
> + update_lpi_config(kvm, irq, vcpu, false);
> vgic_put_irq(kvm, irq);
> }
>
> kfree(intids);
>
> + if (vcpu->arch.vgic_cpu.vgic_v3.its_vpe.its_vm)
> + its_invall_vpe(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe);
> +
> return 0;
> }
>
>
