From patchwork Fri Nov 10 08:20:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 10052549 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5EF90603FA for ; Fri, 10 Nov 2017 08:21:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4DB582B24A for ; Fri, 10 Nov 2017 08:21:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 42A1D2B253; Fri, 10 Nov 2017 08:21:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9082E2B24A for ; Fri, 10 Nov 2017 08:21:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751117AbdKJIUl (ORCPT ); Fri, 10 Nov 2017 03:20:41 -0500 Received: from mail-wr0-f194.google.com ([209.85.128.194]:52885 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750866AbdKJIUk (ORCPT ); Fri, 10 Nov 2017 03:20:40 -0500 Received: by mail-wr0-f194.google.com with SMTP id j23so7864174wra.9 for ; Fri, 10 Nov 2017 00:20:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mI+fHc1CeIK85D0L/S5f/cYddEsjACz1feYw5Zra6DQ=; b=F0N35NrMcEoqokWxMrs/Z4TgxSLTL88+wZHIFT6f2U5aRHGwtFua6rTrTSIMGQ8lYI lNF7sgzW+QXna4C04hJyO5krSnanRtY49hRDn3zYfwKaVKe5Y4nKvB+OaYXTOnwVMKZ/ twzd6etiwExM5KlJWrvwCYw1QklKyKgUqnNHI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mI+fHc1CeIK85D0L/S5f/cYddEsjACz1feYw5Zra6DQ=; b=DdlWCRzvV4xXuZsLcauMILJsOIrxHv65K8KOCEPDKNAqrjCW683L4Nq/yTct4BCed2 rePwAEodIPb9MNZdocCi4o7o3n1QknXHlwN5ZYz3hIZ4RClfYrOIyhPSmyl0PaasbJ6w 5jPpk4vsErEI+mCDI7jIR7pT5cDH1xaqbCuJ8TdZPgCCUwBzOMXnEHRos/x3qoVpRE/q qanT/cyGdzrh0K0+/5ZV3O0FMJHFb6Z5NW1trSkFrWEOVzRhxcL98/Kx7Cfbb2r8FMqT uyw9L8vqi9ttdmUPGCWhDpdzoZCiremAg+cCGz2OMsbrpnGciHIFayt93DNdjTVVG2L5 vFGA== X-Gm-Message-State: AJaThX7X2Cj1A3h30Zf/PLKwk4w74MgcDpocrAUlLbCrpLf1DfaAhrVQ O37r8aWSxMdoshdauwh5tpcdZw== X-Google-Smtp-Source: ABhQp+T5Uc9m62dV8rnGQkufyAK/dBeKUM3TujhFYBRx6DbmcL5GslXsh9UrsbtBRjanXKmu/35bSQ== X-Received: by 10.223.183.13 with SMTP id l13mr2831663wre.1.1510302038703; Fri, 10 Nov 2017 00:20:38 -0800 (PST) Received: from localhost (xd93dd96b.cust.hiper.dk. [217.61.217.107]) by smtp.gmail.com with ESMTPSA id v5sm536553wme.26.2017.11.10.00.20.37 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Fri, 10 Nov 2017 00:20:37 -0800 (PST) Date: Fri, 10 Nov 2017 09:20:43 +0100 From: Christoffer Dall To: Auger Eric Cc: Marc Zyngier , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Christoffer Dall , Shanker Donthineni , Mark Rutland , Shameerali Kolothum Thodi , Andre Przywara Subject: Re: [PATCH v5 09/26] KVM: arm/arm64: GICv4: Add init/teardown of the per-VM vPE irq domain Message-ID: <20171110082043.GG14144@cbox> References: <20171027142855.21584-1-marc.zyngier@arm.com> <20171027142855.21584-10-marc.zyngier@arm.com> <4a41a67d-a04b-b3ca-a5e8-c582e27c17d1@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4a41a67d-a04b-b3ca-a5e8-c582e27c17d1@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Nov 07, 2017 at 02:08:23PM +0100, Auger Eric wrote: > Hi Marc, > > On 27/10/2017 16:28, Marc Zyngier wrote: > > In order to control the GICv4 view of virtual CPUs, we rely > > on an irqdomain allocated for that purpose. Let's add a couple > > of helpers to that effect. > > > > At the same time, the vgic data structures gain new fields to > > track all this... erm... wonderful stuff. > > > > The way we hook into the vgic init is slightly convoluted. We > > need the vgic to be initialized (in order to guarantee that > > the number of vcpus is now fixed), and we must have a vITS > > (otherwise this is all very pointless). So we end-up calling > > the init from both vgic_init and vgic_its_create. > > > > Reviewed-by: Christoffer Dall > > Signed-off-by: Marc Zyngier > > --- > > arch/arm/kvm/Makefile | 1 + > > arch/arm64/kvm/Makefile | 1 + > > include/kvm/arm_vgic.h | 19 ++++++++++ > > virt/kvm/arm/vgic/vgic-init.c | 9 +++++ > > virt/kvm/arm/vgic/vgic-its.c | 8 +++++ > > virt/kvm/arm/vgic/vgic-v4.c | 83 +++++++++++++++++++++++++++++++++++++++++++ > > virt/kvm/arm/vgic/vgic.h | 2 ++ > > 7 files changed, 123 insertions(+) > > create mode 100644 virt/kvm/arm/vgic/vgic-v4.c > > > > diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile > > index d9beee652d36..0a1dd2cdb928 100644 > > --- a/arch/arm/kvm/Makefile > > +++ b/arch/arm/kvm/Makefile > > @@ -31,6 +31,7 @@ obj-y += $(KVM)/arm/vgic/vgic-init.o > > obj-y += $(KVM)/arm/vgic/vgic-irqfd.o > > obj-y += $(KVM)/arm/vgic/vgic-v2.o > > obj-y += $(KVM)/arm/vgic/vgic-v3.o > > +obj-y += $(KVM)/arm/vgic/vgic-v4.o > > obj-y += $(KVM)/arm/vgic/vgic-mmio.o > > obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o > > obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o > > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile > > index 5d9810086c25..c30fd388ef80 100644 > > --- a/arch/arm64/kvm/Makefile > > +++ b/arch/arm64/kvm/Makefile > > @@ -26,6 +26,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-init.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-irqfd.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-v2.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-v3.o > > +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-v4.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > > index ba9fb450aa1b..7eeb6c2a2f9c 100644 > > --- a/include/kvm/arm_vgic.h > > +++ b/include/kvm/arm_vgic.h > > @@ -26,6 +26,8 @@ > > #include > > #include > > > > +#include > > + > > #define VGIC_V3_MAX_CPUS 255 > > #define VGIC_V2_MAX_CPUS 8 > > #define VGIC_NR_IRQS_LEGACY 256 > > @@ -236,6 +238,15 @@ struct vgic_dist { > > > > /* used by vgic-debug */ > > struct vgic_state_iter *iter; > > + > > + /* > > + * GICv4 ITS per-VM data, containing the IRQ domain, the VPE > > + * array, the property table pointer as well as allocation > > + * data. This essentially ties the Linux IRQ core and ITS > > + * together, and avoids leaking KVM's data structures anywhere > > + * else. > > + */ > > + struct its_vm its_vm; > > }; > > > > struct vgic_v2_cpu_if { > > @@ -254,6 +265,14 @@ struct vgic_v3_cpu_if { > > u32 vgic_ap0r[4]; > > u32 vgic_ap1r[4]; > > u64 vgic_lr[VGIC_V3_MAX_LRS]; > > + > > + /* > > + * GICv4 ITS per-VPE data, containing the doorbell IRQ, the > > + * pending table pointer, the its_vm pointer and a few other > > + * HW specific things. As for the its_vm structure, this is > > + * linking the Linux IRQ subsystem and the ITS together. > > + */ > > + struct its_vpe its_vpe; > > }; > > > > struct vgic_cpu { > > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c > > index 5801261f3add..40be908da238 100644 > > --- a/virt/kvm/arm/vgic/vgic-init.c > > +++ b/virt/kvm/arm/vgic/vgic-init.c > > @@ -285,6 +285,12 @@ int vgic_init(struct kvm *kvm) > > if (ret) > > goto out; > > > > + if (vgic_supports_direct_msis(kvm)) { > > + ret = vgic_v4_init(kvm); > > + if (ret) > > + goto out; > > + } > > + > > kvm_for_each_vcpu(i, vcpu, kvm) > > kvm_vgic_vcpu_enable(vcpu); > > > > @@ -320,6 +326,9 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm) > > > > kfree(dist->spis); > > dist->nr_spis = 0; > > + > > + if (vgic_supports_direct_msis(kvm)) > > + vgic_v4_teardown(kvm); > > } > > > > void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > > index 8ee03f1e89fc..89768d2b6a91 100644 > > --- a/virt/kvm/arm/vgic/vgic-its.c > > +++ b/virt/kvm/arm/vgic/vgic-its.c > > @@ -1603,6 +1603,14 @@ static int vgic_its_create(struct kvm_device *dev, u32 type) > > if (!its) > > return -ENOMEM; > > > > + if (vgic_initialized(dev->kvm)) { > Don't we need to test vgic_supports_direct_msis() on this path too? > Seems to me that we should, otherwise creating an ITS after the VGIC has been initialized would fail on non-GICv4 compatible systems, right? How about this patch as a follow-up to the series: commit 48ec1662d0f10d6468907cdc7e12c46ca1ef497c (HEAD -> next-gicv4) Author: Christoffer Dall Date: Fri Nov 10 09:16:23 2017 +0100 KVM: arm/arm64: Fix GICv4 ITS initialization issues We should only try to initialize GICv4 data structures on a GICv4 capable system. Move the vgic_supports_direct_msis() check inito vgic_v4_init() so that any KVM VGIC initialization path does not fail on non-GICv4 systems. Also be slightly more strict in the checking of the return value in vgic_its_create, and only error out on negative return values from the vgic_v4_init() function. This is important because the kvm device code only treats negative values as errors and only cleans up in this case. Errornously treating a positive return value as an error from the vgic_v4_init() function can lead to NULL pointer dereferences, as has recently been observed. Signed-off-by: Christoffer Dall Acked-by: Marc Zyngier diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c index 40be908da238..62310122ee78 100644 --- a/virt/kvm/arm/vgic/vgic-init.c +++ b/virt/kvm/arm/vgic/vgic-init.c @@ -285,11 +285,9 @@ int vgic_init(struct kvm *kvm) if (ret) goto out; - if (vgic_supports_direct_msis(kvm)) { - ret = vgic_v4_init(kvm); - if (ret) - goto out; - } + ret = vgic_v4_init(kvm); + if (ret) + goto out; kvm_for_each_vcpu(i, vcpu, kvm) kvm_vgic_vcpu_enable(vcpu); diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c index b8c1b724ba3e..c93ecd4a903b 100644 --- a/virt/kvm/arm/vgic/vgic-its.c +++ b/virt/kvm/arm/vgic/vgic-its.c @@ -1673,7 +1673,7 @@ static int vgic_its_create(struct kvm_device *dev, u32 type) if (vgic_initialized(dev->kvm)) { int ret = vgic_v4_init(dev->kvm); - if (ret) { + if (ret < 0) { kfree(its); return ret; } diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c index e367d65a0ebe..bb7e31fcee35 100644 --- a/virt/kvm/arm/vgic/vgic-v4.c +++ b/virt/kvm/arm/vgic/vgic-v4.c @@ -118,6 +118,9 @@ int vgic_v4_init(struct kvm *kvm) struct kvm_vcpu *vcpu; int i, nr_vcpus, ret; + if (!vgic_supports_direct_msis(kvm)) + return 0; /* Nothing to see here... move along. */ + if (dist->its_vm.vpes) return 0;