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

[Fuad Tabba]

Hi Mark,

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...

> > @@ -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.

>
> > -               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