On Thu, 25 Mar 2021 19:27:59 +0000,
Will Deacon [off-list ref] wrote:

On Thu, Mar 25, 2021 at 12:47:20PM +0000, Marc Zyngier wrote:

quoted

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