Re: [PATCH 0/2] arm64/sve: Fix mutating register endianness on big-endian

From: Alan Hayward <Alan.Hayward@arm.com>
To: Dave P Martin <Dave.Martin@arm.com>
Cc: "Will Deacon" <Will.Deacon@arm.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Catalin Marinas" <Catalin.Marinas@arm.com>,
	"Zhang Lei" <zhang.lei@jp.fujitsu.com>,
	"Julien Grall" <Julien.Grall@arm.com>,
	"gdb@sourceware.org" <gdb@sourceware.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, nd <nd@arm.com>
Subject: Re: [PATCH 0/2] arm64/sve: Fix mutating register endianness on big-endian
Date: Tue, 11 Jun 2019 16:16:00 -0000	[thread overview]
Message-ID: <207E140D-AC57-4B0D-B838-94B92BE2B0A0@arm.com> (raw)
In-Reply-To: <20190607154832.GH28398@e103592.cambridge.arm.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2427 bytes --]

> On 7 Jun 2019, at 16:48, Dave Martin <Dave.Martin@arm.com> wrote:
> 
> On Fri, Jun 07, 2019 at 10:38:58AM +0100, Will Deacon wrote:
>> On Thu, Jun 06, 2019 at 05:44:53PM +0100, Dave Martin wrote:
>>> By inspection while debugging something else, I noticed that the byte
>>> order of FPSIMD V-register stores and SVE Z-register stores is not the
>>> same when running on big-endian.
>>> 
>>> This is not properly taken into account when moving between the FPSIMD
>>> and SVE register views inside the kernel, resulting in the bytes of a
>>> V-register getting spontaneously reversed in some situations, from
>>> userspace's point of view.  The signal frame and ptrace interface are
>>> also affected.  The KVM ABI forbids mixing the two views and so should
>>> not be affected.
>>> 
>>> See patch 2 for details.
>>> 
>>> Patch 1 does some trivial preparatory refactoring.
>> 
>> Sorry to be a pain, but would you be able to flip this series round so that
>> the fix doesn't depend on the refactoring, please? That way we can put it
>> into stable without the dependency.
>> 
>>> gdb may or may not be affected by this, depending on how it uses the
>>> NT_PRFPREG and NT_ARM_SVE regsets.  I'll leave it to the developers to
>>> assess that.
>> 
>> Wouldn't this be easy enough to test?
> 
> So, gdb works OK on big-endian but weird stuff happening on both with
> and without the fix.
> 
> There are places in the gdb code itself where it is likely missing
> endianness conversions, but I need to follow up with the gdb folks to
> clarify whether my patch is missing somethingâ€¦

(I added the SVE support for GDB).

Iâ€™ve tried these changes out myself using GDB.
With your changes everything looks good, apart from:
* GDB gets it wrong when the ptrace sve structure contains a fpsimd.
* I need to do some testing around sigcontexts, but again I think GDB
  will need a slight change.
Iâ€™ll get some patches together for GDB.

> The ptrace change is theoretically an ABI break, but since the current
> behaviour is obviously wrong, I consider this a fix.

Iâ€™m happy with this change from GDB's side.

Thanks,
Alan.

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

\x16º&ÖëzÛ«ŸŽôçYb²Ö«r\x18\x1d