Re: [PATCH v4 00/27] KVM: arm64: Implement support for SME in non-protected guests

[Marc Zyngier <maz@kernel.org>]

On Fri, 14 Feb 2025 15:13:52 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Fri, Feb 14, 2025 at 09:24:03AM +0000, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > Just to be clear: I do not intend to review a series that doesn't
> > cover the full gamut of KVM from day 1. Protected mode is an absolute
> > requirement. It is the largest KVM deployment, and Android phones the
> > only commonly available platform with SME. If CCA gets merged prior to
> > SME support, supporting it will also be a requirement.
> 
> OK, no problem.  This is a new requirement and I'd been trying to
> balance the concerns people have with the size of serieses like this
> with the need to get everything in, my plan had been to follow up as
> soon as possible with pKVM.

If size is an issue, split the UAPI from the core code. But don't
fragment the overall world switch and exception handling, because
that's a sure way to end-up with the same sort of problems we ended up
fixing in SVE. pKVM has a direct influence on what you track, where
you track it, and implementing it as an afterthought is a very bad
idea.

> 
> > > Access to the floating point registers follows the architecture:
> 
> > >  - When both SVE and SME are present:
> > >    - If PSTATE.SM == 0 the vector length used for the Z and P registers
> > >      is the SVE vector length.
> > >    - If PSTATE.SM == 1 the vector length used for the Z and P registers
> > >      is the SME vector length.
> > >  - If only SME is present:
> > >    - If PSTATE.SM == 0 the Z and P registers are inaccessible and the
> > >      floating point state accessed via the encodings for the V registers. 
> > >    - If PSTATE.SM == 1 the vector length used for the Z and P registers
> > >  - The SME specific ZA and ZT0 registers are only accessible if SVCR.ZA is 1.
> 
> > > The VMM must understand this, in particular when loading state SVCR
> > > should be configured before other state.
> 
> > Why SVCR? This isn't a register, just an architected accessor to
> > PSTATE.{ZA,SM}. Userspace already has direct access to PSTATE, so I
> > don't understand this requirement.
> 
> Could you be more explicit as to what you mean by direct access to
> PSTATE here?  The direct access to these PSTATE fields is in the form of
> SVCR register accesses, or writes via SMSTART or SMSTOP instructions
> when executing code - is there another access mechanism I'm not aware of
> here?  They don't appear in SPSR.  Or is this a terminology issue with
> referring to SVCR as the mechanism for configuring PSTATE.{SM,ZA}
> without explicitly calling out that that's what's happening?

I'm painfully aware of the architecture limitations.

However, I don't get your mention of SPSR here. The architecture is
quite clear that PSTATE is where these bits are held, that they are
not propagated anywhere else, and that's where userspace should expect
to find them.

The fact that SW must use SVCR to alter PSTATE.{ZA,SM} doesn't matter.
We save/restore registers, not accessors. If this means we need to
play a dance when the VMM accesses PSTATE to reconciliate KVM's
internal view with the userspace view, so be it.

It probably means you need to obtain a clarification of the
architecture to define *where* these bits are stored in PSTATE,
because that's not currently defined.

>
> > Isn't it that there is simply a dependency between restoring PSTATE
> > and any of the vector stuff? Also, how do you preserve the current ABI
> > that do not have this requirement?
> 
> Yes, that's the dependency - I'm spelling out explicitly what changes in
> the register view when PSTATE.{SM,ZA} are restored.  This ABI is what
> you appeared to be asking for the last time we discussed this.
> Previously I had also proposed either:
> 
>  - Exposing the streaming mode view of the register state as separate
>    registers, selecting between the standard and streaming views for
>    load/save based on the mode when the guest runs and clearing the
>    inactive registers on userspace access.
> 
>  - Always presenting userspace with the largest available vector length,
>    zero padding when userspace reads and discarding unused high bits
>    when loading into the registers for the guest.  This ends up
>    requiring rewriting between VLs, or to/from FPSIMD format, around
>    periods of userspace access since when normally executing and context
>    switching the guest we want to store the data in the native format
>    for the current PSTATE.SM for performance.
> 
> both of which largely avoid the ordering requirements but add complexity
> to the implementation, and memory overhead in the first case.  I'd
> originally implemented the second case, that seems the best of the
> available options to me.  You weren't happy with these options and said
> that we should not translate register formats and always use the current
> mode for the vCPU, but given that changing PSTATE.SM changes the
> register sizes that ends up creating an ordering requirement.  You
> seemed to agree that it was reasonable to have an ordering requirement
> with PSTATE.SM so long as it only came when SME had been explicitly
> enabled.
> 
> Would you prefer:
> 
>  - Changing the register view based on the current value of PSTATE.SM.
>  - Exposing streaming mode Z and P as separate registers.
>  - Exposing the existing Z and P registers with the maximum S?E VL.
> 
> or some other option?

My take on this hasn't changed. I want to see something that behaves
*exactly* like the architecture defines the expected behaviour of a
CPU.

But you still haven't answered my question: How is the *current* ABI
preserved? Does it require *not* selecting SME? Does it require
anything else? I'm expecting simple answers to simple questions, not a
wall of text describing something that is not emulating the
architecture.

	M.

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