[PATCH v2 08/19] KVM: arm64: Save/restore TCR2_EL1

[Joey Gouly]

Hi,

On Thu, Apr 20, 2023 at 10:13:38AM +0100, Marc Zyngier wrote:
> On Thu, 13 Apr 2023 12:05:02 +0100,
> Joey Gouly <joey.gouly at arm.com> wrote:
> > 
> > Define the new system register TCR2_EL1 and context switch it.
> > 
> > Signed-off-by: Joey Gouly <joey.gouly at arm.com>
> > Cc: Marc Zyngier <maz at kernel.org>
> > Cc: Oliver Upton <oliver.upton at linux.dev>
> > Cc: James Morse <james.morse at arm.com>
> > Cc: Suzuki K Poulose <suzuki.poulose at arm.com>
> > Cc: Zenghui Yu <yuzenghui at huawei.com>
> > Cc: Catalin Marinas <catalin.marinas at arm.com>
> > Cc: Will Deacon <will at kernel.org>
> > Reviewed-by: Catalin Marinas <catalin.marinas at arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_host.h          | 1 +
> >  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 4 ++++
> >  arch/arm64/kvm/sys_regs.c                  | 1 +
> >  3 files changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index bcd774d74f34..e1137832a01f 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -269,6 +269,7 @@ enum vcpu_sysreg {
> >  	TTBR0_EL1,	/* Translation Table Base Register 0 */
> >  	TTBR1_EL1,	/* Translation Table Base Register 1 */
> >  	TCR_EL1,	/* Translation Control Register */
> > +	TCR2_EL1,	/* Extended Translation Control Register */
> >  	ESR_EL1,	/* Exception Syndrome Register */
> >  	AFSR0_EL1,	/* Auxiliary Fault Status Register 0 */
> >  	AFSR1_EL1,	/* Auxiliary Fault Status Register 1 */
> > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > index 699ea1f8d409..16199a107a47 100644
> > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > @@ -44,6 +44,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> >  	ctxt_sys_reg(ctxt, TTBR0_EL1)	= read_sysreg_el1(SYS_TTBR0);
> >  	ctxt_sys_reg(ctxt, TTBR1_EL1)	= read_sysreg_el1(SYS_TTBR1);
> >  	ctxt_sys_reg(ctxt, TCR_EL1)	= read_sysreg_el1(SYS_TCR);
> > +	if (cpus_have_final_cap(ARM64_HAS_TCR2))
> > +		ctxt_sys_reg(ctxt, TCR2_EL1)	= read_sysreg_el1(SYS_TCR2);
> >  	ctxt_sys_reg(ctxt, ESR_EL1)	= read_sysreg_el1(SYS_ESR);
> >  	ctxt_sys_reg(ctxt, AFSR0_EL1)	= read_sysreg_el1(SYS_AFSR0);
> >  	ctxt_sys_reg(ctxt, AFSR1_EL1)	= read_sysreg_el1(SYS_AFSR1);
> > @@ -114,6 +116,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
> >  	write_sysreg_el1(ctxt_sys_reg(ctxt, CPACR_EL1),	SYS_CPACR);
> >  	write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR0_EL1),	SYS_TTBR0);
> >  	write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR1_EL1),	SYS_TTBR1);
> > +	if (cpus_have_final_cap(ARM64_HAS_TCR2))
> > +		write_sysreg_el1(ctxt_sys_reg(ctxt, TCR2_EL1),	SYS_TCR2);
> >  	write_sysreg_el1(ctxt_sys_reg(ctxt, ESR_EL1),	SYS_ESR);
> >  	write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR0_EL1),	SYS_AFSR0);
> >  	write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR1_EL1),	SYS_AFSR1);
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 53749d3a0996..5e7e4a433035 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1871,6 +1871,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> >  	{ SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 },
> >  	{ SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 },
> >  	{ SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 },
> > +	{ SYS_DESC(SYS_TCR2_EL1), NULL, reset_unknown, TCR2_EL1 },
> 
> I'm not convinced reset_unknown is the right thing, at least for the
> bits that are defined as "If EL2 and EL3 is not implemented, this bit
> resets to 0b0 on a reset."
> 
> Given that an EL1 guest isn't in control of EL2, I'm a bit wary that
> we start execution of the guest in a context that isn't well defined.
> My strong preference would be to reset TCR2 just like TCR, unless you
> can provide a explanation of why UNKNOWN is actually more correct.


You're right, I have changed this to reset to 0 like TCR_EL1.

Thanks,
Joey