On Mon, Dec 16, 2024 at 02:44:07PM +0000, Marc Zyngier wrote: > On Mon, 16 Dec 2024 14:31:47 +0000, > Mark Rutland <mark.rutland@xxxxxxx> wrote: > > > > On Mon, Dec 16, 2024 at 01:23:55PM +0000, Mark Brown wrote: > > > On Mon, Dec 16, 2024 at 12:44:14PM +0000, Mark Rutland wrote: > > > > > > > ... 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. > > > > > > Right, that was my best guess as to what was supposed to be going on > > > but it wasn't super clear. The code could use some more comments. > > > > > > > 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. > > > > > > That should be doable. > > > > > > There's a few other similar ID registers (eg, we already read GMID_EL1 > > > and MPAMIDR_EL1) make me a bit nervous that we might need to generalise > > > it a bit, but we can deal with that if it comes up. Even for SME the > > > disable was added speculatively, the factors that made this come up for > > > SVE are less likely to be an issue with SME. > > > > FWIW, I had a quick go (with only the SME case), and I think the shape > > that we want is roughly as below, which I think is easy to generalise to > > those other cases. > > > > MarcZ, thoughts? > > > > Mark. [... dodgy patch was here ...] > I don't think blindly applying the override at this stage is a good > thing. Specially not the whole register, and definitely not > non-disabling values. > > It needs to be done on a per feature basis, and only to disable > things. > > See the hack I posted for the things I think need checking. Understood; sorry for the noise -- we raced when replying and I only spotted your reply after sending this. I think I'm more in favour of the revert option now; I've repled with more details at: https://lore.kernel.org/linux-arm-kernel/Z2BCI61c9QWG7mMB@xxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#m8d6ace8d6201591ca72d51cf14c4a605e7d98a88 Mark.
