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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 388D0D358EE for ; Thu, 29 Jan 2026 10:14:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=i2BjtfFKJ7EJoDj0mOpq4Vki8DNsNIjcWOld1LjOd5E=; b=VIiE57/Ly7GViIXxzJa2gO0f++ dt0MUZB2xMpXPwQO6GXjont/PPsjeIFUDF6evE8qjc0dgXlz6C857gTJPwXN69pK71cFjTo7UZmrn VK/ei9jQzrqUG7oWu7LdZdToZlGmyUDbOeaIdSMTo2mk52LX38B3Uz5s1OYOl9aY6zJM+mipi7TkC gWNkC4YTNfoJnTh/L5pMC/6Dw8YLTE5J7YyjlWoosHGgay3It/gzc8w+fgkux/xUxfu1JreUTzdE0 R8+XtupQOlYL9iHod2btLMSo+ylwkh19HBNHfI5cAOB/8+gtAD/N3l0mkNRuolCcSMXYEgE6FK8rQ 8L1zQfGA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vlP2x-0000000HZ8d-0TPl; Thu, 29 Jan 2026 10:14:43 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vlP2u-0000000HZ7n-3CMn for linux-arm-kernel@lists.infradead.org; Thu, 29 Jan 2026 10:14:42 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 43E4243D2C; Thu, 29 Jan 2026 10:14:39 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F1B84C4CEF7; Thu, 29 Jan 2026 10:14:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769681679; bh=iQvMrDyasksTllCjdfeh6A5zWB6EVZnl+Zr/KX7IDw8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=sjhhhedW3IMVefMoFUpZ9k0oX5+2HHGarbL4dqBcb8BsftFFHGFhPXGxV0ufOeSpG 1zFCx/xvfwgk99K95SmOED30KufKAAzY+NjnglJJ1lggiKUX4B4XPMoTI0QBZIu+MC lrSny8tvEcY+I+kaMNyxYPu3fCaIQSju+2ZN5FbHx/3nr0npaJhXewW4z2VHY/O3m2 tHc7aqqgSLeBoQHuhGmFefc/DJXV9Pb9FPD76At1V3dQn68DNdg/AOtOCNMMut5Zz5 YV5vDf82x4YJwuLdKQZbCz9ao1FUtLa6hGdDtj4WrDLCejvF8mgkc/qENju8VR+Xke qFewpsNjfxpQg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1vlP2q-00000006i5m-1uFn; Thu, 29 Jan 2026 10:14:36 +0000 Date: Thu, 29 Jan 2026 10:14:36 +0000 Message-ID: <86bjicbu9f.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, Joey Gouly , Suzuki K Poulose , Oliver Upton , Zenghui Yu , Will Deacon , Catalin Marinas Subject: Re: [PATCH 12/20] KVM: arm64: Add RESx_WHEN_E2Hx constraints as configuration flags In-Reply-To: References: <20260126121655.1641736-1-maz@kernel.org> <20260126121655.1641736-13-maz@kernel.org> 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) 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: tabba@google.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, joey.gouly@arm.com, suzuki.poulose@arm.com, oupton@kernel.org, yuzenghui@huawei.com, will@kernel.org, catalin.marinas@arm.com 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-20260129_021440_842259_AD14836E X-CRM114-Status: GOOD ( 46.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hey Fuad, On Wed, 28 Jan 2026 17:43:40 +0000, Fuad Tabba wrote: > > Hi Marc, > > On Mon, 26 Jan 2026 at 12:17, Marc Zyngier wrote: > > > > "Thanks" to VHE, SCTLR_EL2 radically changes shape depending on the > > value of HCR_EL2.E2H, as a lot of the bits that didn't have much > > meaning with E2H=0 start impacting EL0 with E2H=1. > > > > This has a direct impact on the RESx behaviour of these bits, and > > we need a way to express them. > > > > For this purpose, introduce a set of 4 new constaints that, when > > the controlling feature is not present, force the RESx value to > > be either 0 or 1 depending on the value of E2H. > > > > This allows diverging RESx values depending on the value of E2H, > > something that is required by a bunch of SCTLR_EL2 bits. > > > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/kvm/config.c | 24 +++++++++++++++++++++--- > > 1 file changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c > > index 1990cebc77c66..7063fffc22799 100644 > > --- a/arch/arm64/kvm/config.c > > +++ b/arch/arm64/kvm/config.c > > @@ -26,6 +26,10 @@ struct reg_bits_to_feat_map { > > #define MASKS_POINTER BIT(3) /* Pointer to fgt_masks struct instead of bits */ > > #define AS_RES1 BIT(4) /* RES1 when not supported */ > > #define REQUIRES_E2H1 BIT(5) /* Add HCR_EL2.E2H RES1 as a pre-condition */ > > +#define RES0_WHEN_E2H0 BIT(6) /* RES0 when E2H=0 and not supported */ > > +#define RES0_WHEN_E2H1 BIT(7) /* RES0 when E2H=1 and not supported */ > > +#define RES1_WHEN_E2H0 BIT(8) /* RES1 when E2H=0 and not supported */ > > +#define RES1_WHEN_E2H1 BIT(9) /* RES1 when E2H=1 and not supported */ > > > > unsigned long flags; > > > > @@ -1298,10 +1302,24 @@ struct resx compute_resx_bits(struct kvm *kvm, > > match &= !e2h0; > > > > if (!match) { > > + u64 bits = reg_feat_map_bits(&map[i]); > > + > > + if (e2h0) { > > + if (map[i].flags & RES1_WHEN_E2H0) > > + resx.res1 |= bits; > > + else if (map[i].flags & RES0_WHEN_E2H0) > > + resx.res0 |= bits; > > + } else { > > + if (map[i].flags & RES1_WHEN_E2H1) > > + resx.res1 |= bits; > > + else if (map[i].flags & RES0_WHEN_E2H1) > > + resx.res0 |= bits; > > + } > > + > > if (map[i].flags & AS_RES1) > > - resx.res1 |= reg_feat_map_bits(&map[i]); > > - else > > - resx.res0 |= reg_feat_map_bits(&map[i]); > > + resx.res1 |= bits; > > + else if (!(resx.res1 & bits)) > > + resx.res0 |= bits; > > The logic here feels a bit more complex than necessary, specifically > regarding the interaction between the E2H checks and the fallthrough > to AS_RES1. > > Although AS_RES1 and RES0_WHEN_E2H0 are mutually exclusive in > practice, the current structure technically permits a scenario where > both res0 and res1 get set if the flags are mixed (the e2h0 block sets > res0, and the AS_RES1 block falls through and sets res1). This cannot > be ruled out by looking at this function alone. > > It might be cleaner (and safer) to determine the res1 first, and > then apply the masks. Something like: > > + bool is_res1 = false; > + > + if (map[i].flags & AS_RES1) > + is_res1 = true; > + else if (e2h0) > + is_res1 = (map[i].flags & RES1_WHEN_E2H0); > + else > + is_res1 = (map[i].flags & RES1_WHEN_E2H1); > ... I think you have just put your finger on something that escaped me so far. You are totally right that the code as written today is ugly, and the trick to work out that we need to account the bits as RES0 is awful. But it additionally outlines something else: since RES0 is an implicit property (we don't specify a flag for it), RES0_WHEN_E2Hx could also be implicit properties. I couldn't find an example where anything would break. This would also avoid the combination with AS_RES1 by construction. > > This also brings up a side point: given the visual similarity of these > flags, it is quite easy to make a typo and accidentally combine > incompatible flags (e.g., AS_RES1 | RESx_WHEN_E2Hx, or RES0_WHEN_E2H0 > | RES1_WHEN_E2H0), would it be worth adding a check to warn on > obviously invalid combinations? > > Or maybe even redefining AS_RES1 to be > (RES1_WHEN_E2H1|RES1_WHEN_E2H0), which is what it is conceptually. > That could simplify this code even further: > > + if (e2h0) > + is_res1 = (map[i].flags & RES1_WHEN_E2H0); > + else > + is_res1 = (map[i].flags & RES1_WHEN_E2H1); While that would work, I think this is a step too far. Eventually, we should be able to sanitise things outside of NV, and RES1 should not depend on E2H at all in this case. I ended up with the following hack, completely untested (needs renumbering, and the rest of SCTLR_EL2 repainted). Let me know what you think. Thanks, M. diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c index 562513a4683e2..204e5aeda4d24 100644 --- a/arch/arm64/kvm/config.c +++ b/arch/arm64/kvm/config.c @@ -26,8 +26,6 @@ struct reg_bits_to_feat_map { #define MASKS_POINTER BIT(3) /* Pointer to fgt_masks struct instead of bits */ #define AS_RES1 BIT(4) /* RES1 when not supported */ #define REQUIRES_E2H1 BIT(5) /* Add HCR_EL2.E2H RES1 as a pre-condition */ -#define RES0_WHEN_E2H0 BIT(6) /* RES0 when E2H=0 and not supported */ -#define RES0_WHEN_E2H1 BIT(7) /* RES0 when E2H=1 and not supported */ #define RES1_WHEN_E2H0 BIT(8) /* RES1 when E2H=0 and not supported */ #define RES1_WHEN_E2H1 BIT(9) /* RES1 when E2H=1 and not supported */ @@ -1375,22 +1373,15 @@ struct resx compute_resx_bits(struct kvm *kvm, if (!match) { u64 bits = reg_feat_map_bits(&map[i]); + bool res1; - if (e2h0) { - if (map[i].flags & RES1_WHEN_E2H0) - resx.res1 |= bits; - else if (map[i].flags & RES0_WHEN_E2H0) - resx.res0 |= bits; - } else { - if (map[i].flags & RES1_WHEN_E2H1) - resx.res1 |= bits; - else if (map[i].flags & RES0_WHEN_E2H1) - resx.res0 |= bits; - } - - if (map[i].flags & AS_RES1) + res1 = (map[i].flags & AS_RES1); + res1 |= e2h0 && (map[i].flags & RES1_WHEN_E2H0); + res1 |= !e2h0 && (map[i].flags & RES1_WHEN_E2H1); + + if (res1) resx.res1 |= bits; - else if (!(resx.res1 & bits)) + else resx.res0 |= bits; } } -- Without deviation from the norm, progress is not possible.