[PATCH v10 1/5] KVM: arm64: Save ID registers' sanitized value per guest

[Marc Zyngier]

On Tue, 30 May 2023 19:02:03 +0100,
Jing Zhang <jingzhangos at google.com> wrote:

[...]

> > > +/*
> > > + * Set the guest's ID registers with ID_SANITISED() to the host's sanitized value.
> > > + */
> > > +void kvm_arm_init_id_regs(struct kvm *kvm)
> > > +{
> > > +     const struct sys_reg_desc *idreg;
> > > +     struct sys_reg_params params;
> > > +     u32 id;
> > > +
> > > +     /* Find the first idreg (SYS_ID_PFR0_EL1) in sys_reg_descs. */
> > > +     id = SYS_ID_PFR0_EL1;
> > > +     params = encoding_to_params(id);
> > > +     idreg = find_reg(&params, sys_reg_descs, ARRAY_SIZE(sys_reg_descs));
> > > +     if (WARN_ON(!idreg))
> > > +             return;
> >
> > What is this trying to guard against? Not finding ID_PFR0_EL1 in the
> > sysreg table? But this says nothing about the following registers (all
> > 55 of them), so why do we need to special-case this one?
> Here is to find the first idreg in the array and warn that no idregs
> found in the array with the assumption that ID_PFR0_EL1 is the first
> one defined and if it is not found, then no other idregs are defined
> either.

I didn't make my point clear. What we have is a purely static array.
Why should we perform such a test on every single VM creation? Any
structural validation should only happen once, at KVM init time.

> Another way is to go through all the regs in array sys_reg_descs and
> do the initialization if it is a idreg.

That'd be a waste of precious cycles.

This WARN_ON()+early return should go, but the rest is fine.

Thanks,

	M.

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