From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD178C56201 for ; Thu, 12 Nov 2020 15:39:44 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5BF5E20A8B for ; Thu, 12 Nov 2020 15:39:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="FMnTgoZd"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="uGtUXtFj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5BF5E20A8B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kXYaOS9W9enmE3ZYJPUcbhrhXo2syZZDkVvIqjjVkyc=; b=FMnTgoZdV0kNB+UsC+1sYh08s sLQO2RuHyM05awjTrx4gqn98AGg6ELAkc6kAHwlC6BFVJ4+Gd3Hpi/WAA8xv63ziS8G7WwrMyY6pC L2neozdOTY9igw/6UbvOwgIv0AV3as4CDc7mM//XO1cD4AhdIKyYfyTp+tW6a5bl9Z7d5mn2V/0AQ 8TD8BhW+trZp7/sAYDO4I1lt2CQqvBsMwi3YSiCZxvBattAHEOL2PrhPI2mWxKh2G0HnlsfyDSuP4 /zd7mcmxDfSZ+2wLnbMkr/B9BqUmWKkX5iqR4LsGuDdoB+tee0qMI0dzqhomIMStlWNxgDLNOg1l2 gJIHt9dpw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kdEgz-0005Qd-Ho; Thu, 12 Nov 2020 15:39:21 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kdEgw-0005Q1-RN for linux-arm-kernel@lists.infradead.org; Thu, 12 Nov 2020 15:39:19 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 818D720A8B; Thu, 12 Nov 2020 15:39:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1605195556; bh=h+PXJILPK3vWtJPVkd80+1udd6Z5oz5CIpL0CVltcDg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=uGtUXtFjb66UIqvp2M3Dd0/n0rBy/pcLNp1b92MLtoAho6dYvII5idSdJrsPtiFap Ug/86RNyybZBlHH/tdyY4TAC/F7WAHL3i6g5+J7F+uP8bgw0r8i8krROOsApxl3BtY syi4v67/7vMQwpNmKkM0gWcYJWvlSgbSGyjyb4uo= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1kdEgs-00A5LN-C0; Thu, 12 Nov 2020 15:39:14 +0000 MIME-Version: 1.0 Date: Thu, 12 Nov 2020 15:39:14 +0000 From: Marc Zyngier To: Lorenzo Pieralisi Subject: Re: [PATCH 2/2] irqchip/gic-v3-its: Disable vSGI upon (CPUIF < v4.1) detection In-Reply-To: <20201112144031.GA24454@e121166-lin.cambridge.arm.com> References: <20201111162841.3151-1-lorenzo.pieralisi@arm.com> <20201111162841.3151-3-lorenzo.pieralisi@arm.com> <20201112144031.GA24454@e121166-lin.cambridge.arm.com> User-Agent: Roundcube Webmail/1.4.9 Message-ID: <9c7853fa7ad331f768950e45d0bb5ef4@kernel.org> X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: lorenzo.pieralisi@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201112_103919_084035_4DE3EC17 X-CRM114-Status: GOOD ( 34.77 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, LAKML Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2020-11-12 14:40, Lorenzo Pieralisi wrote: > On Thu, Nov 12, 2020 at 09:36:10AM +0000, Marc Zyngier wrote: >> Hi Lorenzo, >> >> On 2020-11-11 16:28, Lorenzo Pieralisi wrote: >> > GIC CPU interfaces versions predating GIC v4.1 were not built to >> > accommodate vINTID within the vSGI range; as reported in the GIC >> > specifications (8.2 "Changes to the CPU interface"), it is >> > CONSTRAINED UNPREDICTABLE to deliver a vSGI to a PE with >> > ID_AA64PFR0_EL1.GIC == b0001. >> >> Hmmm. This goes against the very reason v4.1 was designed the way >> it is, which was that all existing implementation supporting GICv4.0 >> would seamlessly let virtual SGIs in, and it would "just work". >> >> If we start enforcing this, I question the very design of the >> architecture, >> because we could have done so much better by changing the CPU >> interface. >> >> What has changed in two years? Have you spotted a fundamental problem? > > Hi Marc, > > long story short: there are systems being designed with this > configuration, vSGIs may or may not work on them, to prevent > *potential* misbehaviour I am disabling vSGIs, I am not fixing > anything, it is belt and braces. > >> My concern is that if we prevent it, we're going to end-up with quirks >> allowing it anyway, because people will realise that it actually >> works. > > We may wait and fix it *if* this breaks, I would argue though that at > that point it is not a quirk since architecturally we know that vSGIs > may not work in this configuration. I don't mind either way, as I doubt I'll see this kind of system any time soon. I'm just mildly annoyed at the missed opportunity to do something better... > >> In the meantime, to the meat of the change: >> >> > >> > Check the GIC CPUIF version through the arm64 capabilities >> > infrastructure and disable vSGIs if a CPUIF version < 4.1 is >> > detected to prevent using vSGIs on systems where they may >> > misbehave. >> > >> > Signed-off-by: Lorenzo Pieralisi >> > Cc: Marc Zyngier >> > --- >> > drivers/irqchip/irq-gic-v3-its.c | 20 +++++++++++++++++++- >> > 1 file changed, 19 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/irqchip/irq-gic-v3-its.c >> > b/drivers/irqchip/irq-gic-v3-its.c >> > index 0fec31931e11..6ed4ba60ba7e 100644 >> > --- a/drivers/irqchip/irq-gic-v3-its.c >> > +++ b/drivers/irqchip/irq-gic-v3-its.c >> > @@ -39,6 +39,20 @@ >> > >> > #include "irq-gic-common.h" >> > >> > +#ifdef CONFIG_ARM64 >> > +#include >> > + >> > +static inline bool gic_cpuif_has_vsgi(void) >> > +{ >> > + return cpus_have_const_cap(ARM64_HAS_GIC_CPUIF_VSGI); >> > +} >> > +#else >> > +static inline bool gic_cpuif_has_vsgi(void) >> > +{ >> > + return false; >> > +} >> > +#endif >> > + >> > #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0) >> > #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) >> > #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) >> > @@ -5415,7 +5429,11 @@ int __init its_init(struct fwnode_handle >> > *handle, struct rdists *rdists, >> > if (has_v4 & rdists->has_vlpis) { >> > const struct irq_domain_ops *sgi_ops; >> > >> > - if (has_v4_1) >> > + /* >> > + * Enable vSGIs only if the ITS and the >> > + * GIC CPUIF support them. >> > + */ >> > + if (has_v4_1 && gic_cpuif_has_vsgi()) >> > sgi_ops = &its_sgi_domain_ops; >> > else >> > sgi_ops = NULL; >> >> Is that enough? > > No, I obviously missed the VGIC bits built on top of > GICD_TYPER2.nASSGIcap. > >> KVM is still going to expose GICD_TYPER2.nASSGIcap, making things even >> more confusing for the guest: it will be able to select active-less >> SGIs >> via GICD_CTLR.nASSGIreq, and if I'm not mistaken, we'd still try to >> switch >> to HW-backed SGIs, leading to some *very* unpleasant things in >> gic_v4_enable_vsgis(). > > Yes (AFAICS GICD_TYPER2.nASSGIcap is not in the public specs though, > that's why I missed it while vetting architectural state that is > affecting vSGIs). You can find it in the errata to the spec (I just checked the October 2020 version). I doubt it is public though, and people have been asking for this update to be published for a while now. > I should change the logic in vgic_mmio_{uaccess}_write_v3_misc() to > handle it properly - to redefine the logic around > > kvm_vgic_global_state.has_gicv4_1 > > somehow. You probably need a separate predicate, indicating HW-baked vSGI support. Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel