Re: [PATCH v2 2/3] KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests

[Marc Zyngier <maz@kernel.org>]

On Thu, 22 Aug 2024 00:35:37 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> When the guest does not support FEAT_TCR2 we should not allow any access
> to it in order to ensure that we do not create spurious issues with guest
> migration. Add a visibility operation for it.
> 
> Fixes: fbff56068232 ("KVM: arm64: Save/restore TCR2_EL1")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_host.h |  3 +++
>  arch/arm64/kvm/sys_regs.c         | 29 ++++++++++++++++++++++++++---
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ab4c675b491d..7889e5f4009f 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1476,4 +1476,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
>  		(pa + pi + pa3) == 1;					\
>  	})
>  
> +#define kvm_has_tcr2(k)				\
> +	(kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP))
> +
>  #endif /* __ARM64_KVM_HOST_H__ */
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1af15140e067..6d5f43781042 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2319,6 +2319,27 @@ static bool access_zcr_el2(struct kvm_vcpu *vcpu,
>  	return true;
>  }
>  
> +static unsigned int tcr2_visibility(const struct kvm_vcpu *vcpu,
> +				    const struct sys_reg_desc *rd)
> +{
> +	if (kvm_has_tcr2(vcpu->kvm))
> +		return 0;
> +
> +	return REG_HIDDEN;
> +}
> +
> +static unsigned int tcr2_el2_visibility(const struct kvm_vcpu *vcpu,
> +				    const struct sys_reg_desc *rd)
> +{
> +	unsigned int r;
> +
> +	r = el2_visibility(vcpu, rd);
> +	if (r)
> +		return r;
> +
> +	return tcr2_visibility(vcpu, rd);
> +}
> +
>  /*
>   * Architected system registers.
>   * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
> @@ -2503,7 +2524,8 @@ 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), access_vm_reg, reset_val, TCR2_EL1, 0 },
> +	{ SYS_DESC(SYS_TCR2_EL1), access_vm_reg, reset_val, TCR2_EL1, 0,
> +	  .visibility = tcr2_visibility },

With this, we should be able to simplify the accessor, shouldn't we?

>  
>  	PTRAUTH_KEY(APIA),
>  	PTRAUTH_KEY(APIB),
> @@ -2820,7 +2842,8 @@ 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(TCR2_EL2, access_tcr2_el2, reset_val, TCR2_EL2_RES1),
> +	EL2_REG_FILTERED(TCR2_EL2, access_tcr2_el2, reset_val, TCR2_EL2_RES1,
> +			 tcr2_el2_visibility),

Same thing here.

Thanks,

	M.

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