From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934144AbdKGVXb (ORCPT ); Tue, 7 Nov 2017 16:23:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42510 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932345AbdKGVXa (ORCPT ); Tue, 7 Nov 2017 16:23:30 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2F72EA8A3 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=eric.auger@redhat.com From: Auger Eric Subject: Re: [PATCH v5 17/26] KVM: arm/arm64: GICv4: Handle INVALL applied to a vPE To: Marc Zyngier , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20171027142855.21584-1-marc.zyngier@arm.com> <20171027142855.21584-18-marc.zyngier@arm.com> Cc: Mark Rutland , Christoffer Dall , Andre Przywara , Shameerali Kolothum Thodi , Christoffer Dall , Shanker Donthineni Message-ID: Date: Tue, 7 Nov 2017 22:23:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20171027142855.21584-18-marc.zyngier@arm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 07 Nov 2017 21:23:30 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Thanks Eric > > Acked-by: Christoffer Dall > Signed-off-by: Marc Zyngier > --- > 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; > } > >