[PATCH] KVM: arm64: Writable TGRAN*_2

[PATCH] KVM: arm64: Writable TGRAN*_2

[Sebastian Ott]

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>
---
 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),
-- 
2.42.0

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

[Marc Zyngier]

On Thu, 06 Mar 2025 18:40:13 +0000,
Sebastian Ott <sebott@redhat.com> 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>

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

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

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

[Oliver Upton]

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

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

[Oliver Upton]

On Wed, Mar 12, 2025 at 10:20:03AM -0700, Oliver Upton wrote:
> 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.

/me finishes a cup of coffee

The conflict is obviously with 9d6745572899 ("KVM: arm64: Hide
ID_AA64MMFR2_EL1.NV from guest and userspace"), which is in
kvmarm/next...

> > ---
> >  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
> 

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

[Sebastian Ott]

Hi Oliver,

On Wed, 12 Mar 2025, Oliver Upton wrote:
> 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.

Ok, I'll keep it in mind for next time.
(It was kvm-arm64/next + 2 pkvm series from the list that I applied
manually.)

> 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!
Sebastian

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

[Oliver Upton]

On Thu, 06 Mar 2025 19:40:13 +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.
> 
> 

Applied to next, thanks!

[1/1] KVM: arm64: Writable TGRAN*_2
      https://git.kernel.org/kvmarm/kvmarm/c/edfd826b8be7

--
Best,
Oliver