[PATCH v3 05/11] KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM

[Mark Brown]

On Mon, Jun 03, 2024 at 09:37:16AM +0100, Fuad Tabba wrote:
> On Fri, May 31, 2024 at 3:09 PM Mark Brown <broonie at kernel.org> wrote:
> > On Tue, May 28, 2024 at 01:59:08PM +0100, Fuad Tabba wrote:

> > > +     write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);

> > As well as the sync issue Oliver mentioned on the removal of
> > _cond_update() just doing these updates as a blind write creates a
> > surprise if we ever get more control bits in ZCR_EL2.

> I'm not sure it does. The other bits are RES0, and this is always
> setting the length. So even if new control bits are added, this
> shouldn't matter.

The surprise would be that if new control bits were added this would
result in clearing them on restore.

> Also, one of the concerns in terms of performance is now with
> nested-virt support being added, and the overhead of doing the
> conditional update when we know that it's unlikely that anyone is
> implementing vectors as big as the max.

I guess there's the option of doing a restore of a value fixed during
initialisation instead?

> > > +     /*
> > > +      * On saving/restoring host sve state, always use the maximum VL for
> > > +      * the host. The layout of the data when saving the sve state depends
> > > +      * on the VL, so use a consistent (i.e., the maximum) host VL.
> > > +      *
> > > +      * Setting ZCR_EL2 to ZCR_ELx_LEN_MASK sets the effective length
> > > +      * supported by the system (or limited at EL3).
> > > +      */
> > > +     write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);

> > Setting ZCR_ELx_LEN_MASK sets the VL to the maximum supported/configured
> > for the current PE, not the system.  This will hopefully be the same
> > since we really hope implementors continue to build symmetric systems
> > but there is handling for that case in the kernel just in case.  Given
> > that we record the host's maximum VL should we use it?

> You're right, but even if the current PE had a different vector
> length, ZCR_ELx_LEN_MASK is the default value for ZCR_EL2 when the
> host is running (this is the existing behavior before this patch
> series). It is also the value this patch series uses when saving the
> host SVE state. So since we are consistent I think this is correct.

The reason we just set all bits in ZCR_EL2.LEN is that we don't
currently use SVE at EL2 so we're just passing everything through to EL1
and letting it worry about things.  As we start adding more SVE code at
EL2 we need to care more and I think we should start explicitly
programming what we think we're using to use to avoid surprises.  For
example in this series we allocate the buffer used to store the host SVE
state based on the probed maximum usable VL for the system but here we
use whatever the PE has as the maximum VL.  This means that in the
(hopefully unlikely) case where the probed value is lower than the PE
value we'll overflow the buffer.
-------------- 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/20240603/63176e40/attachment-0001.sig>