| Message ID | 20241004110714.2051604-6-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> > > The sys_reg_descs array holds function pointers and reset value for > managing the user-space and guest view of system registers. These > are mostly created by a set of macro's as only some combinations > of behaviour are needed. > > If a register needs special treatment, its sys_reg_descs entry is > open-coded. This is true of some id registers where the value provided > by user-space is validated by some helpers. > > Before adding another one of these, add a helper that covers the > existing special cases. 'ID_FILTERED' expects helpers to set the > user-space value, and retrieve the modified reset value. > > Signed-off-by: James Morse <james.morse@arm.com> > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > --- > arch/arm64/kvm/sys_regs.c | 54 ++++++++++++++++++++------------------- > 1 file changed, 28 insertions(+), 26 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 8b1a6cedc49e..1fd08f12f2bb 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1833,6 +1833,12 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, > return set_id_reg(vcpu, rd, val); > } > > +static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd, u64 val) > +{ > + return set_id_reg(vcpu, rd, val); > +} > + > /* > * cpufeature ID register user accessors > * > @@ -2131,6 +2137,15 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu, > .val = 0, \ > } > > +/* sys_reg_desc initialiser for cpufeature ID registers that need filtering */ > +#define ID_FILTERED(sysreg, name, filtered_fields) { \ > + ID_DESC(sysreg), \ > + .set_user = set_##name, \ > + .visibility = id_visibility, \ > + .reset = read_sanitised_##name, \ > + .val = (filtered_fields), \ > +} > + > /* sys_reg_desc initialiser for known cpufeature ID registers */ > #define AA32_ID_SANITISED(name) { \ > ID_DESC(name), \ > @@ -2337,14 +2352,9 @@ static const struct sys_reg_desc sys_reg_descs[] = { > /* CRm=1 */ > AA32_ID_SANITISED(ID_PFR0_EL1), > AA32_ID_SANITISED(ID_PFR1_EL1), > - { SYS_DESC(SYS_ID_DFR0_EL1), > - .access = access_id_reg, > - .get_user = get_id_reg, > - .set_user = set_id_dfr0_el1, > - .visibility = aa32_id_visibility, > - .reset = read_sanitised_id_dfr0_el1, > - .val = ID_DFR0_EL1_PerfMon_MASK | > - ID_DFR0_EL1_CopDbg_MASK, }, > + ID_FILTERED(ID_DFR0_EL1, id_dfr0_el1, > + ID_DFR0_EL1_PerfMon_MASK | > + ID_DFR0_EL1_CopDbg_MASK), The replacement causes logical change because the 'visibility' callback has been changed from aa32_id_visibility() to id_visibility(). I think we need an aa32 variant of ID_FILTERED, to be used here. #define AA32_ID_FILTERED(sysreg, name, filtered_fields) { \ ID_DESC(sysreg), \ .set_user = set_##name, \ .visibility = aa32_id_visibility, \ /* the only differentiated field */ .reset = read_sanitised_##name, \ .val = (filtered_fields), \ } > ID_HIDDEN(ID_AFR0_EL1), > AA32_ID_SANITISED(ID_MMFR0_EL1), > AA32_ID_SANITISED(ID_MMFR1_EL1), > @@ -2373,17 +2383,13 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > /* AArch64 ID registers */ > /* CRm=4 */ > - { SYS_DESC(SYS_ID_AA64PFR0_EL1), > - .access = access_id_reg, > - .get_user = get_id_reg, > - .set_user = set_id_reg, > - .reset = read_sanitised_id_aa64pfr0_el1, > - .val = ~(ID_AA64PFR0_EL1_AMU | > - ID_AA64PFR0_EL1_MPAM | > - ID_AA64PFR0_EL1_SVE | > - ID_AA64PFR0_EL1_RAS | > - ID_AA64PFR0_EL1_AdvSIMD | > - ID_AA64PFR0_EL1_FP), }, > + ID_FILTERED(ID_AA64PFR0_EL1, id_aa64pfr0_el1, > + ~(ID_AA64PFR0_EL1_AMU | > + ID_AA64PFR0_EL1_MPAM | > + ID_AA64PFR0_EL1_SVE | > + ID_AA64PFR0_EL1_RAS | > + ID_AA64PFR0_EL1_AdvSIMD | > + ID_AA64PFR0_EL1_FP)), Note that the 'visibility' callback has been changed from NULL to id_visibility(), not resulting in logical changes since id_visibility() returns 0 for ID_AA64PFR0_EL1. However, it would be nice to be documented in the change log? > ID_SANITISED(ID_AA64PFR1_EL1), > ID_WRITABLE(ID_AA64PFR2_EL1, ID_AA64PFR2_EL1_FPMR), > ID_UNALLOCATED(4,3), > @@ -2393,13 +2399,9 @@ static const struct sys_reg_desc sys_reg_descs[] = { > ID_WRITABLE(ID_AA64FPFR0_EL1, ~ID_AA64FPFR0_EL1_RES0), > > /* CRm=5 */ > - { SYS_DESC(SYS_ID_AA64DFR0_EL1), > - .access = access_id_reg, > - .get_user = get_id_reg, > - .set_user = set_id_aa64dfr0_el1, > - .reset = read_sanitised_id_aa64dfr0_el1, > - .val = ID_AA64DFR0_EL1_PMUVer_MASK | > - ID_AA64DFR0_EL1_DebugVer_MASK, }, > + ID_FILTERED(ID_AA64DFR0_EL1, id_aa64dfr0_el1, > + ID_AA64DFR0_EL1_PMUVer_MASK | > + ID_AA64DFR0_EL1_DebugVer_MASK), Same as above. The 'visibility' callback has been changed from NULL to id_visibility(), even there is no logical changes. > ID_SANITISED(ID_AA64DFR1_EL1), > ID_UNALLOCATED(5,2), > ID_UNALLOCATED(5,3), Thanks, Gavin
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 8b1a6cedc49e..1fd08f12f2bb 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1833,6 +1833,12 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, return set_id_reg(vcpu, rd, val); } +static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, + const struct sys_reg_desc *rd, u64 val) +{ + return set_id_reg(vcpu, rd, val); +} + /* * cpufeature ID register user accessors * @@ -2131,6 +2137,15 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu, .val = 0, \ } +/* sys_reg_desc initialiser for cpufeature ID registers that need filtering */ +#define ID_FILTERED(sysreg, name, filtered_fields) { \ + ID_DESC(sysreg), \ + .set_user = set_##name, \ + .visibility = id_visibility, \ + .reset = read_sanitised_##name, \ + .val = (filtered_fields), \ +} + /* sys_reg_desc initialiser for known cpufeature ID registers */ #define AA32_ID_SANITISED(name) { \ ID_DESC(name), \ @@ -2337,14 +2352,9 @@ static const struct sys_reg_desc sys_reg_descs[] = { /* CRm=1 */ AA32_ID_SANITISED(ID_PFR0_EL1), AA32_ID_SANITISED(ID_PFR1_EL1), - { SYS_DESC(SYS_ID_DFR0_EL1), - .access = access_id_reg, - .get_user = get_id_reg, - .set_user = set_id_dfr0_el1, - .visibility = aa32_id_visibility, - .reset = read_sanitised_id_dfr0_el1, - .val = ID_DFR0_EL1_PerfMon_MASK | - ID_DFR0_EL1_CopDbg_MASK, }, + ID_FILTERED(ID_DFR0_EL1, id_dfr0_el1, + ID_DFR0_EL1_PerfMon_MASK | + ID_DFR0_EL1_CopDbg_MASK), ID_HIDDEN(ID_AFR0_EL1), AA32_ID_SANITISED(ID_MMFR0_EL1), AA32_ID_SANITISED(ID_MMFR1_EL1), @@ -2373,17 +2383,13 @@ static const struct sys_reg_desc sys_reg_descs[] = { /* AArch64 ID registers */ /* CRm=4 */ - { SYS_DESC(SYS_ID_AA64PFR0_EL1), - .access = access_id_reg, - .get_user = get_id_reg, - .set_user = set_id_reg, - .reset = read_sanitised_id_aa64pfr0_el1, - .val = ~(ID_AA64PFR0_EL1_AMU | - ID_AA64PFR0_EL1_MPAM | - ID_AA64PFR0_EL1_SVE | - ID_AA64PFR0_EL1_RAS | - ID_AA64PFR0_EL1_AdvSIMD | - ID_AA64PFR0_EL1_FP), }, + ID_FILTERED(ID_AA64PFR0_EL1, id_aa64pfr0_el1, + ~(ID_AA64PFR0_EL1_AMU | + ID_AA64PFR0_EL1_MPAM | + ID_AA64PFR0_EL1_SVE | + ID_AA64PFR0_EL1_RAS | + ID_AA64PFR0_EL1_AdvSIMD | + ID_AA64PFR0_EL1_FP)), ID_SANITISED(ID_AA64PFR1_EL1), ID_WRITABLE(ID_AA64PFR2_EL1, ID_AA64PFR2_EL1_FPMR), ID_UNALLOCATED(4,3), @@ -2393,13 +2399,9 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_WRITABLE(ID_AA64FPFR0_EL1, ~ID_AA64FPFR0_EL1_RES0), /* CRm=5 */ - { SYS_DESC(SYS_ID_AA64DFR0_EL1), - .access = access_id_reg, - .get_user = get_id_reg, - .set_user = set_id_aa64dfr0_el1, - .reset = read_sanitised_id_aa64dfr0_el1, - .val = ID_AA64DFR0_EL1_PMUVer_MASK | - ID_AA64DFR0_EL1_DebugVer_MASK, }, + ID_FILTERED(ID_AA64DFR0_EL1, id_aa64dfr0_el1, + ID_AA64DFR0_EL1_PMUVer_MASK | + ID_AA64DFR0_EL1_DebugVer_MASK), ID_SANITISED(ID_AA64DFR1_EL1), ID_UNALLOCATED(5,2), ID_UNALLOCATED(5,3),