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

[Marc Zyngier <maz@kernel.org>]

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

> Can we just invoke the filter function again here to figure out if
> the field has been ignored? Then in match_options, we can just clear
> the override val/mask to zero.

The filter function isn't available outside of idreg-override.c:
that's where the per-field override structures are defined, and I'd
rather not expose that to the rest of the kernel.

Also, calling the filter implies that you parse the whole command-line
again, and you get into a real mess because the invalid override can
come from the expansion of an alias (e.g. 'kvm-arm.mode=nvhe'). Seems
totally overkill to me.

If, for some reason that I can't see at the moment, we need an extra
u64 to communicate that there is an invalid option, we can add that to
the override structure.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel