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

From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	kvm@vger.kernel.org, James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>,
	James Clark <james.clark@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Dongli Zhang <dongli.zhang@oracle.com>
Subject: Re: [PATCH v2 5/5] KVM: arm64: Exclude FP ownership from kvm_vcpu_arch
Date: Sat, 23 Mar 2024 19:06:24 +0000	[thread overview]
Message-ID: <87edc0sr7z.wl-maz@kernel.org> (raw)
In-Reply-To: <fe5edef8-e61d-42b3-b4da-6f6bebd60013@sirena.org.uk>

On Fri, 22 Mar 2024 17:52:45 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Fri, Mar 22, 2024 at 05:09:45PM +0000, Marc Zyngier wrote:
> > In retrospect, it is fairly obvious that the FP state ownership
> > is only meaningful for a given CPU, and that locating this
> > information in the vcpu was just a mistake.
> > 
> > Move the ownership tracking into the host data structure, and
> > rename it from fp_state to fp_owner, which is a better description
> > (name suggested by Mark Brown).
> 
> There's still the thing with the interaction with SME support - to
> summarise what I think you're asking for the userspace ABI there:

Well, the SME support is still pretty prospective, and this patch has
no impact on an existing ABI.

> 
>  - 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, and if FFR can be accessed if not available in streaming
>    mode.
>  - Changes to SVCR.SM zero register data in the same way writes to the
>    physical register do.
>  - This also implies discarding or failing all writes to ZA and ZT0
>    unless SVCR.ZA is set for consistency.

All of that seems reasonable, as long as it is comes as a consequence
of enabling SME. It should be run by the QEMU people though, as they
are the ones that will make use of it. Please Cc them when you post
the patches or even better, reach out to them beforehand.

>  - Add support for the V registers in the sysreg interface when SVE is
>    enabled.

We already support the V registers with KVM_REG_ARM_CORE_REG(). Why
would you add any new interface for this?  The kernel should be
perfectly capable of dealing with the placement of the data in the
internal structures, and there is no need to tie the userspace ABI to
how we deal with that placement (kvm_regs is already purely
userspace).

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

Apart from the statement about the V registers, this seems OK. But
again, I want to see this agreed with the QEMU folks.

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel