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: 10052551 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 2F9E0603FA for ; Fri, 10 Nov 2017 08:21:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1BA892B253 for ; Fri, 10 Nov 2017 08:21:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 109952B24A; Fri, 10 Nov 2017 08:21:41 +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=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A0CE62B24A for ; Fri, 10 Nov 2017 08:21:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=lKaDJWoqEfe7XSfXuXA7Dd8/stD1o3m5GoqyeHNB0Mo=; b=Ne/7Lg5PRxVakE 66amDhf8rWqx7TS7+DZvPP5k6qlLPIZ/Q1V7aBCYogQGm79Q22cZhJU/RIBosAAGAUa+ZwPaALSyN HzbsmWOki42R6/+LWCifG6luhDxNUFEMwFMDrWS4VrCDl6xma5aEOrfMDt2Ryi+eLqTHjB3I5vMe/ Jv8e6JQE5W/gAsMK3+7AW/FbgLrY0IBjySkwd/pvRPdzegAk3eAumfmoQ8HNflsU/c9gDnnpBD5f3 dhgxDyNsVnDFWRP9p9CDZR64ZqGmbHsNrdZcI+6RIYJVL8SNByUdX9ZQ2flwr3QpVgvhhmMVpQz11 FOJjDveO7sdGES2yUjVw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eD4ZG-0007zr-VX; Fri, 10 Nov 2017 08:21:39 +0000 Received: from mail-wr0-x242.google.com ([2a00:1450:400c:c0c::242]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1eD4Yf-0007A9-EX for linux-arm-kernel@lists.infradead.org; Fri, 10 Nov 2017 08:21:11 +0000 Received: by mail-wr0-x242.google.com with SMTP id 4so7886157wrt.0 for ; Fri, 10 Nov 2017 00:20:40 -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=SGC2WQlntF7frzFOgPSABOxTy0U/LLRqIcPbZVghhmO1NpTC6WOx9RmrjoMQY1jLAf u5ZD49b66/LtIC0Vp2PO+YTJSQrEDYrKNnFJBNddRI1eyXDRjbt105cFFZLUj7tuzuOo Ry82Y8La2dHqaZeZtNw96uhv8Uee2euPLa5teY7jwP4QG5fVNakyqDacr0jCPqQ5cGHu B+X8Nbb99emSJv7NfRMBvbfO+Qmv+1I3eQ/F8U302Ax7Ely9lpxf3pOXUsXAC0CMhQcr lFzRxIa0rIWh/K6L8Y4UoZYBx9c+pkqD/W8OD/HN11h2MTZVpMpvbvzLmXx2FWOEWHDt /kcw== X-Gm-Message-State: AJaThX6GAqaGFHOh1QRiZjK8KdIKVVq0D0erNk7/lPzXl5m6eeG0jstG lPiEID3KmbjcYoqQyGVOpXeKsA== 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 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) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171110_002101_923522_091A44D5 X-CRM114-Status: GOOD ( 31.43 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , kvm@vger.kernel.org, Marc Zyngier , Andre Przywara , linux-kernel@vger.kernel.org, Shameerali Kolothum Thodi , Shanker Donthineni , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, Christoffer Dall Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.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;