> > > +/*
> > > + * Set the guest's ID registers that are defined in id_reg_descs[]
> > > + * with ID_SANITISED() to the host's sanitized value.
> > > + */
> > > +void kvm_arm_set_default_id_regs(struct kvm *kvm)
> > > +{
> > > + int i;
> > > + u32 id;
> > > + u64 val;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(id_reg_descs); i++) {
> > > + id = reg_to_encoding(&id_reg_descs[i]);
> > > + if (WARN_ON_ONCE(!is_id_reg(id)))
> > > + /* Shouldn't happen */
> > > + continue;
> > > +
> > > + if (id_reg_descs[i].visibility == raz_visibility)
> > > + /* Hidden or reserved ID register */
> > > + continue;
> >
> > Relying on function pointer comparison is really fragile. If I wrap
> > raz_visibility() in another function, this won't catch it. It also
> > doesn't bode well with your 'inline' definition of this function.
> >
> > More importantly, why do we care about checking for visibility at all?
> > We can happily populate the array and rely on the runtime visibility.
> Right. I'll remove this checking.
Without the check, calling read_sanitised_ftr_reg() for some hidden
ID registers will show a warning as some of them are not in
arm64_ftr_regs[] (e.g. reserved ones). This checking is needed
temporarily to avoid the warning (the check is removed in the following
patches of this series). It would be much less fragile to call the
visibility function instead, but I don't think this is a also good way
to check the availability of the sanitized values for ID registers
either. I didn't found a good (proper) way to check that without
making changes in cpufeature.c, and I'm not sure if it is worth it
for this temporary purpose.
Thank you,
Reiji
> >
> > > +
> > > + val = read_sanitised_ftr_reg(id);
> > > + kvm->arch.id_regs[IDREG_IDX(id)] = val;
> > > + }
> > > +}
> >
> > Thanks,
> >
> > M.
> >
> > --
> > Without deviation from the norm, progress is not possible.
>
> Thanks,
> Jing
