[PATCH v5 29/39] arm64: Add helpers to probe local CPU for PAC and BTI support

[Ard Biesheuvel]

On Fri, 24 Nov 2023 at 13:37, Marc Zyngier <maz at kernel.org> wrote:
>
> On Fri, 24 Nov 2023 10:19:09 +0000,
> Ard Biesheuvel <ardb at google.com> wrote:
> >
> > From: Ard Biesheuvel <ardb at kernel.org>
> >
> > Add some helpers that will be used by the early kernel mapping code to
> > check feature support on the local CPU. This permits the early kernel
> > mapping to be created with the right attributes, removing the need for
> > tearing it down and recreating it.
> >
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > ---
> >  arch/arm64/include/asm/cpufeature.h | 44 ++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index 99c01417e544..301d4d2211d5 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -921,6 +921,50 @@ static inline bool kaslr_disabled_cmdline(void)
> >  u32 get_kvm_ipa_limit(void);
> >  void dump_cpu_features(void);
> >
> > +static inline bool cpu_has_bti(void)
> > +{
> > +     u64 pfr1;
> > +
> > +     if (!IS_ENABLED(CONFIG_ARM64_BTI))
> > +             return false;
> > +
> > +     pfr1 = read_cpuid(ID_AA64PFR1_EL1);
> > +     pfr1 &= ~id_aa64pfr1_override.mask;
> > +     pfr1 |= id_aa64pfr1_override.val;
>
> There is a potential gotcha here. If the override failed because a
> filter rejected it, val will be set to full ones for the size of the
> failed override field, and the corresponding mask will be set to 0
> (see match_options()).
>
> A safer pattern would be:
>
>         pfr1 |= id_aa64pfr1_override.val & id_aa64pfr1_override.mask;
>
> although we currently don't have such a filter for BTI nor PAC, so the
> code is OK for now.
>

This confuses me.

What is the point of a value/mask pair if the mask is applied to the
value, and not to the quantity that is being overridden? Surely, not
setting those bits in 'value' to begin with makes the whole mask
redundant, no?

If the filter is supposed to prevent the override from taking effect,
wouldn't it be better to use 0/0 for the mask/value pair?

(/me likely misses some context here)