Re: [PATCH v5 08/37] KVM: arm64: Correctly access TCR2_EL1, PIR_EL1, PIRE0_EL1 with VHE

Joey Gouly
2024-10-24T10:03:42Z

On Wed, Oct 23, 2024 at 03:53:16PM +0100, Marc Zyngier wrote:
> For code that accesses any of the guest registers for emulation
> purposes, it is crucial to know where the most up-to-date data is.
>
> While this is pretty clear for nVHE (memory is the sole repository),
> things are a lot muddier for VHE, as depending on the SYSREGS_ON_CPU
> flag, registers can either be loaded on the HW or be in memory.
>
> Even worse with NV, where the loaded state is by definition partial.
>
> For these reasons, KVM offers the vcpu_read_sys_reg() and
> vcpu_write_sys_reg() primitives that always do the right thing.
> However, these primitive must know what register to access, and
> this is the role of the __vcpu_read_sys_reg_from_cpu() and
> __vcpu_write_sys_reg_to_cpu() helpers.
>
> As it turns out, TCR2_EL1, PIR_EL1, PIRE0_EL1 and not described
> in the latter helpers, meaning that the AT code cannot use them
> to emulate S1PIE.
>
> Add the three registers to the (long) list.
>
> Fixes: 86f9de9db178 ("KVM: arm64: Save/restore PIE registers")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: Joey Gouly <joey.gouly@arm.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 329619c6fa961..1adf68971bb17 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1030,6 +1030,9 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
> case TTBR0_EL1: *val = read_sysreg_s(SYS_TTBR0_EL12); break;
> case TTBR1_EL1: *val = read_sysreg_s(SYS_TTBR1_EL12); break;
> case TCR_EL1: *val = read_sysreg_s(SYS_TCR_EL12); break;
> + case TCR2_EL1: *val = read_sysreg_s(SYS_TCR2_EL12); break;
> + case PIR_EL1: *val = read_sysreg_s(SYS_PIR_EL12); break;
> + case PIRE0_EL1: *val = read_sysreg_s(SYS_PIRE0_EL12); break;
> case ESR_EL1: *val = read_sysreg_s(SYS_ESR_EL12); break;
> case AFSR0_EL1: *val = read_sysreg_s(SYS_AFSR0_EL12); break;
> case AFSR1_EL1: *val = read_sysreg_s(SYS_AFSR1_EL12); break;
> @@ -1076,6 +1079,9 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)
> case TTBR0_EL1: write_sysreg_s(val, SYS_TTBR0_EL12); break;
> case TTBR1_EL1: write_sysreg_s(val, SYS_TTBR1_EL12); break;
> case TCR_EL1: write_sysreg_s(val, SYS_TCR_EL12); break;
> + case TCR2_EL1: write_sysreg_s(val, SYS_TCR2_EL12); break;
> + case PIR_EL1: write_sysreg_s(val, SYS_PIR_EL12); break;
> + case PIRE0_EL1: write_sysreg_s(val, SYS_PIRE0_EL12); break;
> case ESR_EL1: write_sysreg_s(val, SYS_ESR_EL12); break;
> case AFSR0_EL1: write_sysreg_s(val, SYS_AFSR0_EL12); break;
> case AFSR1_EL1: write_sysreg_s(val, SYS_AFSR1_EL12); break;

Reviewed-by: Joey Gouly <joey.gouly@arm.com>