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:
> > __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: [email protected]
> > Signed-off-by: Fuad Tabba <[email protected]>
> > ---
> 
> [...]
> 
> > 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/
> 
> 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