From mboxrd@z Thu Jan 1 00:00:00 1970 From: Auger Eric Subject: Re: [PATCH v3 16/19] KVM: arm64: ITS: vgic_its_check_id returns the entry's GPA Date: Wed, 22 Mar 2017 15:22:48 +0100 Message-ID: <393b3bef-07b6-146c-823e-fcf2541c921f@redhat.com> References: <1488800074-21991-1-git-send-email-eric.auger@redhat.com> <1488800074-21991-17-git-send-email-eric.auger@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Prasun.Kapoor@cavium.com, quintela@redhat.com, dgilbert@redhat.com, pbonzini@redhat.com To: Andre Przywara , eric.auger.pro@gmail.com, marc.zyngier@arm.com, christoffer.dall@linaro.org, vijayak@caviumnetworks.com, Vijaya.Kumar@cavium.com, peter.maydell@linaro.org, linux-arm-kernel@lists.infradead.org, drjones@redhat.com, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.vger.kernel.org Hi Andre, On 21/03/2017 19:12, Andre Przywara wrote: > Hi, > > On 06/03/17 11:34, Eric Auger wrote: >> As vgic_its_check_id() computes the device/collection entry's >> GPA, let's return it so that new callers can retrieve it easily. >> >> Signed-off-by: Eric Auger >> >> --- >> >> v2: new >> --- >> virt/kvm/arm/vgic/vgic-its.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c >> index ad67759..1060125 100644 >> --- a/virt/kvm/arm/vgic/vgic-its.c >> +++ b/virt/kvm/arm/vgic/vgic-its.c >> @@ -637,7 +637,8 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its, >> * is actually valid (covered by a memslot and guest accessible). >> * For this we have to read the respective first level entry. >> */ >> -static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id) >> +static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id, >> + gpa_t *eaddr) >> { >> int l1_tbl_size = GITS_BASER_NR_PAGES(baser) * SZ_64K; >> int index; >> @@ -657,6 +658,7 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id) >> addr = BASER_ADDRESS(baser) + id * esz; >> gfn = addr >> PAGE_SHIFT; >> >> + *eaddr = addr; > > If you check that eaddr is not NULL before writing to it, you can save > the dummy variable in the two callers below (which doesn't hurt, but > does not help readability either). Done Thanks Eric > > Cheers, > Andre. > >> return kvm_is_visible_gfn(its->dev->kvm, gfn); >> } >> >> @@ -689,6 +691,7 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id) >> indirect_ptr += index * esz; >> gfn = indirect_ptr >> PAGE_SHIFT; >> >> + *eaddr = indirect_ptr; >> return kvm_is_visible_gfn(its->dev->kvm, gfn); >> } >> >> @@ -697,8 +700,9 @@ static int vgic_its_alloc_collection(struct vgic_its *its, >> u32 coll_id) >> { >> struct its_collection *collection; >> + gpa_t eaddr; >> >> - if (!vgic_its_check_id(its, its->baser_coll_table, coll_id)) >> + if (!vgic_its_check_id(its, its->baser_coll_table, coll_id, &eaddr)) >> return E_ITS_MAPC_COLLECTION_OOR; >> >> collection = kzalloc(sizeof(*collection), GFP_KERNEL); >> @@ -910,8 +914,9 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its, >> size_t size = its_cmd_get_size(its_cmd); >> gpa_t itt_addr = its_cmd_get_ittaddr(its_cmd); >> struct its_device *device; >> + gpa_t eaddr; >> >> - if (!vgic_its_check_id(its, its->baser_device_table, device_id)) >> + if (!vgic_its_check_id(its, its->baser_device_table, device_id, &eaddr)) >> return E_ITS_MAPD_DEVICE_OOR; >> >> if (valid && size > VITS_TYPER_IDBITS) >>