From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 76C8B86348; Mon, 4 Aug 2025 17:51:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754329919; cv=none; b=hFAAZGtAzotyZcxjaZYlSIf445JPgspe71gY38VP6JieT09xSmuyToGJUgw8gJepkftytQjcijO18yZD96h1DhpkCqyGhg+PYsh9DjU44f7JkRMk5Havj2RUG4OiM5719Ic2gN2tDVOf7Ss1wL5ntQ5/sdKRJQnPYFntXsNjcKM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754329919; c=relaxed/simple; bh=EJzIu1QX0O6qT3aP+utXo337E9JKKXcRcl5UpjqxCMI=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=m+KUkvdyJr5pjsFjYBQ6u41enH+sbLhOvRcgI0wJs8dQFwqTOGvmOwa7d5nEFlwMkJCiX2ij0+rrBEyb+/r7YKHI2Iq8sZmroRFsu5Y4VE83U696Ie8SatNlptRPdb59iPt89rmuACeSNIVP64k6v7PKedwO9SurpIfJpGZisEg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=FW/9AF6D; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="FW/9AF6D" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BB6BCC4CEE7; Mon, 4 Aug 2025 17:51:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1754329918; bh=EJzIu1QX0O6qT3aP+utXo337E9JKKXcRcl5UpjqxCMI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=FW/9AF6DsVHy9coYZKbXRayDL4YU/gvTvAkDYcBqtuNqrnhtbn5GtKScuZQPr6Tqo a14co/OXlS5BPzIirpvVxARWKME3mOyU3xPXuVhXul3loZ/F7zXtdmtdsLrRoUFoIx z4iQ6vrn5gnlaXeb3oSPMhfVSPzTXQ8W0xszdt2JUs59DFzfiG0jBfSJELDWEapfRQ jI22tpIvSd5Vm/Wd5OgULEWoNbKeUZ8rvOc5slH82aL/xfeaLrGCdubDX+0ccRJLCN F7jKjtA7eJsKEYtgjyps2ZaAihyftdMYP3Ig8VAmulknirmSDtZCARVEV2mDWlGbxo ZfYw58glxeCSw== Received: from sofa.misterjones.org ([185.219.108.64] helo=lobster-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1uizLo-003syH-Qb; Mon, 04 Aug 2025 18:51:56 +0100 Date: Mon, 04 Aug 2025 18:51:56 +0100 Message-ID: <87cy9bt1oj.wl-maz@kernel.org> From: Marc Zyngier To: Alexandru Elisei Cc: Andre Przywara , Will Deacon , Julien Thierry , kvm@vger.kernel.org, kvmarm@lists.linux.dev Subject: Re: [PATCH kvmtool v3 3/6] arm64: nested: add support for setting maintenance IRQ In-Reply-To: References: <20250729095745.3148294-1-andre.przywara@arm.com> <20250729095745.3148294-4-andre.przywara@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: alexandru.elisei@arm.com, andre.przywara@arm.com, will@kernel.org, julien.thierry.kdev@gmail.com, kvm@vger.kernel.org, kvmarm@lists.linux.dev X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Mon, 04 Aug 2025 15:43:09 +0100, Alexandru Elisei wrote: > > Hi Andre, > > 'add' should be capitalized. > > On Tue, Jul 29, 2025 at 10:57:42AM +0100, Andre Przywara wrote: > > Uses the new VGIC KVM device attribute to set the maintenance IRQ. > > This is fixed to use PPI 9, as a platform decision made by kvmtool, > > matching the SBSA recommendation. > > > > Signed-off-by: Andre Przywara > > --- > > arm64/arm-cpu.c | 3 ++- > > arm64/gic.c | 21 ++++++++++++++++++++- > > arm64/include/kvm/gic.h | 2 +- > > 3 files changed, 23 insertions(+), 3 deletions(-) > > > > diff --git a/arm64/arm-cpu.c b/arm64/arm-cpu.c > > index 69bb2cb2c..1e456f2c6 100644 > > --- a/arm64/arm-cpu.c > > +++ b/arm64/arm-cpu.c > > @@ -14,7 +14,8 @@ static void generate_fdt_nodes(void *fdt, struct kvm *kvm) > > { > > int timer_interrupts[4] = {13, 14, 11, 10}; > > > > - gic__generate_fdt_nodes(fdt, kvm->cfg.arch.irqchip); > > + gic__generate_fdt_nodes(fdt, kvm->cfg.arch.irqchip, > > + kvm->cfg.arch.nested_virt); > > timer__generate_fdt_nodes(fdt, kvm, timer_interrupts); > > pmu__generate_fdt_nodes(fdt, kvm); > > } > > diff --git a/arm64/gic.c b/arm64/gic.c > > index b0d3a1abb..7461b0f3f 100644 > > --- a/arm64/gic.c > > +++ b/arm64/gic.c > > @@ -11,6 +11,8 @@ > > > > #define IRQCHIP_GIC 0 > > > > +#define GIC_MAINT_IRQ 9 > > + > > static int gic_fd = -1; > > static u64 gic_redists_base; > > static u64 gic_redists_size; > > @@ -302,10 +304,15 @@ static int gic__init_gic(struct kvm *kvm) > > > > int lines = irq__get_nr_allocated_lines(); > > u32 nr_irqs = ALIGN(lines, 32) + GIC_SPI_IRQ_BASE; > > + u32 maint_irq = GIC_MAINT_IRQ + 16; /* PPI */ > > There's already a define for PPIs: > > u32 maint_irq = GIC_PPI_IRQ_BASE + GIC_MAINT_IRQ; > > > struct kvm_device_attr nr_irqs_attr = { > > .group = KVM_DEV_ARM_VGIC_GRP_NR_IRQS, > > .addr = (u64)(unsigned long)&nr_irqs, > > }; > > + struct kvm_device_attr maint_irq_attr = { > > + .group = KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, > > + .addr = (u64)(unsigned long)&maint_irq, > > + }; > > struct kvm_device_attr vgic_init_attr = { > > .group = KVM_DEV_ARM_VGIC_GRP_CTRL, > > .attr = KVM_DEV_ARM_VGIC_CTRL_INIT, > > @@ -325,6 +332,13 @@ static int gic__init_gic(struct kvm *kvm) > > return ret; > > } > > > > + if (kvm->cfg.arch.nested_virt && > > + !ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &maint_irq_attr)) { > > I'm not sure how useful the HAS_DEVICE_ATTR call is here: kvm_cpu__arch_init(), > which checks for KVM_CAP_ARM_EL2 capability, is called before gic__init_gic() > (base_init() vs late_init()). So at this point we know that KVM supports nested > virtualization. I disagree. All optional features should be guarded by a check for that particular feature. If anything, this serves as validation for the kernel side (which is, let's face it, the *only* use-case for kvmtool). > Was it that KVM at some point supported nested virtualization but didn't have > the KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ device attribute implemented? And if that was > the case, do we want to support that version of KVM in kvmtool? Then make kvmtool die in this case. But the check stays. M. -- Jazz isn't dead. It just smells funny.