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

On Tue, Jun 11, 2019 at 04:16:11PM +0000, Alan Hayward wrote:
> 
> 
> > On 7 Jun 2019, at 16:48, Dave Martin <[email protected]> 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.

OK, thanks for confirming.

Cheers
---Dave