From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S937874AbdJQWfF (ORCPT ); Tue, 17 Oct 2017 18:35:05 -0400 Received: from mail-wm0-f46.google.com ([74.125.82.46]:53080 "EHLO mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935130AbdJQWfA (ORCPT ); Tue, 17 Oct 2017 18:35:00 -0400 X-Google-Smtp-Source: AOwi7QBoZ3/RQIeHL13ueITvdIPlmErUmZqwwOPANf7oCXjZLLxLAesraUCaQHyRrLjzDXxcBTWBvg== Date: Wed, 18 Oct 2017 00:34:56 +0200 From: Christoffer Dall To: Eric Auger Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com, peter.maydell@linaro.org, andre.przywara@arm.com, wanghaibin.wang@huawei.com, wu.wubin@huawei.com, drjones@redhat.com, wei@redhat.com Subject: Re: [PATCH v4 09/11] KVM: arm/arm64: vgic-its: free caches when GITS_BASER Valid bit is cleared Message-ID: <20171017223456.GI8326@lvm> References: <1508224209-15366-1-git-send-email-eric.auger@redhat.com> <1508224209-15366-10-git-send-email-eric.auger@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1508224209-15366-10-git-send-email-eric.auger@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 17, 2017 at 09:10:07AM +0200, Eric Auger wrote: > When the GITS_BASER.Valid gets cleared, the data structures in > guest RAM are not valid anymore. The device, collection > 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 > > --- > > v2 -> v3: > - add a comment and clear cache in if block > --- > 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 f3f0026f..084239c 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; > + > + /* Table no longer valid: clear cached data */ > + if (!(reg & GITS_BASER_VALID)) { > + 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; > + } > + } So we do this after setting the *regptr, which makes we worried about races. How are guest writes to these registers synchronized with, for example trying to save the tables. Perhaps we don't care because userspace should have stopped the VM before trying to save the ITS state? > } > > static unsigned long vgic_mmio_read_its_ctlr(struct kvm *vcpu, > -- > 2.5.5 > Otherwise looks good to me. -Christoffer