On Sat, Oct 31, 2020 at 06:23:00PM +0000, Marc Zyngier wrote:
> On Thu, 29 Oct 2020 20:11:04 +0000,
> Andrew Jones <drjones@xxxxxxxxxx> 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 <drjones@xxxxxxxxxx>
> > ---
> > 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
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
