[PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[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, trapping the read to EL2, where pKVM injects
an UNDEF the host cannot handle:

  Internal error: Oops - Undefined instruction: 0000000002000000 [#1]
  pc : __cpuinfo_store_cpu+0xf4/0x264
  Kernel panic - not syncing: Attempted to kill the idle task!

Only pKVM is affected, and only on a CPU that is offlined and brought
back online: a first bring-up either precedes KVM's initcall or is
refused by pKVM's CPU_ON relay. That relay sets the host HCR before the
CPU enters EL1, whereas plain nVHE sets it at CPUHP_AP_KVM_ONLINE,
after cpuinfo_store_cpu().

Gate the read on ID_AA64PFR1_EL1 with the cmdline override applied, and
on CONFIG_ARM64_MTE, which no register value reflects. That leaves the
SYS_GMID_EL1 feature register uninitialised when MTE is off, and it has
no readers.

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 v2:
    - Keep the GMID_EL1 read in __cpuinfo_store_cpu() and gate it on the
      local ID_AA64PFR1_EL1 with the cmdline override applied, rather than
      deferring the read to {init,update}_cpu_features() (Will). This also
      keeps reg_gmid set in cpu_data[0], which v2 dropped (Catalin).
    
    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/include/asm/cpufeature.h | 15 +++++++++++++++
 arch/arm64/kernel/cpufeature.c      |  6 ++----
 arch/arm64/kernel/cpuinfo.c         |  2 +-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index a57870fa96db5..5bb242721e4b9 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -1085,6 +1085,21 @@ static inline bool cpu_has_lpa2(void)
 #endif
 }
 
+/*
+ * Reading GMID_EL1 when the kernel has disabled MTE traps to EL2, and the raw
+ * ID_AA64PFR1_EL1 reflects neither CONFIG_ARM64_MTE nor the cmdline override.
+ */
+static inline bool gmid_el1_accessible(u64 pfr1)
+{
+	if (!IS_ENABLED(CONFIG_ARM64_MTE))
+		return false;
+
+	pfr1 &= ~id_aa64pfr1_override.mask;
+	pfr1 |= id_aa64pfr1_override.val;
+
+	return id_aa64pfr1_mte(pfr1);
+}
+
 #endif /* __ASSEMBLER__ */
 
 #endif
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9a22df0c5120f..4b339346f6c78 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1228,7 +1228,7 @@ 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 (gmid_el1_accessible(info->reg_id_aa64pfr1))
 		init_cpu_ftr_reg(SYS_GMID_EL1, info->reg_gmid);
 }
 
@@ -1490,11 +1490,9 @@ 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 (gmid_el1_accessible(info->reg_id_aa64pfr1))
 		taint |= check_update_ftr_reg(SYS_GMID_EL1, cpu,
 					      info->reg_gmid, boot->reg_gmid);
-	}
 
 	/*
 	 * If we don't have AArch32 at all then skip the checks entirely
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index d50e2a9b066b3..52a161fc38250 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -502,7 +502,7 @@ 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))
+	if (gmid_el1_accessible(info->reg_id_aa64pfr1))
 		info->reg_gmid = read_cpuid(GMID_EL1);
 
 	if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
-- 
2.39.5

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Kernel panic on non-MTE hardware caused by unsanitized command-line overrides in gmid_el1_accessible().
--

--- Patch [1]: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled ---
commit ff2144eb5268c6bf4e244c7063fffca15d574919
Author: Fuad Tabba <fuad.tabba@linux.dev>

arm64: Don't read GMID_EL1 when MTE is disabled

This patch prevents __cpuinfo_store_cpu() from reading the GMID_EL1 register
when MTE is disabled, avoiding a host panic when pKVM injects an UNDEF. It
introduces a new helper to check if MTE is accessible by evaluating both
CONFIG_ARM64_MTE and the raw ID_AA64PFR1_EL1 combined with command-line
overrides.

Cc: stable@vger.kernel.org

> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index d90040fb9de6c..aaf267c7eb243 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
[ ... ]
> @@ -1085,6 +1085,21 @@ static inline bool cpu_has_lpa2(void)
>  #endif
>  }
>  
> +/*
> + * Reading GMID_EL1 when the kernel has disabled MTE traps to EL2, and the raw
> + * ID_AA64PFR1_EL1 reflects neither CONFIG_ARM64_MTE nor the cmdline override.
> + */
> +static inline bool gmid_el1_accessible(u64 pfr1)
> +{
> +	if (!IS_ENABLED(CONFIG_ARM64_MTE))
> +		return false;
> +
> +	pfr1 &= ~id_aa64pfr1_override.mask;
> +	pfr1 |= id_aa64pfr1_override.val;
> +
> +	return id_aa64pfr1_mte(pfr1);
> +}

[Severity: High]
Does this manual application of the command-line override bypass the standard
feature sanitization?

If a user specifies a command-line override to explicitly enable MTE on
hardware that does not natively support it (e.g., id_aa64pfr1.mte=1), this
code applies the raw id_aa64pfr1_override directly.

Normally, the cpufeature framework uses arm64_ftr_safe_value() to drop unsafe
overrides that attempt to enable unimplemented hardware features. Because this
function skips that sanitization, it will return true for an invalid override,
leading to an UNDEF exception and kernel panic when __cpuinfo_store_cpu()
subsequently attempts to read the unallocated GMID_EL1 register.

Could this check be adjusted to use a sanitized value or ensure unsafe
overrides are safely ignored?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824184155.2644646-1-fuad.tabba@linux.dev?part=1

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Fuad Tabba]

Sashiko, I managed to elude you in V1 and V2, but you got me...

On Mon, 24 Aug 2026 at 19:56, <sashiko-bot@kernel.org> wrote:
...

> [Severity: High]
> Does this manual application of the command-line override bypass the standard
> feature sanitization?

I'm afraid so, sashiko... Guarding on the unmodified register should avoid it:

        if (!IS_ENABLED(CONFIG_ARM64_MTE))
            return false;
        if (!id_aa64pfr1_mte(pfr1))
            return false;
        pfr1 &= ~id_aa64pfr1_override.mask;
        pfr1 |= id_aa64pfr1_override.val;
        return id_aa64pfr1_mte(pfr1);

Let's first see what the others think before I respin anything...

Cheers,
/fuad




>
> If a user specifies a command-line override to explicitly enable MTE on
> hardware that does not natively support it (e.g., id_aa64pfr1.mte=1), this
> code applies the raw id_aa64pfr1_override directly.
>
> Normally, the cpufeature framework uses arm64_ftr_safe_value() to drop unsafe
> overrides that attempt to enable unimplemented hardware features. Because this
> function skips that sanitization, it will return true for an invalid override,
> leading to an UNDEF exception and kernel panic when __cpuinfo_store_cpu()
> subsequently attempts to read the unallocated GMID_EL1 register.
>
> Could this check be adjusted to use a sanitized value or ensure unsafe
> overrides are safely ignored?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260824184155.2644646-1-fuad.tabba@linux.dev?part=1

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Catalin Marinas]

On Mon, Aug 24, 2026 at 07:41:55PM +0100, Fuad Tabba wrote:
> +/*
> + * Reading GMID_EL1 when the kernel has disabled MTE traps to EL2, and the raw
> + * ID_AA64PFR1_EL1 reflects neither CONFIG_ARM64_MTE nor the cmdline override.
> + */
> +static inline bool gmid_el1_accessible(u64 pfr1)
> +{
> +	if (!IS_ENABLED(CONFIG_ARM64_MTE))
> +		return false;
> +
> +	pfr1 &= ~id_aa64pfr1_override.mask;
> +	pfr1 |= id_aa64pfr1_override.val;
> +
> +	return id_aa64pfr1_mte(pfr1);
> +}

Can this function not use __read_sysreg_by_encoding() directly and not
get an argument at all? We'd get the override from that function rather
than open-coding it here. Eventually, once we get Suzuki's clamping fix,
it also ensures we won't be able to bump the MTE version to unsafe
values (but that's a different fix from this one).

If you are worried about additional trapping of the MRS, we should go
for Will's improvement to always read the cached values in
__read_sysreg_by_encoding().

-- 
Catalin

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Fuad Tabba]

On Mon, 24 Aug 2026 at 21:29, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, Aug 24, 2026 at 07:41:55PM +0100, Fuad Tabba wrote:
> > +/*
> > + * Reading GMID_EL1 when the kernel has disabled MTE traps to EL2, and the raw
> > + * ID_AA64PFR1_EL1 reflects neither CONFIG_ARM64_MTE nor the cmdline override.
> > + */
> > +static inline bool gmid_el1_accessible(u64 pfr1)
> > +{
> > +     if (!IS_ENABLED(CONFIG_ARM64_MTE))
> > +             return false;
> > +
> > +     pfr1 &= ~id_aa64pfr1_override.mask;
> > +     pfr1 |= id_aa64pfr1_override.val;
> > +
> > +     return id_aa64pfr1_mte(pfr1);
> > +}
>
> Can this function not use __read_sysreg_by_encoding() directly and not
> get an argument at all? We'd get the override from that function rather
> than open-coding it here.

Like this?

      static inline bool gmid_el1_accessible(void)
      {
              if (!IS_ENABLED(CONFIG_ARM64_MTE))
                      return false;

              return
id_aa64pfr1_mte(__read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1));
      }

> Eventually, once we get Suzuki's clamping fix,
> it also ensures we won't be able to bump the MTE version to unsafe
> values (but that's a different fix from this one).

Agreed it's separate, but v3 applies the override without checking
that the CPU has MTE2, so id_aa64pfr1.mte=2 on a CPU without it reads
a GMID_EL1 that isn't there. On the boot CPU that's before the
override is sanitised. Should I check the raw register before applying
the override, until the clamp lands?

>
> If you are worried about additional trapping of the MRS, we should go
> for Will's improvement to always read the cached values in
> __read_sysreg_by_encoding().

Not worried :) __cpuinfo_store_cpu() has already read ID_AA64PFR1_EL1
a few lines up.

Cheers,
/fuad


>
> --
> Catalin

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Will Deacon]

On Mon, Aug 24, 2026 at 11:29:54PM +0100, Fuad Tabba wrote:
> On Mon, 24 Aug 2026 at 21:29, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Aug 24, 2026 at 07:41:55PM +0100, Fuad Tabba wrote:
> > > +/*
> > > + * Reading GMID_EL1 when the kernel has disabled MTE traps to EL2, and the raw
> > > + * ID_AA64PFR1_EL1 reflects neither CONFIG_ARM64_MTE nor the cmdline override.
> > > + */
> > > +static inline bool gmid_el1_accessible(u64 pfr1)
> > > +{
> > > +     if (!IS_ENABLED(CONFIG_ARM64_MTE))
> > > +             return false;
> > > +
> > > +     pfr1 &= ~id_aa64pfr1_override.mask;
> > > +     pfr1 |= id_aa64pfr1_override.val;
> > > +
> > > +     return id_aa64pfr1_mte(pfr1);
> > > +}
> >
> > Can this function not use __read_sysreg_by_encoding() directly and not
> > get an argument at all? We'd get the override from that function rather
> > than open-coding it here.
> 
> Like this?
> 
>       static inline bool gmid_el1_accessible(void)
>       {
>               if (!IS_ENABLED(CONFIG_ARM64_MTE))
>                       return false;
> 
>               return
> id_aa64pfr1_mte(__read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1));
>       }
> 
> > Eventually, once we get Suzuki's clamping fix,
> > it also ensures we won't be able to bump the MTE version to unsafe
> > values (but that's a different fix from this one).
> 
> Agreed it's separate, but v3 applies the override without checking
> that the CPU has MTE2, so id_aa64pfr1.mte=2 on a CPU without it reads
> a GMID_EL1 that isn't there. On the boot CPU that's before the
> override is sanitised. Should I check the raw register before applying
> the override, until the clamp lands?

At which point, why don't we bite the bullet and implement Suzuki's
idea so that __read_sysreg_by_encoding() does what you want more generally?

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

> > If you are worried about additional trapping of the MRS, we should go
> > for Will's improvement to always read the cached values in
> > __read_sysreg_by_encoding().
> 
> Not worried :) __cpuinfo_store_cpu() has already read ID_AA64PFR1_EL1
> a few lines up.

But if you go with __read_sysreg_by_encoding() then you'd still be
reading it (and trapping) twice, no?

Will

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Fuad Tabba]

On Tue, 25 Aug 2026 at 15:14, Will Deacon <will@kernel.org> wrote:
...

> > Agreed it's separate, but v3 applies the override without checking
> > that the CPU has MTE2, so id_aa64pfr1.mte=2 on a CPU without it reads
> > a GMID_EL1 that isn't there. On the boot CPU that's before the
> > override is sanitised. Should I check the raw register before applying
> > the override, until the clamp lands?
>
> At which point, why don't we bite the bullet and implement Suzuki's
> idea so that __read_sysreg_by_encoding() does what you want more generally?
>
> https://lore.kernel.org/all/afc5bd00-28ca-413b-b047-ee53589c285d@arm.com/

v4 will be two patches: Suzuki's clamp first, then the GMID_EL1 gate
on __read_sysreg_by_encoding().

>
> > > If you are worried about additional trapping of the MRS, we should go
> > > for Will's improvement to always read the cached values in
> > > __read_sysreg_by_encoding().
> >
> > Not worried :) __cpuinfo_store_cpu() has already read ID_AA64PFR1_EL1
> > a few lines up.
>
> But if you go with __read_sysreg_by_encoding() then you'd still be
> reading it (and trapping) twice, no?

It would, and I'm fine with that on a CPU bring-up path.

Stay tuned!
/fuad
>
> Will

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Catalin Marinas]

On Tue, Aug 25, 2026 at 03:46:15PM +0100, Fuad Tabba wrote:
> On Tue, 25 Aug 2026 at 15:14, Will Deacon <will@kernel.org> wrote:
> ...
> > > > If you are worried about additional trapping of the MRS, we should go
> > > > for Will's improvement to always read the cached values in
> > > > __read_sysreg_by_encoding().
> > >
> > > Not worried :) __cpuinfo_store_cpu() has already read ID_AA64PFR1_EL1
> > > a few lines up.
> >
> > But if you go with __read_sysreg_by_encoding() then you'd still be
> > reading it (and trapping) twice, no?
> 
> It would, and I'm fine with that on a CPU bring-up path.

For backports, yes. Going forward, I like Will's idea of reading the
cached value in __read_sysreg_by_encoding().

-- 
Catalin

Re: [PATCH v3] arm64: Don't read GMID_EL1 when MTE is disabled

[Will Deacon]

On Wed, Aug 26, 2026 at 12:06:26PM +0100, Catalin Marinas wrote:
> On Tue, Aug 25, 2026 at 03:46:15PM +0100, Fuad Tabba wrote:
> > On Tue, 25 Aug 2026 at 15:14, Will Deacon <will@kernel.org> wrote:
> > ...
> > > > > If you are worried about additional trapping of the MRS, we should go
> > > > > for Will's improvement to always read the cached values in
> > > > > __read_sysreg_by_encoding().
> > > >
> > > > Not worried :) __cpuinfo_store_cpu() has already read ID_AA64PFR1_EL1
> > > > a few lines up.
> > >
> > > But if you go with __read_sysreg_by_encoding() then you'd still be
> > > reading it (and trapping) twice, no?
> > 
> > It would, and I'm fine with that on a CPU bring-up path.
> 
> For backports, yes. Going forward, I like Will's idea of reading the
> cached value in __read_sysreg_by_encoding().

I'll try to incorporate that on the end of my parallel hotplug series,
which I intend to post next week. I just need to untangle the mpam stuff,
as it populates the cpuinfo structure far too late.

Will