Re: [PATCH] KVM: arm64: Writable TGRAN*_2

[Oliver Upton <oliver.upton@linux.dev>]

Hi Sebastian,

On Thu, Mar 06, 2025 at 07:40:13PM +0100, Sebastian Ott wrote:
> Allow userspace to write the safe (NI) value for ID_AA64MMFR0_EL1.TGRAN*_2.
> Disallow to change these fields for NV since kvm provides a sanitized view
> for them based on the PAGE_SIZE.
> Also add these bits to the set_id_regs selftest.
> 
> Signed-off-by: Sebastian Ott <sebott@redhat.com>

I can't tell what you've based this patch on, it certainly doesn't apply
on a 6.14 rc. Consider telling git to include the base commit next time
you generate a patch.

> ---
>  arch/arm64/kvm/sys_regs.c                     | 21 +++++++++++++++----
>  .../testing/selftests/kvm/arm64/set_id_regs.c |  3 +++
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 14faf213d483..0730ed8314d0 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1948,6 +1948,21 @@ static int set_id_aa64pfr1_el1(struct kvm_vcpu *vcpu,
>  	return set_id_reg(vcpu, rd, user_val);
>  }
>  
> +static int set_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
> +				const struct sys_reg_desc *rd, u64 user_val)
> +{
> +	u64 sanitized_val = kvm_read_sanitised_id_reg(vcpu, rd);
> +	u64 tgran2_mask = ID_AA64MMFR0_EL1_TGRAN4_2_MASK |
> +			  ID_AA64MMFR0_EL1_TGRAN16_2_MASK |
> +			  ID_AA64MMFR0_EL1_TGRAN64_2_MASK;
> +
> +	if (vcpu_has_nv(vcpu) &&
> +	    ((sanitized_val & tgran2_mask) != (user_val & tgran2_mask)))
> +		return -EINVAL;
> +
> +	return set_id_reg(vcpu, rd, user_val);
> +}
> +
>  static int set_id_aa64mmfr2_el1(struct kvm_vcpu *vcpu,
>  				const struct sys_reg_desc *rd, u64 user_val)
>  {
> @@ -2787,10 +2802,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	ID_UNALLOCATED(6,7),
>  
>  	/* CRm=7 */
> -	ID_WRITABLE(ID_AA64MMFR0_EL1, ~(ID_AA64MMFR0_EL1_RES0 |
> -					ID_AA64MMFR0_EL1_TGRAN4_2 |
> -					ID_AA64MMFR0_EL1_TGRAN64_2 |
> -					ID_AA64MMFR0_EL1_TGRAN16_2 |
> +	ID_FILTERED(ID_AA64MMFR0_EL1, id_aa64mmfr0_el1,
> +				      ~(ID_AA64MMFR0_EL1_RES0 |
>  					ID_AA64MMFR0_EL1_ASIDBITS)),
>  	ID_WRITABLE(ID_AA64MMFR1_EL1, ~(ID_AA64MMFR1_EL1_RES0 |
>  					ID_AA64MMFR1_EL1_HCX |
> diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> index 1d65f4a09e6f..322b9d3b0125 100644
> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> @@ -146,6 +146,9 @@ static const struct reg_ftr_bits ftr_id_aa64pfr1_el1[] = {
>  static const struct reg_ftr_bits ftr_id_aa64mmfr0_el1[] = {
>  	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR0_EL1, ECV, 0),
>  	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR0_EL1, EXS, 0),
> +	REG_FTR_BITS(FTR_EXACT, ID_AA64MMFR0_EL1, TGRAN4_2, 1),
> +	REG_FTR_BITS(FTR_EXACT, ID_AA64MMFR0_EL1, TGRAN64_2, 1),
> +	REG_FTR_BITS(FTR_EXACT, ID_AA64MMFR0_EL1, TGRAN16_2, 1),
>  	S_REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR0_EL1, TGRAN4, 0),
>  	S_REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR0_EL1, TGRAN64, 0),
>  	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR0_EL1, TGRAN16, 0),

Please do selftests changes in a separate patch.

Don't worry about respinning, I'll fix this up and queue it in a moment.

Thanks,
Oliver