Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
@ 2026-08-24  7:10 Fuad Tabba
  2026-08-24 12:24 ` Catalin Marinas
  2026-08-24 13:09 ` Will Deacon
  0 siblings, 2 replies; 8+ messages in thread
From: Fuad Tabba @ 2026-08-24  7:10 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 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>
---

Notes:
    Changes since v1:
    - Clarified that the trap only fires on a CPU that is offlined and
      brought back online, not a late first boot (Marc).
    
    Tested on QEMU with -machine virt,mte=on, under pKVM.
    
    Offline/online CPU1 with arm64.nomte on the host cmdline: unpatched
    panics in __cpuinfo_store_cpu(), patched does not.

 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 related	[flat|nested] 8+ messages in thread

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

On Mon, Aug 24, 2026 at 08:10:04AM +0100, Fuad Tabba wrote:
> 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.
> +	 */

The only nit I have is that cpu_data[0] now won't have the GMID_EL1 set.
It doesn't matter as we don't expose it via c_show() or sysfs, rather
for consistency. We have this inconsistency already with mpamidr.

I guess we just need to call init_cpu_features() on the per-CPU data
before the assignment to boot_cpu_data. Something like (untested):

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index d50e2a9b066b..d22f3514ab8b 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -538,7 +538,7 @@ void __init cpuinfo_store_boot_cpu(void)
 {
 	struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0);
 	__cpuinfo_store_cpu(info);
+	init_cpu_features(info);

 	boot_cpu_data = *info;
-	init_cpu_features(&boot_cpu_data);
 }

Either way:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>


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

* Re: [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
  2026-08-24  7:10 [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features() Fuad Tabba
  2026-08-24 12:24 ` Catalin Marinas
@ 2026-08-24 13:09 ` Will Deacon
  2026-08-24 15:14   ` Fuad Tabba
  2026-08-24 16:25   ` Catalin Marinas
  1 sibling, 2 replies; 8+ messages in thread
From: Will Deacon @ 2026-08-24 13:09 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Catalin Marinas, linux-arm-kernel, Marc Zyngier, Oliver Upton,
	Mark Rutland, Suzuki K Poulose, Mark Brown, kvmarm, linux-kernel,
	Fuad Tabba

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: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---

[...]

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

The only snag is that I didn't convert all the registers over, so
there's some work there.

WDYT?

Will


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

* Re: [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
  2026-08-24 13:09 ` Will Deacon
@ 2026-08-24 15:14   ` Fuad Tabba
  2026-08-24 15:33     ` Catalin Marinas
  2026-08-24 15:54     ` Will Deacon
  2026-08-24 16:25   ` Catalin Marinas
  1 sibling, 2 replies; 8+ messages in thread
From: Fuad Tabba @ 2026-08-24 15:14 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, linux-arm-kernel, Marc Zyngier, Oliver Upton,
	Mark Rutland, Suzuki K Poulose, Mark Brown, kvmarm, linux-kernel

Hi Will,

On Mon, 24 Aug 2026 at 14:09, Will Deacon <will@kernel.org> wrote:

> > +     /*
> > +      * 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.

I just did, but I don't think it'll work. The thing is,
__read_sysreg_by_encoding() gives the override-applied local value,
not the folded sys_val.

arm64.nomte is a command-line override, so that works. But
CONFIG_ARM64_MTE=n sets none. Any gate would still need an explicit
IS_ENABLED(CONFIG_ARM64_MTE).

That said, I don't have to defer it. I can keep the info->reg_gmid
read in __cpuinfo_store_cpu() and gate that read on the state that
arms the trap:

        static inline bool gmid_el1_accessible(u64 pfr1)
        {
                if (!IS_ENABLED(CONFIG_ARM64_MTE))
                        return false;
                if (system_capabilities_finalized())
                        return system_supports_mte();
                return id_aa64pfr1_mte(pfr1);
        }


TID5 is set from system_supports_mte() and only once capabilities are
finalised, so the local check is still correct before that. Gating on
system_supports_mte() keeps the different-physical-CPU case you want
to preserve working: if one re-onlines with MTE while the system view
has it folded off, the gate stays false and it won't read GMID_EL1.

If you agree, I'll respin with that.

Cheers,
/fuad





>
> The only snag is that I didn't convert all the registers over, so
> there's some work there.
>
> WDYT?
>
> Will


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

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

On Mon, Aug 24, 2026 at 04:14:41PM +0100, Fuad Tabba wrote:
> Hi Will,
> 
> On Mon, 24 Aug 2026 at 14:09, Will Deacon <will@kernel.org> wrote:
> 
> > > +     /*
> > > +      * 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.
> 
> I just did, but I don't think it'll work. The thing is,
> __read_sysreg_by_encoding() gives the override-applied local value,
> not the folded sys_val.
> 
> arm64.nomte is a command-line override, so that works. But
> CONFIG_ARM64_MTE=n sets none. Any gate would still need an explicit
> IS_ENABLED(CONFIG_ARM64_MTE).

We need to revive this series:

https://lore.kernel.org/all/20260302115653.1517326-1-maz@kernel.org/

I don't remember where we left it but in principle config off or
override should look similar to the kernel.

There's also the override making an absent feature present. Suzuki has
attempted to fix this in a reply:

https://lore.kernel.org/all/afc5bd00-28ca-413b-b047-ee53589c285d@arm.com/

> That said, I don't have to defer it. I can keep the info->reg_gmid
> read in __cpuinfo_store_cpu() and gate that read on the state that
> arms the trap:
> 
>         static inline bool gmid_el1_accessible(u64 pfr1)
>         {
>                 if (!IS_ENABLED(CONFIG_ARM64_MTE))
>                         return false;
>                 if (system_capabilities_finalized())
>                         return system_supports_mte();
>                 return id_aa64pfr1_mte(pfr1);
>         }
> 
> 
> TID5 is set from system_supports_mte() and only once capabilities are
> finalised, so the local check is still correct before that. Gating on
> system_supports_mte() keeps the different-physical-CPU case you want
> to preserve working: if one re-onlines with MTE while the system view
> has it folded off, the gate stays false and it won't read GMID_EL1.

I think this would be the easiest to backport and we can look at
reworking this infrastructure in the future (Will's proposal, Marc's
override fixes etc.)

-- 
Catalin


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

* Re: [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features()
  2026-08-24 15:14   ` Fuad Tabba
  2026-08-24 15:33     ` Catalin Marinas
@ 2026-08-24 15:54     ` Will Deacon
  2026-08-24 18:36       ` Fuad Tabba
  1 sibling, 1 reply; 8+ messages in thread
From: Will Deacon @ 2026-08-24 15:54 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Catalin Marinas, linux-arm-kernel, Marc Zyngier, Oliver Upton,
	Mark Rutland, Suzuki K Poulose, Mark Brown, kvmarm, linux-kernel

On Mon, Aug 24, 2026 at 04:14:41PM +0100, Fuad Tabba wrote:
> On Mon, 24 Aug 2026 at 14:09, Will Deacon <will@kernel.org> wrote:
> 
> > > +     /*
> > > +      * 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.
> 
> I just did, but I don't think it'll work. The thing is,
> __read_sysreg_by_encoding() gives the override-applied local value,
> not the folded sys_val.
> 
> arm64.nomte is a command-line override, so that works. But
> CONFIG_ARM64_MTE=n sets none. Any gate would still need an explicit
> IS_ENABLED(CONFIG_ARM64_MTE).
> 
> That said, I don't have to defer it. I can keep the info->reg_gmid
> read in __cpuinfo_store_cpu() and gate that read on the state that
> arms the trap:
> 
>         static inline bool gmid_el1_accessible(u64 pfr1)
>         {
>                 if (!IS_ENABLED(CONFIG_ARM64_MTE))
>                         return false;
>                 if (system_capabilities_finalized())
>                         return system_supports_mte();
>                 return id_aa64pfr1_mte(pfr1);
>         }
> 
> 
> TID5 is set from system_supports_mte() and only once capabilities are
> finalised, so the local check is still correct before that. Gating on
> system_supports_mte() keeps the different-physical-CPU case you want
> to preserve working: if one re-onlines with MTE while the system view
> has it folded off, the gate stays false and it won't read GMID_EL1.

I still don't understand why you have system_supports_mte() here. If
the CPU has MTE, the CONFIG option is enabled and the cmdline override
isn't set, we can read GMID just fine on this CPU, regardless of the
system capabilities.

> If you agree, I'll respin with that.

I can't tell who's calling gmid_el1_accessible() so it's hard to tell
whether I agree or not. May as well send a v3 though, so we can see what
you have in mind!

Will


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

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

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: stable@vger.kernel.org
> > Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> > ---
> 
> [...]
> 
> > 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


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

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

On Mon, 24 Aug 2026 at 16:54, Will Deacon <will@kernel.org> wrote:
>
> On Mon, Aug 24, 2026 at 04:14:41PM +0100, Fuad Tabba wrote:
> > On Mon, 24 Aug 2026 at 14:09, Will Deacon <will@kernel.org> wrote:
> >
> > > > +     /*
> > > > +      * 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.
> >
> > I just did, but I don't think it'll work. The thing is,
> > __read_sysreg_by_encoding() gives the override-applied local value,
> > not the folded sys_val.
> >
> > arm64.nomte is a command-line override, so that works. But
> > CONFIG_ARM64_MTE=n sets none. Any gate would still need an explicit
> > IS_ENABLED(CONFIG_ARM64_MTE).
> >
> > That said, I don't have to defer it. I can keep the info->reg_gmid
> > read in __cpuinfo_store_cpu() and gate that read on the state that
> > arms the trap:
> >
> >         static inline bool gmid_el1_accessible(u64 pfr1)
> >         {
> >                 if (!IS_ENABLED(CONFIG_ARM64_MTE))
> >                         return false;
> >                 if (system_capabilities_finalized())
> >                         return system_supports_mte();
> >                 return id_aa64pfr1_mte(pfr1);
> >         }
> >
> >
> > TID5 is set from system_supports_mte() and only once capabilities are
> > finalised, so the local check is still correct before that. Gating on
> > system_supports_mte() keeps the different-physical-CPU case you want
> > to preserve working: if one re-onlines with MTE while the system view
> > has it folded off, the gate stays false and it won't read GMID_EL1.
>
> I still don't understand why you have system_supports_mte() here. If
> the CPU has MTE, the CONFIG option is enabled and the cmdline override
> isn't set, we can read GMID just fine on this CPU, regardless of the
> system capabilities.

I could gate on the local value with the override applied instead.

> > If you agree, I'll respin with that.
>
> I can't tell who's calling gmid_el1_accessible() so it's hard to tell
> whether I agree or not. May as well send a v3 though, so we can see what
> you have in mind!

Sending it now. Probably shorter than just describing it.

Cheers,
/fuad

> Will


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

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

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24  7:10 [PATCH v2] arm64: Defer the GMID_EL1 read to {init,update}_cpu_features() Fuad Tabba
2026-08-24 12:24 ` Catalin Marinas
2026-08-24 13:09 ` Will Deacon
2026-08-24 15:14   ` Fuad Tabba
2026-08-24 15:33     ` Catalin Marinas
2026-08-24 15:54     ` Will Deacon
2026-08-24 18:36       ` Fuad Tabba
2026-08-24 16:25   ` Catalin Marinas

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