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

On Mon, Aug 24, 2026 at 11:29:54PM +0100, Fuad Tabba wrote:
> On Mon, 24 Aug 2026 at 21:29, Catalin Marinas <[email protected]> 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/[email protected]/

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