Re: [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()

On Mon, Aug 24, 2026 at 02:09:39PM +0100, Will Deacon wrote:

On Mon, Aug 24, 2026 at 08:10:04AM +0100, Fuad Tabba wrote:

quoted

__cpuinfo_store_cpu() gates the GMID_EL1 read on the raw
ID_AA64PFR1_EL1, so it reads the register on MTE hardware even when the
kernel has disabled MTE (CONFIG_ARM64_MTE=n or arm64.nomte). KVM sets
HCR_EL2.TID5 in that case, which traps the read to EL2 and injects an
UNDEF:

  Internal error: Oops - Undefined instruction: 0000000002000000 [#1]
  pc : __cpuinfo_store_cpu+0xf4/0x264
  Call trace:
   __cpuinfo_store_cpu+0xf4/0x264 (P)
   secondary_start_kernel+0xc8/0x1d0
   __secondary_switched+0xc0/0xc4
  Kernel panic - not syncing: Attempted to kill the idle task!

Only pKVM is affected, and only for a CPU that is offlined and brought
back online. Every CPU's first bring-up precedes KVM's initcall, and
pKVM refuses CPU_ON for a CPU that was not online when KVM initialised,
so hotplug is the only path to cpuinfo_store_cpu() with TID5 set. pKVM's
PSCI CPU_ON relay sets the host HCR before the CPU enters EL1, whereas
plain nVHE sets it at CPUHP_AP_KVM_ONLINE, after cpuinfo_store_cpu().

Defer the read to {init,update}_cpu_features() and gate it on the
sanitised ID register, as MPAM already does. system_supports_mte()
cannot serve as the gate, as update_cpu_features() also runs during
initial SMP bring-up, before smp_cpus_done() calls
setup_system_features(). The init path gains the CONFIG_ARM64_MTE test
the update path already had, leaving SYS_GMID_EL1 uninitialised when
MTE is compiled out, where its only other user (lib/mte.S) is not
built.

Fixes: f35abcbb8a084 ("KVM: arm64: Trap MTE access and discovery when MTE is disabled")
Cc: stable@vger.kernel.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---

[...]

quoted

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index d50e2a9b066b3..c8967f185e3dd 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c

@@ -502,12 +502,14 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 	info->reg_id_aa64smfr0 = read_cpuid(ID_AA64SMFR0_EL1);
 	info->reg_id_aa64fpfr0 = read_cpuid(ID_AA64FPFR0_EL1);
 
-	if (id_aa64pfr1_mte(info->reg_id_aa64pfr1))
-		info->reg_gmid = read_cpuid(GMID_EL1);
-
 	if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
 		__cpuinfo_store_cpu_32bit(&info->aarch32);
 
+	/*
+	 * info->reg_gmid deferred to {init,update}_cpu_features because
+	 * reading it traps to EL2 when MTE is disabled.
+	 */

I don't think we should defer this, as I've been actively doing the
opposite for parallel CPU onlining (where the ID registers can be read
concurrently by incoming CPUs to amortise the cost of a trap) and also
for the RNG traps during early boot:

https://lore.kernel.org/all/annJ0oDB2HObQC5j@willie-the-truck/ (local)

If you look at the diff I sent in the thread above (I didn't get a
reply), the idea is that __read_sysreg_by_encoding() reads from the ID
register values stashed by cpuinfo_store_cpu(). So you could use that
to check id_aa64pfr1 before reading gmid, as it will give you the
sanitised view.

Since cpu_data[] has the raw per-cpu regs, I think it makes a lot of
sense to read the cached values in __read_sysreg_by_encoding() than
issuing the MRS again.

Given Marc's earlier series to make config=n and override behave
similarly, I wonder whether we should just store the overridden value in
cpu_data (with the clamp Suzuki was proposing to avoid making up
features on the command line). We can then get rid of the override
applied in various places and it might be easier to take the config into
account as well.

-- 
Catalin