Re: [PATCH v7 1/2] arm64/sve: Split TIF_SVE into separate execute and register state flags

From: Mark Brown <broonie@kernel.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Julien Grall <julien@xen.com>, Julien Grall <julien@xen.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Zhang Lei <zhang.lei@jp.fujitsu.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Daniel Kiss <Daniel.Kiss@arm.com>
Subject: Re: [PATCH v7 1/2] arm64/sve: Split TIF_SVE into separate execute and register state flags
Date: Wed, 10 Feb 2021 17:14:42 +0000	[thread overview]
Message-ID: <20210210171442.GC4748@sirena.org.uk> (raw)
In-Reply-To: <20210210154249.GK21837@arm.com>

[-- Attachment #1.1: Type: text/plain, Size: 1692 bytes --]

On Wed, Feb 10, 2021 at 03:42:51PM +0000, Dave Martin wrote:
> On Wed, Feb 10, 2021 at 02:54:52PM +0000, Mark Brown wrote:

> > Oh, *that's* what that's all about.  I spent quite a bit of time trying
> > to figure out why we were sometimes using vq_minus_1 but never managed
> > to get to the bottom of it - it's an awkward name and there's nothing in
> > the code that explains the logic behind when we use it so it was really
> > confusing.  We can do the rename but I'm not sure it's achieving the
> > goal of comprehensibility.

> Ah, I see.  The reason for the difference is that the vector length is
> encoded in ZCR_ELx.LEN as the vector length in quadwords ("vq" -- see
> Documentation/arm64/sve.rst) minus one.  It seemed poor practice to do
> the conversion in asm where the compiler can't see or optimise it, plus
> I didn't want the possibility of passing meaningless values at that
> level.  So the caller has to validate the vector length with
> sve_vl_valid() where deemed necessary, and then convert explicitly.

Yeah, it's relatively clear to get to the fact that it's due to the
ZCR_ELx.LEN - what was not at all clear was what the rule for choosing
between the two representations was, my instinct would've been to hide
the different representation, something like a static inline wrapper for
the assembly function would still let the compiler see what's going on.  

> Either way, calling this "vl" is breaking a useful convention that's
> followed throughout the rest of the kernel, so I'd prefer we call it
> something else -- but within reason, I don't mind what name is used.

That's the convention in the arm64 FP code or something else?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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