public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Fuad Tabba <tabba@google.com>
Cc: Will Deacon <will@kernel.org>,
	maz@kernel.org, oliver.upton@linux.dev, james.morse@arm.com,
	suzuki.poulose@arm.com, yuzenghui@huawei.com, qperret@google.com,
	vdonnefort@google.com, catalin.marinas@arm.com,
	linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH 1/8] KVM: arm64: Make EL2 exception entry and exit context-synchronization events
Date: Thu, 30 Apr 2026 13:37:42 +0100	[thread overview]
Message-ID: <afNNFtoZJk2IXwwU@J2N7QTR9R3> (raw)
In-Reply-To: <CA+EHjTw6rx5rCVnR7Dfva3xmmgGjqUeUaT=3zDCEsN0J909Wsg@mail.gmail.com>

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.


  reply	other threads:[~2026-04-30 12:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-28 10:30 [PATCH 0/8] KVM: arm64: EL2 synchronisation and pKVM stage-2 error propagation fixes Fuad Tabba
2026-04-28 10:30 ` [PATCH 1/8] KVM: arm64: Make EL2 exception entry and exit context-synchronization events Fuad Tabba
2026-04-30  2:22   ` Yao Yuan
2026-04-30  9:07   ` Will Deacon
2026-04-30 12:18     ` Fuad Tabba
2026-04-30 12:37       ` Mark Rutland [this message]
2026-04-28 10:30 ` [PATCH 2/8] KVM: arm64: Synchronise HCR_EL2 writes on the guest exit path Fuad Tabba
2026-04-28 13:50   ` Will Deacon
2026-04-28 14:21     ` Fuad Tabba
2026-04-28 10:30 ` [PATCH 3/8] KVM: arm64: Guard against NULL vcpu on VHE hyp panic path Fuad Tabba
2026-04-28 10:30 ` [PATCH 4/8] KVM: arm64: Fix __deactivate_fgt macro parameter typo Fuad Tabba
2026-04-28 10:30 ` [PATCH 5/8] KVM: arm64: Propagate stage-2 map failure on host->guest share Fuad Tabba
2026-04-28 10:30 ` [PATCH 6/8] KVM: arm64: Propagate stage-2 map failure on host->guest donation Fuad Tabba
2026-04-28 13:45   ` Will Deacon
2026-04-28 14:36     ` Fuad Tabba
2026-04-28 16:57       ` Will Deacon
2026-04-28 17:03         ` Fuad Tabba
2026-04-28 10:30 ` [PATCH 7/8] KVM: arm64: Propagate stage-2 map failure on guest->host share Fuad Tabba
2026-04-28 10:30 ` [PATCH 8/8] KVM: arm64: Propagate stage-2 map failure on guest->host unshare Fuad Tabba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afNNFtoZJk2IXwwU@J2N7QTR9R3 \
    --to=mark.rutland@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=qperret@google.com \
    --cc=stable@vger.kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=tabba@google.com \
    --cc=vdonnefort@google.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox