On Fri, 24 Feb 2023 01:01:44 +0000,
Reiji Watanabe <reijiw@xxxxxxxxxx> wrote:
[...]
> > +const struct sys_reg_desc *kvm_arm_find_id_reg(const struct sys_reg_params *params)
> > +{
>
> You might want to check if the param indicates the ID register,
> before running the binary search for the ID register table.
> (I have the same comment to kvm_arm_get_id_reg() and
> kvm_arm_set_id_reg()).
It would be much better if the discrimination was done in
emulate_sys_reg(), just like we do for NV[1].
Save yourself pointless search on the critical path, and make the
decoding tree visible in one place.
>
> > + return find_reg(params, id_reg_descs, ARRAY_SIZE(id_reg_descs));
> > +}
> > +
> > +void kvm_arm_reset_id_regs(struct kvm_vcpu *vcpu)
> > +{
> > + unsigned long i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(id_reg_descs); i++)
> > + if (id_reg_descs[i].reset)
> > + id_reg_descs[i].reset(vcpu, &id_reg_descs[i]);
> > +}
> > +
> > +int kvm_arm_get_id_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> > +{
> > + return kvm_sys_reg_get_user(vcpu, reg,
> > + id_reg_descs, ARRAY_SIZE(id_reg_descs));
> > +}
> > +
> > +int kvm_arm_set_id_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> > +{
> > + return kvm_sys_reg_set_user(vcpu, reg,
> > + id_reg_descs, ARRAY_SIZE(id_reg_descs));
> > +}
> > +
> > +bool kvm_arm_check_idreg_table(void)
> > +{
> > + return check_sysreg_table(id_reg_descs, ARRAY_SIZE(id_reg_descs), false);
> > +}
> > +
> > +/* Assumed ordered tables, see kvm_sys_reg_table_init. */
>
> I don't think we need this comment, as the code doesn't seem to
> assume that.
Yeah, it only makes sense for the binary search.
Thanks,
M.
[1] https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-maz@xxxxxxxxxx/T/#mced7be0152816d3a1d02cf8c8b95d3ab3ef4e0c8
--
Without deviation from the norm, progress is not possible.
