From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122266 invoked by alias); 12 Jun 2019 13:50:20 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 122258 invoked by uid 89); 12 Jun 2019 13:50:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.1 spammy=H*i:sk:87sgsfd, H*f:sk:87sgsfd X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Jun 2019 13:50:18 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C720428; Wed, 12 Jun 2019 06:50:16 -0700 (PDT) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 566AC3F557; Wed, 12 Jun 2019 06:50:15 -0700 (PDT) Date: Wed, 12 Jun 2019 13:50:00 -0000 From: Dave Martin To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: Peter Maydell , "gdb@sourceware.org" , Catalin Marinas , Will Deacon , Zhang Lei , Julien Grall , "richard.henderson@linaro.org" , Alan Hayward , nd , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH 0/2] arm64/sve: Fix mutating register endianness on big-endian Message-ID: <20190612135012.GT28398@e103592.cambridge.arm.com> References: <1559839495-22315-1-git-send-email-Dave.Martin@arm.com> <20190607093858.GA21378@fuggles.cambridge.arm.com> <20190607154832.GH28398@e103592.cambridge.arm.com> <207E140D-AC57-4B0D-B838-94B92BE2B0A0@arm.com> <87v9xbdr3o.fsf@zen.linaroharston> <20190612124712.GR28398@e103592.cambridge.arm.com> <87sgsfdjru.fsf@zen.linaroharston> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87sgsfdjru.fsf@zen.linaroharston> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2019-06/txt/msg00022.txt.bz2 On Wed, Jun 12, 2019 at 02:18:29PM +0100, Alex Bennée wrote: > > Dave Martin writes: > > > On Wed, Jun 12, 2019 at 11:40:11AM +0100, Alex Bennée wrote: > >> > >> Alan Hayward writes: > >> > >> >> On 7 Jun 2019, at 16:48, Dave Martin 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. > > >> >>> > >> >>> 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. > >> > >> Where is the latest state of SVE support for GDB? I really should check > >> the QEMU gdbstub does the correct things for SVE registers but I was > >> waiting for upstream gdb support. > > > > Does this issue need looking at for the QEMU userspace emulation too? > > Hmm I think we are OK. For the SVE frame itself we explicitly store in > LE order: > > static void target_setup_sve_record(struct target_sve_context *sve, > CPUARMState *env, int vq, int size) > { > int i, j; > > __put_user(TARGET_SVE_MAGIC, &sve->head.magic); > __put_user(size, &sve->head.size); > __put_user(vq * TARGET_SVE_VQ_BYTES, &sve->vl); > > /* Note that SVE regs are stored as a byte stream, with each byte element > * at a subsequent address. This corresponds to a little-endian store > * of our 64-bit hunks. > */ > for (i = 0; i < 32; ++i) { > uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i); > for (j = 0; j < vq * 2; ++j) { > __put_user_e(env->vfp.zregs[i].d[j], z + j, le); > } > } > for (i = 0; i <= 16; ++i) { > uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i); > for (j = 0; j < vq; ++j) { > uint64_t r = env->vfp.pregs[i].p[j >> 2]; > __put_user_e(r >> ((j & 3) * 16), p + j, le); > } > } > } > > For the aliased fpsimd registers we store in the target endian format: > > static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd, > CPUARMState *env) > { > int i; > > __put_user(TARGET_FPSIMD_MAGIC, &fpsimd->head.magic); > __put_user(sizeof(struct target_fpsimd_context), &fpsimd->head.size); > __put_user(vfp_get_fpsr(env), &fpsimd->fpsr); > __put_user(vfp_get_fpcr(env), &fpsimd->fpcr); > > for (i = 0; i < 32; i++) { > uint64_t *q = aa64_vfp_qreg(env, i); > #ifdef TARGET_WORDS_BIGENDIAN > __put_user(q[0], &fpsimd->vregs[i * 2 + 1]); > __put_user(q[1], &fpsimd->vregs[i * 2]); > #else > __put_user(q[0], &fpsimd->vregs[i * 2]); > __put_user(q[1], &fpsimd->vregs[i * 2 + 1]); > #endif > } > } That's reassuring. What about where you merge the fpsimd registers back into the SVE regs on sigreturn? > Where our layout for the quads is always: > > Qn = regs[n].d[1]:regs[n].d[0] You mean always on BE? That would be wrong for LE. Cheers ---Dave