[PATCH v1] KVM: arm64: Allocate for maximum vector length for pKVM host sve state

[Mark Brown]

On Wed, Jun 05, 2024 at 05:14:23PM +0100, Fuad Tabba wrote:
> On Wed, Jun 5, 2024 at 4:46 PM Mark Brown <broonie at kernel.org> wrote:
> > On Wed, Jun 05, 2024 at 04:03:12PM +0100, Fuad Tabba wrote:

> > > +     /* The maximum vl encountered for any cpu in the system. */
> > > +     int max_system_vl;
> > > +

> > As previously mentioned I'd really prefer not to call this _system_ due
> > to the potential confusion with the maximum VL we're using.  Naming is
> > hard.

> I agree :) I have no strong preference really. It's just that for me
> max_cpu_vl sounds like it's per cpu. But now I'm just bikeshedding...

seen?

> > > @@ -1031,7 +1031,12 @@ static void vec_probe_vqs(struct vl_info *info,
> > >
> > >               vq = sve_vq_from_vl(vl); /* skip intervening lengths */
> > >               set_bit(__vq_to_bit(vq), map);
> > > +
> > > +             if (!max_vl)
> > > +                     max_vl = vl;
> > >       }
> > > +
> > > +     info->max_system_vl = max((int) max_vl, info->max_system_vl);

> > It's not clear to me why we stage this in a local variable prior to
> > updating the global value, and like I say it does feel cleaner and safer
> > to keep the current split between enumeration of the PE and integration
> > of the results.  This should *work* but it doesn't feel consistent with
> > the rest of the code.

> To me it felt more clear like this, but I'm fine with not doing the
> staging first.

That was the start of the thought path that lead me to implement based
on splitting it out of vec_probe_vqs() like the rest of the code does,
the logic doesn't really fit in here for multiple reasons.

> 
> >
> > > -               kvm_host_sve_max_vl = sve_max_vl();
> > > +               kvm_host_sve_max_vl = sve_max_system_vl();
> >
> > I appreciate that we're adding a host_ on the front of it but it would
> > feel safer to keep the name of the hypervisor copy of the value
> > consistent with where we copied it from, sve_max_vl is a different thing
> > in the host kernel.
> 
> I agree, this should be renamed.
> 
> Thanks!
> /fuad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240605/5d996580/attachment.sig>