linux-kernel - Re: [PATCH v4 03/14] arm64/fpsimd: Support FEAT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 6 Mar 2024 16:41:42 +0000
From: Mark Brown <broonie@...nel.org>
To: Marc Zyngier <maz@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>,
	Oliver Upton <oliver.upton@...ux.dev>,
	James Morse <james.morse@....com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Dave Martin <dave.martin@....com>, kvmarm@...ts.linux.dev,
	linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v4 03/14] arm64/fpsimd: Support FEAT_FPMR

On Fri, Feb 23, 2024 at 11:07:02AM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@...nel.org> wrote:

> > --- a/arch/arm64/kvm/fpsimd.c
> > +++ b/arch/arm64/kvm/fpsimd.c
> > @@ -153,6 +153,7 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
> >  		fp_state.sve_vl = vcpu->arch.sve_max_vl;
> >  		fp_state.sme_state = NULL;
> >  		fp_state.svcr = &vcpu->arch.svcr;
> > +		fp_state.fpmr = &vcpu->arch.fpmr;
> >  		fp_state.fp_type = &vcpu->arch.fp_type;

> Given the number of fields you keep track of, it would make a lot more
> sense if these FP-related fields were in their own little structure
> and tracked by a single pointer (I don't think there is a case where
> we track them independently).

I sent a patch series that attempted to address this somewhat by
refactoring things so that we initialise the struct we bind during vCPU
setup rather than each time we prepare to switch away from the guest:

    https://lore.kernel.org/linux-arm-kernel/20240226-kvm-arm64-group-fp-data-v1-0-07d13759517e@kernel.org/

which you weren't terribly enthusiastic about:

    https://lore.kernel.org/all/86y1b027mc.wl-maz@kernel.org/

As covered in the changelog for patch 2 of that series the FP state
includes both system registers which KVM wants to expose via it's system
register ABI and FPSIMD state which the host kernel wants to flag as
suitable for hardended usercopy by embedding in thread_struct.uw.  This
means that pulling everything into a single structure which is used
throughout the kernel would require a bunch of surgery which I'm not
sure is worth the effort or ongoing maintainance cost. 

The current approach is verbose but not complicated and localised to the
code managing the FP state, as I said in the discussion on that thread I
think it represents a reasonable tradeoff.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)