Re: [PATCH v2 1/6] KVM: arm64: Move CPU ID feature registers emulation into a separate file

[Marc Zyngier <maz@kernel.org>]

On Fri, 24 Feb 2023 01:01:44 +0000,
Reiji Watanabe <reijiw@google.com> 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@kernel.org/T/#mced7be0152816d3a1d02cf8c8b95d3ab3ef4e0c8

-- 
Without deviation from the norm, progress is not possible.