On Sat, Oct 31, 2020 at 06:23:00PM +0000, Marc Zyngier wrote: > On Thu, 29 Oct 2020 20:11:04 +0000, > Andrew Jones <[email protected]> wrote: > > > > The instruction encodings of ID registers are preallocated. Until an > > encoding is assigned a purpose the register is RAZ. KVM's general ID > > register accessor functions already support both paths, RAZ or not. > > If for each ID register we can determine if it's RAZ or not, then all > > ID registers can build on the general functions. The register visibility > > function allows us to check whether a register should be completely > > hidden or not, extending it to also report when the register should > > be RAZ or not allows us to use it for ID registers as well. > > > > No functional change intended. > > > > Signed-off-by: Andrew Jones <[email protected]> > > --- > > arch/arm64/kvm/sys_regs.c | 19 ++++++++++++++++--- > > arch/arm64/kvm/sys_regs.h | 20 ++++++++++++++++++++ > > 2 files changed, 36 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index d24e66ee59b3..9f6151589460 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -1171,7 +1171,9 @@ static bool access_id_reg(struct kvm_vcpu *vcpu, > > struct sys_reg_params *p, > > const struct sys_reg_desc *r) > > { > > - return __access_id_reg(vcpu, p, r, false); > > + bool raz = sysreg_raz_from_guest(vcpu, r); > > + > > + return __access_id_reg(vcpu, p, r, raz); > > } > > > > static bool access_raz_id_reg(struct kvm_vcpu *vcpu, > > @@ -1283,13 +1285,17 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu, > > static int get_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, > > const struct kvm_one_reg *reg, void __user *uaddr) > > { > > - return __get_id_reg(vcpu, rd, uaddr, false); > > + bool raz = sysreg_raz_from_user(vcpu, rd); > > + > > + return __get_id_reg(vcpu, rd, uaddr, raz); > > } > > > > static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, > > const struct kvm_one_reg *reg, void __user *uaddr) > > { > > - return __set_id_reg(vcpu, rd, uaddr, false); > > + bool raz = sysreg_raz_from_user(vcpu, rd); > > + > > + return __set_id_reg(vcpu, rd, uaddr, raz); > > } > > > > static int get_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc > > *rd, > > @@ -1375,12 +1381,19 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, > > struct sys_reg_params *p, > > return false; > > } > > > > +static unsigned int id_visibility(const struct kvm_vcpu *vcpu, > > + const struct sys_reg_desc *r) > > +{ > > + return 0; > > +} > > + > > /* sys_reg_desc initialiser for known cpufeature ID registers */ > > #define ID_SANITISED(name) { \ > > SYS_DESC(SYS_##name), \ > > .access = access_id_reg, \ > > .get_user = get_id_reg, \ > > .set_user = set_id_reg, \ > > + .visibility = id_visibility, \ > > } > > > > /* > > diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h > > index 5a6fc30f5989..d5add36c130a 100644 > > --- a/arch/arm64/kvm/sys_regs.h > > +++ b/arch/arm64/kvm/sys_regs.h > > @@ -61,6 +61,8 @@ struct sys_reg_desc { > > > > #define REG_HIDDEN_USER (1 << 0) /* hidden from userspace > > ioctls */ > > #define REG_HIDDEN_GUEST (1 << 1) /* hidden from guest */ > > +#define REG_RAZ_USER (1 << 2) /* RAZ from userspace ioctls */ > > +#define REG_RAZ_GUEST (1 << 3) /* RAZ from guest */ > > > > static __printf(2, 3) > > inline void print_sys_reg_msg(const struct sys_reg_params *p, > > @@ -129,6 +131,24 @@ static inline bool sysreg_hidden_from_user(const > > struct kvm_vcpu *vcpu, > > return r->visibility(vcpu, r) & REG_HIDDEN_USER; > > } > > > > +static inline bool sysreg_raz_from_guest(const struct kvm_vcpu *vcpu, > > + const struct sys_reg_desc *r) > > +{ > > + if (likely(!r->visibility)) > > + return false; > > + > > + return r->visibility(vcpu, r) & REG_RAZ_GUEST; > > +} > > + > > +static inline bool sysreg_raz_from_user(const struct kvm_vcpu *vcpu, > > + const struct sys_reg_desc *r) > > +{ > > + if (likely(!r->visibility)) > > + return false; > > + > > + return r->visibility(vcpu, r) & REG_RAZ_USER; > > +} > > + > > static inline int cmp_sys_reg(const struct sys_reg_desc *i1, > > const struct sys_reg_desc *i2) > > { > > Is there actually a case for any ID register to have different > RAZ semantics between guest and userspace? I have the feeling that > we'd want them to be consistent at all times. Or did you have any > particular (and future) use case in mind?
I was just following the hidden pattern too closely. You're right that we'll probably only ever need a single RAZ flag. And, if we do ever need both, then we can always add another flag later. I'll respin this patch with just one flag. > > Otherwise, looks good. > Thanks, drew _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
