Re: [PATCH 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>
Subject: Re: [PATCH 5/5] KVM: arm64: Exclude FP ownership from kvm_vcpu_arch
Date: Wed, 06 Mar 2024 09:43:13 +0000	[thread overview]
Message-ID: <87edcnr8zy.wl-maz@kernel.org> (raw)
In-Reply-To: <6acffbef-6872-4a15-b24a-7a0ec6bbb373@sirena.org.uk>

On Mon, 04 Mar 2024 19:10:08 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> On Sat, Mar 02, 2024 at 11:19:35AM +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).
> 
> The SME patch series proposes adding an additional state to this
> enumeration which would say if the registers are stored in a format
> suitable for exchange with userspace, that would make this state part of
> the vCPU state.  With the addition of SME we can have two vector lengths
> in play so the series proposes picking the larger to be the format for
> userspace registers.

What does this addition have anything to do with the ownership of the
physical register file? Not a lot, it seems.

Specially as there better be no state resident on the CPU when
userspace messes up with it.

> 
> We could store this separately to fp_state/owner but it'd still be a
> value stored in the vCPU.

I totally disagree.

> Storing in a format suitable for userspace
> usage all the time when we've got SME would most likely result in
> performance overhead

What performance overhead? Why should we care?

> if nothing else and feels more complicated than
> rewriting the data in the relatively unusual case where userspace looks
> at it.  Trying to convert userspace writes into the current layout would
> have issues if the current layout uses the smaller vector length and
> create fragility with ordering issues when loading the guest state.

What ordering issues? If userspace manipulates the guest state, the
guest isn't running. If it is, all bets are off.

> 
> The proposal is not the most lovely idea ever but given the architecture
> I think some degree of clunkiness would be unavoidable.

It is only unavoidable if we decide to make a bad job of it.

	M.

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