| Message ID | 20241004110714.2051604-7-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Hide unsupported MPAM from the guest | expand |
On 10/4/24 9:07 PM, Joey Gouly wrote: > From: James Morse <james.morse@arm.com> > > commit 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits in > ID_AA64PFR0 register") exposed the MPAM field of AA64PFR0_EL1 to guests, > but didn't add trap handling. A previous patch supplied the missing trap > handling. > > Existing VMs that have the MPAM field of ID_AA64PFR0_EL1 set need to > be migratable, but there is little point enabling the MPAM CPU > interface on new VMs until there is something a guest can do with it. > > Clear the MPAM field from the guest's ID_AA64PFR0_EL1 and on hardware > that supports MPAM, politely ignore the VMMs attempts to set this bit. > > Guests exposed to this bug have the sanitised value of the MPAM field, > so only the correct value needs to be ignored. This means the field > can continue to be used to block migration to incompatible hardware > (between MPAM=1 and MPAM=5), and the VMM can't rely on the field > being ignored. > > Signed-off-by: James Morse <james.morse@arm.com> > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > --- > arch/arm64/kvm/sys_regs.c | 53 ++++++++++++++++++++++++++++++++++++--- > 1 file changed, 50 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 1fd08f12f2bb..f0c55d3907d9 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1723,6 +1723,23 @@ static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > > val &= ~ID_AA64PFR0_EL1_AMU_MASK; > > + /* > + * MPAM is disabled by default as KVM also needs a set of PARTID to > + * program the MPAMVPMx_EL2 PARTID remapping registers with. But some > + * older kernels let the guest see the ID bit. > + */ > + val &= ~ID_AA64PFR0_EL1_MPAM_MASK; > + > + return val; > +} > + > +static u64 read_sanitised_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd) > +{ > + u64 val = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1); > + > + val &= ~ID_AA64PFR1_EL1_MPAM_frac_MASK; > + > return val; > } > > @@ -1834,9 +1851,38 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, > } > > static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > - const struct sys_reg_desc *rd, u64 val) > + const struct sys_reg_desc *rd, u64 user_val) > { > - return set_id_reg(vcpu, rd, val); > + u64 hw_val = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1); > + u64 mpam_mask = ID_AA64PFR0_EL1_MPAM_MASK; > + > + /* > + * Commit 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits > + * in ID_AA64PFR0 register") exposed the MPAM field of AA64PFR0_EL1 to > + * guests, but didn't add trap handling. KVM doesn't support MPAM and > + * always returns an UNDEF for these registers. The guest must see 0 > + * for this field. > + * > + * But KVM must also accept values from user-space that were provided > + * by KVM. On CPUs that support MPAM, permit user-space to write > + * the sanitizied value to ID_AA64PFR0_EL1.MPAM, but ignore this field. > + */ > + if ((hw_val & mpam_mask) == (user_val & mpam_mask)) > + user_val &= ~ID_AA64PFR0_EL1_MPAM_MASK; > + > + return set_id_reg(vcpu, rd, user_val); > +} > + > +static int set_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd, u64 user_val) > +{ > + u64 hw_val = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1); > + u64 mpam_mask = ID_AA64PFR1_EL1_MPAM_frac_MASK; > + > + if ((hw_val & mpam_mask) == (user_val & mpam_mask)) > + user_val &= ~ID_AA64PFR1_EL1_MPAM_frac_MASK; > + > + return set_id_reg(vcpu, rd, user_val); > } > > /* > @@ -2390,7 +2436,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { > ID_AA64PFR0_EL1_RAS | > ID_AA64PFR0_EL1_AdvSIMD | > ID_AA64PFR0_EL1_FP)), > - ID_SANITISED(ID_AA64PFR1_EL1), > + ID_FILTERED(ID_AA64PFR1_EL1, id_aa64pfr1_el1, > + ~ID_AA64PFR1_EL1_MPAM_frac), The 'reset' callback has been changed from kvm_read_sanitised_id_reg() to the newly introduced read_sanitised_id_aa64pfr1_el1(). They're not interchangeable apart from the MPAM_frac bits. For example, The SME bit has been hidden unconditionally and the MTE bit is hidden conditionally in __kvm_read_sanitised_id_reg(), called by kvm_read_sanitised_id_reg(). > ID_WRITABLE(ID_AA64PFR2_EL1, ID_AA64PFR2_EL1_FPMR), > ID_UNALLOCATED(4,3), > ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0), Thanks, Gavin
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 1fd08f12f2bb..f0c55d3907d9 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1723,6 +1723,23 @@ static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, val &= ~ID_AA64PFR0_EL1_AMU_MASK; + /* + * MPAM is disabled by default as KVM also needs a set of PARTID to + * program the MPAMVPMx_EL2 PARTID remapping registers with. But some + * older kernels let the guest see the ID bit. + */ + val &= ~ID_AA64PFR0_EL1_MPAM_MASK; + + return val; +} + +static u64 read_sanitised_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, + const struct sys_reg_desc *rd) +{ + u64 val = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1); + + val &= ~ID_AA64PFR1_EL1_MPAM_frac_MASK; + return val; } @@ -1834,9 +1851,38 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, } static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, - const struct sys_reg_desc *rd, u64 val) + const struct sys_reg_desc *rd, u64 user_val) { - return set_id_reg(vcpu, rd, val); + u64 hw_val = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1); + u64 mpam_mask = ID_AA64PFR0_EL1_MPAM_MASK; + + /* + * Commit 011e5f5bf529f ("arm64/cpufeature: Add remaining feature bits + * in ID_AA64PFR0 register") exposed the MPAM field of AA64PFR0_EL1 to + * guests, but didn't add trap handling. KVM doesn't support MPAM and + * always returns an UNDEF for these registers. The guest must see 0 + * for this field. + * + * But KVM must also accept values from user-space that were provided + * by KVM. On CPUs that support MPAM, permit user-space to write + * the sanitizied value to ID_AA64PFR0_EL1.MPAM, but ignore this field. + */ + if ((hw_val & mpam_mask) == (user_val & mpam_mask)) + user_val &= ~ID_AA64PFR0_EL1_MPAM_MASK; + + return set_id_reg(vcpu, rd, user_val); +} + +static int set_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, + const struct sys_reg_desc *rd, u64 user_val) +{ + u64 hw_val = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1); + u64 mpam_mask = ID_AA64PFR1_EL1_MPAM_frac_MASK; + + if ((hw_val & mpam_mask) == (user_val & mpam_mask)) + user_val &= ~ID_AA64PFR1_EL1_MPAM_frac_MASK; + + return set_id_reg(vcpu, rd, user_val); } /* @@ -2390,7 +2436,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_AA64PFR0_EL1_RAS | ID_AA64PFR0_EL1_AdvSIMD | ID_AA64PFR0_EL1_FP)), - ID_SANITISED(ID_AA64PFR1_EL1), + ID_FILTERED(ID_AA64PFR1_EL1, id_aa64pfr1_el1, + ~ID_AA64PFR1_EL1_MPAM_frac), ID_WRITABLE(ID_AA64PFR2_EL1, ID_AA64PFR2_EL1_FPMR), ID_UNALLOCATED(4,3), ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0),