Hey Marc,
On Wednesday 13 Jan 2021 at 11:33:13 (+0000), Marc Zyngier wrote:
> > +#undef KVM_HYP_CPU_FTR_REG
> > +#define KVM_HYP_CPU_FTR_REG(id, name) \
> > + { .sys_id = id, .dst = (struct arm64_ftr_reg *)&kvm_nvhe_sym(name) },
> > +static const struct __ftr_reg_copy_entry {
> > + u32 sys_id;
> > + struct arm64_ftr_reg *dst;
>
> Why do we need the whole data structure? Can't we just live with sys_val?
I don't have a use-case for anything else than sys_val, so yes I think I
should be able to simplify. I'll try that for v3.
>
> > +} hyp_ftr_regs[] = {
> > + #include <asm/kvm_cpufeature.h>
> > +};
>
> Can't this be made __initdata?
Good point, that would be nice indeed. Can I use that from outside an
__init function? If not, I'll need to rework the code a bit more, but
that should be simple enough either way.
> > +
> > +static int copy_cpu_ftr_regs(void)
> > +{
> > + int i, ret;
> > +
> > + for (i = 0; i < ARRAY_SIZE(hyp_ftr_regs); i++) {
> > + ret = copy_ftr_reg(hyp_ftr_regs[i].sys_id, hyp_ftr_regs[i].dst);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * Inits Hyp-mode on all online CPUs
> > */
> > @@ -1705,6 +1729,13 @@ static int init_hyp_mode(void)
> > int cpu;
> > int err = 0;
> >
> > + /*
> > + * Copy the required CPU feature register in their EL2 counterpart
> > + */
> > + err = copy_cpu_ftr_regs();
> > + if (err)
> > + return err;
> > +
>
> Just to keep things together, please move any sysreg manipulation into
> sys_regs.c, most probably into kvm_sys_reg_table_init().
Will do.
Thanks,
Quentin
