From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v2 08/10] KVM: arm/arm64: vgic-its: free caches when GITS_BASER Valid bit is cleared Date: Fri, 13 Oct 2017 17:19:41 +0200 Message-ID: <20171013151941.GK8927@cbox> References: <1506518920-18571-1-git-send-email-eric.auger@redhat.com> <1506518920-18571-9-git-send-email-eric.auger@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, marc.zyngier@arm.com, peter.maydell@linaro.org, andre.przywara@arm.com, wanghaibin.wang@huawei.com, wu.wubin@huawei.com To: Eric Auger Return-path: Received: from mail-wm0-f47.google.com ([74.125.82.47]:46058 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758137AbdJMPTf (ORCPT ); Fri, 13 Oct 2017 11:19:35 -0400 Received: by mail-wm0-f47.google.com with SMTP id q124so22196007wmb.0 for ; Fri, 13 Oct 2017 08:19:35 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1506518920-18571-9-git-send-email-eric.auger@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Sep 27, 2017 at 03:28:38PM +0200, Eric Auger wrote: > When the GITS_BASER.Valid gets cleared, the data structures in > guest RAM are not provisionned anymore. The device, collection provisioned (but did you really mean valid?) > and LPI lists stored in the in-kernel ITS represent the same > information in some form of cache. So let's void the cache. > > Signed-off-by: Eric Auger > --- > virt/kvm/arm/vgic/vgic-its.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index 0df6d5f..eaefba2 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -1471,8 +1471,9 @@ static void vgic_mmio_write_its_baser(struct kvm *kvm, > unsigned long val) > { > const struct vgic_its_abi *abi = vgic_its_get_abi(its); > - u64 entry_size, device_type; > + u64 entry_size; > u64 reg, *regptr, clearbits = 0; > + int type; > > /* When GITS_CTLR.Enable is 1, we ignore write accesses. */ > if (its->enabled) > @@ -1482,12 +1483,12 @@ static void vgic_mmio_write_its_baser(struct kvm *kvm, > case 0: > regptr = &its->baser_device_table; > entry_size = abi->dte_esz; > - device_type = GITS_BASER_TYPE_DEVICE; > + type = GITS_BASER_TYPE_DEVICE; > break; > case 1: > regptr = &its->baser_coll_table; > entry_size = abi->cte_esz; > - device_type = GITS_BASER_TYPE_COLLECTION; > + type = GITS_BASER_TYPE_COLLECTION; > clearbits = GITS_BASER_INDIRECT; > break; > default: > @@ -1499,10 +1500,24 @@ static void vgic_mmio_write_its_baser(struct kvm *kvm, > reg &= ~clearbits; > > reg |= (entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT; > - reg |= device_type << GITS_BASER_TYPE_SHIFT; > + reg |= (u64)type << GITS_BASER_TYPE_SHIFT; > reg = vgic_sanitise_its_baser(reg); > > *regptr = reg; > + > + if (reg & GITS_BASER_VALID) > + return; > + > + switch (type) { > + case GITS_BASER_TYPE_DEVICE: > + vgic_its_free_device_list(kvm, its); > + break; > + case GITS_BASER_TYPE_COLLECTION: > + vgic_its_free_collection_list(kvm, its); > + break; > + default: > + break; > + } This block deserves a comment: /* Table no longer valid: clear cached data */ I would also inverse the logic and do it within an if-statement if (!(reg & GITS_BASER_VALID)), but it's up to you. > } > > static unsigned long vgic_mmio_read_its_ctlr(struct kvm *vcpu, > -- > 2.5.5 > Thanks, -Christoffer