Re: [PATCH 1/8] KVM: arm64: Make EL2 exception entry and exit context-synchronization events

Mark Rutland
2026-04-30T12:37:50Z

On Thu, Apr 30, 2026 at 01:18:48PM +0100, Fuad Tabba wrote:
> On Thu, 30 Apr 2026 at 10:08, Will Deacon <will@kernel.org> wrote:
> > On Tue, Apr 28, 2026 at 11:30:01AM +0100, Fuad Tabba wrote:
> > > Fixes: fe2c8d19189e ("KVM: arm64: Turn SCTLR_ELx_FLAGS into INIT_SCTLR_EL2_MMU_ON")
> >
> > I don't think this Fixes: tag is accurate:
> >
> > 1. That commit doesn't do anything with EIS/EOS afaict.
> > 2. Back in 5.12 (when that thing landed), SCTLR_EL2_RES1 did actually
> > include EIS and EOS
> >
> > so I think the issue here might be that the auto-generated sysreg file
> > quietly changes the RES1 definitions as bits get allocated, but the
> > macros using the RES1 definition don't get updated. That's a pretty
> > horrible pit that it feels like we might keep falling into :/

I think that's a review failure, and people need to be careful when
updating the sysreg file (e.g. looking at whether any new bits were
previously RESx, and considering the impact). Regardless of tooling, we
need people to conciosuly review that.

> > Looking at 0a35bd285f43 ("arm64: Convert SCTLR_EL2 to sysreg
> > infrastructure"), I think we ended up dropping a whole bunch of fields
> > from the RES1 mask (which became 0!). Have you checked all of those?

> On the wider question of the other bits dropped from the old mask,
> I went through them against DDI 0487 M.b §D24.2.175. The summary
> (SCTLR_EL2 with E2H=0):
>
> bit field E2H=0 status kernel cares?
> -------------------------------------------------------------
> 4 SA0 RES1 unconditionally no
> 5 CP15BEN RES1 unconditionally no
> 11 EOS RES1 iff !FEAT_ExS, else RW yes (this fix)
> 16 nTWI RES1 unconditionally no
> 18 nTWE RES1 unconditionally no
> 22 EIS RES1 iff !FEAT_ExS, else RW yes (this fix)
> 23 SPAN RES1 unconditionally no
> 28 nTLSMD RES1 unconditionally no
> 29 LSMAOE RES1 unconditionally no
>
> The seven non-EIS/EOS bits all fall under the "Otherwise: Reserved,
> RES1" clause for the E2H=0 layout, with no feature guard. Writing 0
> to them is a no-op, so dropping them from the mask should be harmless
> I think. EIS and EOS are the only positions where the bit
> becomes RW (with UNKNOWN reset) on FEAT_ExS hardware and the
> kernel actively relies on the value being 1, which is what this
> patch addresses.
>
> I agree the auto-generator silently zeroing previously hand-rolled
> RES1 masks is a real problem. Happy to look at either teaching the
> sysreg infrastructure to express conditional RES1 (so config.c's
> AS_RES1/FEAT_X facts can flow back into the header masks),

Please don't add that to the syreg code; that's deliberately *only* the
architectural definitions, and overloading that is going to make things
even more confusing, because "I want to treat this as RESx in this piece
of code" isn't a global property.

> or at least adding a build-time check that flags any auto-generated
> <REG>_RES1 that shrinks. After this series, though. Let me know if
> you'd like me to take a stab.

FWIW, having tooling to compare before/after would be useful, but I
don't think that can be a standard build-time check, given that this
would depend on having the old and new sysreg files available for
comparison.

Mark.