Re: [PATCH v1 2/3] arm64: sysreg: Add TCR_EL2 to sysreg infrastructure

[Marc Zyngier <maz@kernel.org>]

On Thu, 10 Sep 2026 11:58:07 +0100,
Wei-Lin Chang <weilin.chang@arm.com> wrote:
> 
> On Wed, Sep 09, 2026 at 10:38:10PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] Declarative update to TCR_EL2 regenerates the TCR_EL2_RES1 aggregate mask which is consumed by C initialization, requiring confirmation of robustness to future value changes.
> > --
> > 
> > commit 522a184d95be9c0f88cc6fb418668546ef83e3e4
> > Author: Wei-Lin Chang <weilin.chang@arm.com>
> > 
> >     arm64: sysreg: Add TCR_EL2 to sysreg infrastructure
> >     
> >     Convert TCR_EL2 to the sysreg infrastructure with data extracted from
> >     Registers.json of the BSD licenced AARCHMRS
> >     (AARCHMRS_OPENSOURCE_A_profile_FAT-2026-06).
> > 
> > > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> > > index 94bf065c8ac71..8d89f34580bac 100644
> > > --- a/arch/arm64/tools/sysreg
> > > +++ b/arch/arm64/tools/sysreg
> > [ ... ]
> > > +Sysreg	TCR_EL2		3	4	2	0	2
> > [ ... ]
> > > +Field	33	MTX
> > > +Field	32	DS
> > > +Res1	31
> > > +Field	30	TCMA
> > > +Field	29	TBID
> > > +Field	28	HWU62
> > > +Field	27	HWU61
> > > +Field	26	HWU60
> > > +Field	25	HWU59
> > > +Field	24	HPD
> > > +Res1	23
> > > +Field	22	HD
> > 
> > [Severity: Medium]
> > This isn't a bug, but this declarative update regenerates the TCR_EL2_RES1
> > aggregate mask. Are the C initialization consumers of this mask robust to its
> > value silently changing if a future .sysreg patch adds or removes a Res1 bit?
> > 
> > For example, the generated mask is OR'd into the initial EL2 register value:
> > 
> > arch/arm64/kvm/arm.c:cpu_prepare_hyp_mode() {
> >     ...
> >     tcr |= TCR_EL2_RES1 | FIELD_PREP(TCR_EL2_PS_MASK, ips);
> >     ...
> > }

Nonsense. In this case, we run nVHE, and we use the !VHE
definition. The hVHE path correctly derives the setting from TCR_EL1,
which has the same format.

> > 
> > It is also used to initialize the guest emulation reset state in:
> > 
> > arch/arm64/kvm/sys_regs.c:sys_reg_descs[] = {
> >     ...
> >     EL2_REG(TCR_EL2, access_rw, reset_val, TCR_EL2_RES1),
> >     ...
> > }

This one is a potential bug, but not in the current revision of the
architecture. Bits 23 and 31 are RES1 when E2H==0, but reset as
UNKNOWN when E2H==1, so nothing breaks. This is just ugly.

> > 
> > Because the generator's contract is to reflect global architectural truth, a
> > future edit that reclassifies a bit as Res1 will silently change the value of
> > TCR_EL2_RES1 without triggering a compiler or CI warning. Could you confirm
> > that these consumers do not rely on the previous static semantic value of the
> > mask?
> 
> I don't fully understand but doesn't this mean the architecture
> introduced a change that breaks existing software?

That's exactly what VHE (and by extension HCR_EL2.E2H being RES1) did.
Which is why I can't wait for FEAT_E2H0 to be eradicated (it is
already forbidden in 9.6 as FEAT_SRMASK is mandatory).

I think this is simply a case of providing a specific reset function
for TCR_EL2 (untested).

	M.

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 2c90e185c6e8e..964ecbfde9bb6 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3275,6 +3275,14 @@ static u64 reset_mdcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 	return vcpu->kvm->arch.nr_pmu_counters;
 }
 
+static u64 reset_tcr_el2(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
+{
+	__vcpu_assign_sys_reg(vcpu, r->reg,
+			      vcpu_has_feature(vcpu, KVM_ARM_VCPU_HAS_EL2_E2H0) ?
+			      TCR_EL2_RES1 : VHE_TCR_EL2_RES1);
+	return __vcpu_sys_reg(vcpu, r->reg);
+}
+
 /*
  * Architected system registers.
  * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
@@ -3853,7 +3861,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 
 	EL2_REG(TTBR0_EL2, access_rw, reset_val, 0),
 	EL2_REG(TTBR1_EL2, access_rw, reset_val, 0),
-	EL2_REG(TCR_EL2, access_rw, reset_val, TCR_EL2_RES1),
+	EL2_REG(TCR_EL2, access_rw, reset_tcr_el2, 0),
 	EL2_REG_FILTERED(TCR2_EL2, access_rw, reset_val, TCR2_EL2_RES1,
 			 tcr2_el2_visibility),
 	EL2_REG_VNCR(VTTBR_EL2, reset_val, 0),

-- 
Without deviation from the norm, progress is not possible.