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

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

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
>