Re: [PATCH 5/5] KVM: arm64: Exclude FP ownership from kvm_vcpu_arch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Mar 09, 2024 at 11:01:56AM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@xxxxxxxxxx> wrote:

> > I agree that we don't want to store multiple copies, the proposed
> > implementation for SME does that - when converting between guest native
> > and userspace formats we discard the original format after conversion
> > and updates the metadata which says which format is stored.  That
> > metadata is modeled as adding a new owner.

> What conversion? If userspace writes to FP, the upper bits of the SVE
> registers should get zeroed. If it writes to SVE, it writes using the
> current VL for the current streaming/non-streaming mode.

> If the current userspace API doesn't give us the tools to do so, we
> rev it.

This is the conversion of vector lengths rather than something resulting
from acccessing from accessing the V registers via the sysreg interface
(which is not supported by either upstream or the currently posted SME
patches once either vector extension is enabled).  As previously
mentioned the current implementation of SME support for KVM always
presents the vector length sized registers to userspace with the maximum
vector length, not the vector length currently selected by SVCR.

> > > Nothing prevent you from storing the registers using the *current* VL,
> > > since there is no data sharing between the SVE registers and the
> > > streaming-SVE ones. All you need to do is to make sure you don't mix
> > > the two.

> > You seem to be suggesting modeling the streaming mode registers as a
> > completely different set of registers in the KVM ABI.

> Where are you reading this? I *never* said anything of the sort. There
> is only one set of SVE registers. The fact that they change size and
> get randomly zeroed when userspace flips bits is a property of the
> architecture. The HW *can* implements them as two sets of registers if
> SME is a separate accelerator, but that's not architectural.

When you talk about there being "no data sharing" and "mixing the two"
that sounds a lot like there might be two separate sets of data.  I've
been inferring a lot of what you are looking for from statements that
you make about other ideas and from the existing code and documentation
so things are less clear than they might be and it's likely there's been
some assumptions missed in places.  In order to try to clarify things
I've written down a summary of what I currently understand you're
looking for below.

> All I'm saying is that you can have a *single* register file, spanning
> both FPSIMD and SVE, using the maximum VL achievable in the guest, and

When you say "using" there I take it that you mean "sized to store"
rather than "written/accessed in the format for" since elsewhere you've
been taking about storing the current native format and changing the VL
in the ABI based on SVCR.SM?  I would have read "using the maximum VL
achievable in the guest" as meaning we store in a format reflecting the
larger VL but I'm pretty sure it's just the allocation you're
referencing there.

To be clear what I'm currently understanding for the userspace ABI is:

 - Create a requirement for userspace to set SVCR prior to setting any
   vector impacted register to ensure the correct format and that data
   isn't zeroed when SVCR is set.
 - Use the value of SVCR.SM and the guest maximum SVE and SME VLs to
   select the currently visible vector length for the Z, P and FFR
   registers.
 - This also implies discarding or failing all writes to ZA and ZT0
   unless SVCR.ZA is set for consistency, though that's much less
   impactful in terms of the implementation.
 - Add support for the V registers in the sysreg interface when SVE is
   enabled.

then the implementation can do what it likes to achieve that, the most
obvious thing being to store in native format for the current hardware
mode based on SVCR.{SM,ZA}.  Does that sound about right?

If that's all good the main thing I'm unclear on your expectations for
is what should happen with registers that are inaccessible in the
current mode (Z, P and FFR when in non-streaming mode without SVE, FFR
when in streaming mode without FA64, and ZA and ZT0 when SVCR.ZA is 0).
Having these registers cause errors on access as they would in the
architecture but I worry about complicating and potentially breaking the
ABI by having enumerable but inaccessible registers, though I think
that's more in line with your general thinking.  The main alternative
would be RAZ/WI style behavior which isn't quite what the architecture
does but does give what the guest would observe when it changes modes
and looks at the contents of those registers.

> be done with it. Yes, you'll have to refactor the code so that FPSIMD
> lands at the right spot in the SVE register file. Big deal.

At present the ABI just disables the V registers when SVE is enabled, I
was slightly surprised at that implementation and was concerned there
might be something that was considered desirable about the removal of
ambiguity (the changelog for the relevant commit did mention that it was
convenient to implement).

> > My
> > understanding was that it was intended that userspace should be able to
> > do something like just enumerate all the registers, save them and then
> > later on restore them without really understanding them.

> And this statement doesn't get in the way of anything above. We own
> the ABI, and can change it as we see fit when SME is enabled.

If you're willing to require specific ordering knowledge to do guest
state load then that makes life a bit easier for the implementation.  It
had appeared that VMMs could have existing ABI expectations about how
they can enumerate, store and load the state the guest implements which
we would cause problems by breaking.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux