Re: [PATCH] arm64/sme: Move storage of reg_smidr to __cpuinfo_store_cpu()

From: Mark Rutland
Date: Mon Dec 16 2024 - 07:44:31 EST


On Mon, Dec 16, 2024 at 12:17:54PM +0000, Mark Brown wrote:
> On Sat, Dec 14, 2024 at 10:56:13AM +0000, Marc Zyngier wrote:
>
> > I don't understand the need to single out SMIDR_EL1. It seems to only
> > make things even more fragile than they already are by adding more
> > synchronisation phases.
>
> > Why isn't the following a good enough fix? It makes it plain that
> > boot_cpu_data is only a copy of CPU0's initial boot state.
>
> That would work but it's not clear to me that that is what the intent is
> here. The current ordering seemed like a strange enough decision to be
> deliberate, though I couldn't identify the reasoning.

The original intent was that __cpuinfo_store_cpu() read *all* of a CPU's
implemented ID regs, and init_cpu_features() initialised the expected
system features based on the boot CPU's ID regs.

The expectation was that init_cpu_features() would only consume the
register values, and would not alter the cpuinfo_arm64 values, so the
order of:

boot_cpu_data = *info;
init_cpu_features(&boot_cpu_data);

... didn't matter either way, and using '&boot_cpu_data' was intended to
make it clear that the features were based on the boot CPU's info, even
if you just grepped for that and didn't see the surrounding context.

I think the real fix here is to move the reading back into
__cpuinfo_store_cpu(), but to have an explicit check that SME has been
disabled on the commandline, with a comment explaining that this is a
bodge for broken FW which traps the SME ID regs.

Mark.