Re: [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override

Will Deacon
2021-03-29T18:03:57Z

On Fri, Mar 26, 2021 at 10:56:23AM +0000, Marc Zyngier wrote:
> On Thu, 25 Mar 2021 19:27:59 +0000,
> Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, Mar 25, 2021 at 12:47:20PM +0000, Marc Zyngier wrote:
> > > Some CPUs are broken enough that some overrides need to be rejected
> > > at the earliest opportunity. In some cases, that's right at cpu
> > > feature override time.
> > >
> > > Provide the necessary infrastructure to filter out overrides,
> > > and to report such filtered out overrides to the core cpufeature code.
> > >
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > > arch/arm64/kernel/cpufeature.c | 6 ++++++
> > > arch/arm64/kernel/idreg-override.c | 13 +++++++++++++
> > > 2 files changed, 19 insertions(+)
> > >
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index 066030717a4c..6de15deaa912 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -809,6 +809,12 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
> > > reg->name,
> > > ftrp->shift + ftrp->width - 1,
> > > ftrp->shift, str, tmp);
> > > + } else if ((ftr_mask & reg->override->val) == ftr_mask) {
> >
> > This seems to rely on 'val == mask' being invalid, but I'm not sure why
> > that's generally true.
>
> This is really 'ovr->val == mask && ovr->mask != mask', thanks to
> being on the 'else' branch. The encoding rules of val/mask are, for a
> given field:
>
> - no override set: mask = 0, val = 0
> - valid override set: mask = 0xf, val = (override value)
> - invalid override set: mask = 0, val = 0xf
>
> I don't see where the ambiguity could be (though the above could
> figure in a comment to make things clearer).

Thanks, this makes sense to me now; I'd missed the other part of the
conditional. So yeah, with that comment added:

Acked-by: Will Deacon <will@kernel.org>

Will