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 8D2CEC55184 for ; Fri, 20 Feb 2026 10:10:00 +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=Kc6/Eir/gKPSVkbLPrICPq9c00va2Ku7SFS7v5CxSDE=; b=y2VdwVZTewyKCG0Bh1MquAJXLL M+VxA4KnxPYxoyzlsG7L1wr+3pZhCQCMlJUpIbAYhfdt+0q7Po0ByHYUm4v0/lBFwnltnAhnkcuw6 dRk00QpQgvJVPo3tHsWkjaE3uE76Lr20nRGQblEGC0cxFrFZl7/XZklG58Mx6nKd8oYnY6XykZVq7 /zCNDtTBd2H4bcfvf9II/tlUZ6LD5Dg3/ZMpUP/Z5odb/0GK14vUU8k4EpqhUJh0sxTYcpv4CagBO bn3GHbJwgLohjboeB7j4JwTEub4CkSLz5E2CNZMNdGyRufYndNnGxTUnEp17RqlHuCspg73HvPjPX /xoD6uBA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vtNSL-0000000DrdS-3Ilr; Fri, 20 Feb 2026 10:09:53 +0000 Received: from sea.source.kernel.org ([172.234.252.31]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vtNSI-0000000DraT-1WOf for linux-arm-kernel@lists.infradead.org; Fri, 20 Feb 2026 10:09:52 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id BC9414436A; Fri, 20 Feb 2026 10:09:49 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7959BC2BC87; Fri, 20 Feb 2026 10:09:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1771582189; bh=cn4TTDudr7pyoUuW5ws/Qgyj6NNj2ywFq9qEjCdNWOQ=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Js4XPV0vpJZqOW6TsfAgkmrnaDspmvCj+19swe9MRf2GbJNXSdl3DXSwcQAvHnkff lfczSiVLp7RZJthFs7OAQuhE/ADglIxJAWwoFyBVSaTFE7pKUJFj9mZcWQzms3Kt0k F8xiVTqBaxLcu29TcQOvEJWOu/Bca32EiFwoBmp7dbaLOni7teB9cTe1uuy2IJ3fGa VssfyA9Yrg7vUUiCy03mR797BwNIXCG0i5Ikq9AeUjNyvVdRvzgW8icgzu9YTQjNU6 g9R2iswJ5qagdRRgxmXUMjD2TpsEsjQhlm7ZobEcz3N8iA1k435TjER9oodWqQmo2I wvu5tBId+L7cg== 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 1vtNSF-0000000CP7W-07ar; Fri, 20 Feb 2026 10:09:47 +0000 Date: Fri, 20 Feb 2026 10:09:46 +0000 Message-ID: <86v7frafpx.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, Will Deacon , Catalin Marinas , Mark Rutland , Joey Gouly , Suzuki K Poulose , Oliver Upton , Zenghui Yu Subject: Re: [PATCH 1/9] arm64: Add logic to fully remove features from sanitised id registers In-Reply-To: References: <20260219195533.2455736-1-maz@kernel.org> <20260219195533.2455736-2-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, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, joey.gouly@arm.com, suzuki.poulose@arm.com, oupton@kernel.org, yuzenghui@huawei.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-20260220_020950_447050_58DD7503 X-CRM114-Status: GOOD ( 45.31 ) 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, Thanks for taking a look. On Fri, 20 Feb 2026 08:36:04 +0000, Fuad Tabba wrote: > > Hi Marc, > > On Thu, 19 Feb 2026 at 19:55, Marc Zyngier wrote: > > > > We currently make support for some features such as Pointer Auth, > > SVE or S1POE a compile time decision. > > > > However, while we hide that feature from userspace when such support > > is disabled, we still leave the value provided by the HW visible to > > the rest of the kernel, including KVM. > > > > This has the potential to result in ugly state leakage, as half of > > the kernel knows about the feature, and the other doesn't. > > > > Short of completely banning such compilation options and restore > > universal knowledge, introduce the possibility to fully remove such > > knowledge from the sanitised id registers. > > > > This has more or less the same effect as the idreg override that > > a user can pass on the command-line, only defined at build-time. > > > > For that purpose, we provide a new macro (FTR_CONFIG()) that defines > > the behaviour of a feature, both when enabled and disabled. > > > > At this stage, nothing is making use of this anti-feature. > > > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/include/asm/cpufeature.h | 15 ++++++++++----- > > arch/arm64/kernel/cpufeature.c | 21 ++++++++++++++++----- > > 2 files changed, 26 insertions(+), 10 deletions(-) > > > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > > index 4de51f8d92cba..2731ea13c2c86 100644 > > --- a/arch/arm64/include/asm/cpufeature.h > > +++ b/arch/arm64/include/asm/cpufeature.h > > @@ -53,15 +53,20 @@ enum ftr_type { > > #define FTR_SIGNED true /* Value should be treated as signed */ > > #define FTR_UNSIGNED false /* Value should be treated as unsigned */ > > > > -#define FTR_VISIBLE true /* Feature visible to the user space */ > > -#define FTR_HIDDEN false /* Feature is hidden from the user */ > > +enum ftr_visibility { > > + FTR_HIDDEN, /* Feature hidden from the user */ > > + FTR_ALL_HIDDEN, /* Feature hidden from kernel, user and KVM */ > > + FTR_VISIBLE, /* Feature visible to all observers */ > > +}; > > + > > +#define FTR_CONFIG(c, e, d) \ > > + (IS_ENABLED(c) ? FTR_ ## e : FTR_ ## d) > > > > -#define FTR_VISIBLE_IF_IS_ENABLED(config) \ > > - (IS_ENABLED(config) ? FTR_VISIBLE : FTR_HIDDEN) > > +#define FTR_VISIBLE_IF_IS_ENABLED(c) FTR_CONFIG(c, VISIBLE, HIDDEN) > > > > struct arm64_ftr_bits { > > bool sign; /* Value is signed ? */ > > - bool visible; > > + enum ftr_visibility visibility; > > bool strict; /* CPU Sanity check: strict matching required ? */ > > enum ftr_type type; > > u8 shift; > > This introduces bloat. Should you group the bools together and the > enums together? That should be possible as long as everybody ends up using the __ARM64_FTR_BITS macro. On the other hand, I could reduce the width of the enum to something more appropriate and keep the current layout. With the current changes, this looks like this: struct arm64_ftr_bits { bool sign; /* 0 1 */ /* XXX 3 bytes hole, try to pack */ enum ftr_visibility visibility; /* 4 4 */ bool strict; /* 8 1 */ /* XXX 3 bytes hole, try to pack */ enum ftr_type type; /* 12 4 */ u8 shift; /* 16 1 */ u8 width; /* 17 1 */ /* XXX 6 bytes hole, try to pack */ s64 safe_val; /* 24 8 */ /* size: 32, cachelines: 1, members: 7 */ /* sum members: 20, holes: 3, sum holes: 12 */ /* last cacheline: 32 bytes */ }; which is 8 bytes larger than the upstream version. But if I do this: diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index adaae3060851c..d8accc9c94fab 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -64,9 +64,9 @@ enum ftr_visibility { struct arm64_ftr_bits { bool sign; /* Value is signed ? */ - enum ftr_visibility visibility; + enum ftr_visibility visibility:8; bool strict; /* CPU Sanity check: strict matching required ? */ - enum ftr_type type; + enum ftr_type type:8; u8 shift; u8 width; s64 safe_val; /* safe value for FTR_EXACT features */ I end up with the following layout: struct arm64_ftr_bits { bool sign; /* 0 1 */ /* Bitfield combined with previous fields */ enum ftr_visibility visibility:8; /* 0: 8 4 */ /* Bitfield combined with next fields */ bool strict; /* 2 1 */ /* Bitfield combined with previous fields */ enum ftr_type type:8; /* 0:24 4 */ u8 shift; /* 4 1 */ u8 width; /* 5 1 */ /* XXX 2 bytes hole, try to pack */ s64 safe_val; /* 8 8 */ /* size: 16, cachelines: 1, members: 7 */ /* sum members: 12, holes: 1, sum holes: 2 */ /* sum bitfield members: 16 bits (2 bytes) */ /* last cacheline: 16 bytes */ }; which is 8 bytes *smaller* than the upstream version, without reordering. WDYT? > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index c840a93b9ef95..b34a39967d111 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -192,7 +192,7 @@ void dump_cpu_features(void) > > #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ > > { \ > > .sign = SIGNED, \ > > - .visible = VISIBLE, \ > > + .visibility = VISIBLE, \ > > .strict = STRICT, \ > > .type = TYPE, \ > > .shift = SHIFT, \ > > @@ -1057,17 +1057,28 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new) > > ftrp->shift); > > } > > > > - val = arm64_ftr_set_value(ftrp, val, ftr_new); > > - > > valid_mask |= ftr_mask; > > if (!ftrp->strict) > > Should FTR_ALL_HIDDEN also be removed from strict_mask? i.e. > > - if (!ftrp->strict) > + if (!ftrp->strict || || ftrp->visibility == FTR_ALL_HIDDEN) > > (or under the ALL_HIDDEN case below). If we run on a system that has diverging features, we probably want to know irrespective of the feature being enabled. After all, the integration is out of spec, and conveying that information is important, just in case the diverging feature affects behaviour in funny ways... > > > > strict_mask &= ~ftr_mask; > > - if (ftrp->visible) > > + > > + switch (ftrp->visibility) { > > + case FTR_VISIBLE: > > + val = arm64_ftr_set_value(ftrp, val, ftr_new); > > user_mask |= ftr_mask; > > - else > > + break; > > + case FTR_ALL_HIDDEN: > > + val = arm64_ftr_set_value(ftrp, val, ftrp->safe_val); > > + reg->user_val = arm64_ftr_set_value(ftrp, > > + reg->user_val, > > + ftrp->safe_val); > > Should we also take the safe value in update_cpu_ftr_reg() for FTR_ALL_HIDDEN? I would expect arm64_ftr_safe_value() to do the right thing at that stage, given that we have primed the boot CPU with the safe value, and that we rely on that bootstrap to make the registers converge towards something safe. This is also what happens for the command-line override. Or have you spotted a case where this go wrong? Thanks, M. -- Without deviation from the norm, progress is not possible.