> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -263,6 +263,76 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
> > return read_zero(vcpu, p);
> > }
> >
> > +struct id_reg_info {
> > + u32 sys_reg; /* Register ID */
> > + u64 sys_val; /* Sanitized system value */
> > +
> > + /*
> > + * Limit value of the register for a vcpu. The value is sys_val
> > + * with bits cleared for unsupported features for the guest.
> > + */
> > + u64 vcpu_limit_val;
>
> Maybe I'll see a need for both later, but at the moment I'd think we only
> need sys_val with the bits cleared for disabled features.
Uh, yes, sys_val is used in patch-15 and I should have introduced
the field in the patch. I will fix it in v2.
> > -static int __set_id_reg(const struct kvm_vcpu *vcpu,
> > +static int __set_id_reg(struct kvm_vcpu *vcpu,
> > const struct sys_reg_desc *rd, void __user *uaddr,
> > bool raz)
> > {
> > const u64 id = sys_reg_to_index(rd);
> > + u32 encoding = reg_to_encoding(rd);
> > int err;
> > u64 val;
> >
> > @@ -1252,10 +1327,18 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu,
> > if (err)
> > return err;
> >
> > - /* This is what we mean by invariant: you can't change it. */
> > - if (val != read_id_reg(vcpu, rd, raz))
> > + /* Don't allow to change the reg unless the reg has id_reg_info */
> > + if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding))
> > return -EINVAL;
> >
> > + if (raz)
> > + return (val == 0) ? 0 : -EINVAL;
>
> This is already covered by the val != read_id_reg(vcpu, rd, raz) check.
Yes, it can simply return 0 for raz case in this patch.
I will fix this in v2.
> > + err = validate_id_reg(vcpu, rd, val);
> > + if (err)
> > + return err;
> > +
> > + __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(encoding)) = val;
> > return 0;
> > }
> >
> > @@ -2818,6 +2901,23 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
> > return write_demux_regids(uindices);
> > }
> >
> > +static void id_reg_info_init_all(void)
> > +{
> > + int i;
> > + struct id_reg_info *id_reg;
> > +
> > + for (i = 0; i < ARRAY_SIZE(id_reg_info_table); i++) {
> > + id_reg = (struct id_reg_info *)id_reg_info_table[i];
> > + if (!id_reg)
> > + continue;
> > +
> > + if (id_reg->init)
> > + id_reg->init(id_reg);
> > + else
> > + id_reg_info_init(id_reg);
>
> Maybe call id_reg->init(id_reg) from within id_reg_info_init() in case we
> wanted to apply some common id register initialization at some point?
Thank you for the nice suggestion.
That sounds like a better idea. I'll look into fixing it in v2.
Thanks,
Reiji
