mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
@ 2026-08-19 14:25 Fuad Tabba
  2026-08-22  9:13 ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Fuad Tabba @ 2026-08-19 14:25 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, linux-arm-kernel
  Cc: Marc Zyngier, Oliver Upton, Mark Rutland, Suzuki K Poulose,
	Mark Brown, kvmarm, linux-kernel, Fuad Tabba

__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 onlined after KVM init: its
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>
---
 arch/arm64/kernel/cpufeature.c | 19 ++++++++++++++++---
 arch/arm64/kernel/cpuinfo.c    |  8 +++++---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9a22df0c5120f..5120f6721b1e0 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1176,6 +1176,17 @@ static bool detect_ftr_has_mpam(void)
 	return id_aa64pfr0_mpam(pfr0) || id_aa64pfr1_mpamfrac(pfr1);
 }
 
+/*
+ * Mirrors system_supports_mte(), which cannot be used before the capabilities
+ * are finalised. KVM sets HCR_EL2.TID5 when it is false, trapping GMID_EL1.
+ */
+static bool detect_has_mte(void)
+{
+	u64 pfr1 = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1);
+
+	return IS_ENABLED(CONFIG_ARM64_MTE) && id_aa64pfr1_mte(pfr1);
+}
+
 void __init init_cpu_features(struct cpuinfo_arm64 *info)
 {
 	/* Before we start using the tables, make sure it is sorted */
@@ -1228,8 +1239,10 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
 		init_cpu_ftr_reg(SYS_MPAMIDR_EL1, info->reg_mpamidr);
 	}
 
-	if (id_aa64pfr1_mte(info->reg_id_aa64pfr1))
+	if (detect_has_mte()) {
+		info->reg_gmid = read_cpuid(GMID_EL1);
 		init_cpu_ftr_reg(SYS_GMID_EL1, info->reg_gmid);
+	}
 }
 
 static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
@@ -1490,8 +1503,8 @@ void update_cpu_features(int cpu,
 	 * they read/write depends on the GMID_EL1.BS field. Check that the
 	 * value is the same on all CPUs.
 	 */
-	if (IS_ENABLED(CONFIG_ARM64_MTE) &&
-	    id_aa64pfr1_mte(info->reg_id_aa64pfr1)) {
+	if (detect_has_mte()) {
+		info->reg_gmid = read_cpuid(GMID_EL1);
 		taint |= check_update_ftr_reg(SYS_GMID_EL1, cpu,
 					      info->reg_gmid, boot->reg_gmid);
 	}
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.
+	 */
+
 	/*
 	 * info->reg_mpamidr deferred to {init,update}_cpu_features because we
 	 * don't want to read it (and trigger a trap on buggy firmware) if
-- 
2.39.5


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
  2026-08-19 14:25 [PATCH] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features() Fuad Tabba
@ 2026-08-22  9:13 ` Marc Zyngier
  2026-08-23 13:09   ` Fuad Tabba
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2026-08-22  9:13 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Oliver Upton,
	Mark Rutland, Suzuki K Poulose, Mark Brown, kvmarm, linux-kernel,
	Fuad Tabba

On Wed, 19 Aug 2026 15:25:58 +0100,
Fuad Tabba <fuad.tabba@linux.dev> 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 onlined after KVM init: its

Is that for a CPU that has gone through a an offline/online cycle?
Because otherwise, such a CPU wouldn't be able to boot at all (we have
a strong requirement that pKVM sees all CPUs at boot time).

If that's the case, some clarification would be good. Otherwise, some
clarification is absolutely required! ;-)

> 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.

I'm starting to wonder whether having CPUs to go through a feature
collection process during an offline/online cycle is a good idea. The
data should still be there, and is not expected to change.

Is it just that we don't have the correct tracking information?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
  2026-08-22  9:13 ` Marc Zyngier
@ 2026-08-23 13:09   ` Fuad Tabba
  2026-08-24 13:11     ` Will Deacon
  0 siblings, 1 reply; 4+ messages in thread
From: Fuad Tabba @ 2026-08-23 13:09 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Oliver Upton,
	Mark Rutland, Suzuki K Poulose, Mark Brown, kvmarm, linux-kernel

Hi Marc,

On Sat, 22 Aug 2026 at 10:13, Marc Zyngier <maz@kernel.org> wrote:
>
> On Wed, 19 Aug 2026 15:25:58 +0100,
> Fuad Tabba <fuad.tabba@linux.dev> 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 onlined after KVM init: its
>
> Is that for a CPU that has gone through a an offline/online cycle?
> Because otherwise, such a CPU wouldn't be able to boot at all (we have
> a strong requirement that pKVM sees all CPUs at boot time).
>
> If that's the case, some clarification would be good. Otherwise, some
> clarification is absolutely required! ;-)

Yes, an offline/online cycle: every CPU's first bring-up happens
before KVM's initcall, so hotplug is the only way to reach
cpuinfo_store_cpu() with TID5 set. I'll say so in v2, along with a
note on the reproducer (offline then online CPU1 with arm64.nomte, on
QEMU with MTE enabled).

As a side note, to answer a question Will asked me offlist, it's not
an issue in any of the Android versions: Android 17 (and earlier)
never gets the trap. The Android commit that disables MTE there does
the HCR_ATA hunks only, and there is no handle_host_mte() either, so
the read is untrapped.

> > 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.
>
> I'm starting to wonder whether having CPUs to go through a feature
> collection process during an offline/online cycle is a good idea. The
> data should still be there, and is not expected to change.
>
> Is it just that we don't have the correct tracking information?

I think you're right. Nothing on the arm64 side tells a re-online from
a first boot, so secondary_start_kernel() collects everything again.
The tracking is there I think: notify_cpu_starting() sets
cpus_booted_once_mask, and it runs right after cpuinfo_store_cpu(), so
the mask is usable at that point. Gating cpuinfo_store_cpu() on it
would skip the collection and the cross-check against the boot CPU on
a re-online, and leave the verification in
check_local_cpu_capabilities() as is.

That changes what hotplug does on every arm64 system, so I'll send
this other patch after the merge window, and keep the current one as
the narrow fix for stable (the trap is in 7.0 onwards). Does that
work?

Cheers,
/fuad

> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
  2026-08-23 13:09   ` Fuad Tabba
@ 2026-08-24 13:11     ` Will Deacon
  0 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2026-08-24 13:11 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Marc Zyngier, Catalin Marinas, linux-arm-kernel, Oliver Upton,
	Mark Rutland, Suzuki K Poulose, Mark Brown, kvmarm, linux-kernel

On Sun, Aug 23, 2026 at 02:09:02PM +0100, Fuad Tabba wrote:
> On Sat, 22 Aug 2026 at 10:13, Marc Zyngier <maz@kernel.org> wrote:
> > On Wed, 19 Aug 2026 15:25:58 +0100,
> > Fuad Tabba <fuad.tabba@linux.dev> 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 onlined after KVM init: its
> >
> > Is that for a CPU that has gone through a an offline/online cycle?
> > Because otherwise, such a CPU wouldn't be able to boot at all (we have
> > a strong requirement that pKVM sees all CPUs at boot time).
> >
> > If that's the case, some clarification would be good. Otherwise, some
> > clarification is absolutely required! ;-)
> 
> Yes, an offline/online cycle: every CPU's first bring-up happens
> before KVM's initcall, so hotplug is the only way to reach
> cpuinfo_store_cpu() with TID5 set. I'll say so in v2, along with a
> note on the reproducer (offline then online CPU1 with arm64.nomte, on
> QEMU with MTE enabled).
> 
> As a side note, to answer a question Will asked me offlist, it's not
> an issue in any of the Android versions: Android 17 (and earlier)
> never gets the trap. The Android commit that disables MTE there does
> the HCR_ATA hunks only, and there is no handle_host_mte() either, so
> the read is untrapped.
> 
> > > 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.
> >
> > I'm starting to wonder whether having CPUs to go through a feature
> > collection process during an offline/online cycle is a good idea. The
> > data should still be there, and is not expected to change.
> >
> > Is it just that we don't have the correct tracking information?
> 
> I think you're right. Nothing on the arm64 side tells a re-online from
> a first boot, so secondary_start_kernel() collects everything again.

I think that's deliberate, because we support the mostly theoretical
case of a different physical CPU being hotplugged in. Even though it's
a bit far-fetched for most systems, I don't think it's something we
should break unless we have a good reason to do so.

Will

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-24 13:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-19 14:25 [PATCH] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features() Fuad Tabba
2026-08-22  9:13 ` Marc Zyngier
2026-08-23 13:09   ` Fuad Tabba
2026-08-24 13:11     ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®