From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19804 invoked by alias); 22 Aug 2017 14:21:45 -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 18830 invoked by uid 89); 22 Aug 2017 14:21:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=drawn, minded X-Spam-User: qpsmtpd, 2 recipients X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Aug 2017 14:21:42 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1DD2480D; Tue, 22 Aug 2017 07:21:41 -0700 (PDT) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DA0BE3F3E1; Tue, 22 Aug 2017 07:21:38 -0700 (PDT) Date: Tue, 22 Aug 2017 14:21:00 -0000 From: Dave Martin To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel , Szabolcs Nagy , gdb@sourceware.org, Yao Qi , Will Deacon , Richard Sandiford , Alan Hayward , Catalin Marinas , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 09/27] arm64/sve: Signal frame and context structure definition Message-ID: <20170822142135.GU6321@e103592.cambridge.arm.com> References: <1502280338-23002-1-git-send-email-Dave.Martin@arm.com> <1502280338-23002-10-git-send-email-Dave.Martin@arm.com> <87y3qb52ez.fsf@linaro.org> <20170822111705.GT6321@e103592.cambridge.arm.com> <87tw0z4sk2.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87tw0z4sk2.fsf@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2017-08/txt/msg00045.txt.bz2 On Tue, Aug 22, 2017 at 02:53:49PM +0100, Alex Bennée wrote: > > Dave Martin writes: > > > On Tue, Aug 22, 2017 at 11:22:44AM +0100, Alex Bennée wrote: > >> > >> Dave Martin writes: [...] > >> > +/* > >> > + * The SVE architecture leaves space for future expansion of the > >> > + * vector length beyond its initial architectural limit of 2048 bits > >> > + * (16 quadwords). > >> > + */ > >> > +#define SVE_VQ_MIN 1 > >> > +#define SVE_VQ_MAX 0x200 > >> > + > >> > +#define SVE_VL_MIN (SVE_VQ_MIN * 0x10) > >> > +#define SVE_VL_MAX (SVE_VQ_MAX * 0x10) > >> > + > >> > +#define SVE_NUM_ZREGS 32 > >> > +#define SVE_NUM_PREGS 16 > >> > + > >> > +#define sve_vl_valid(vl) \ > >> > + ((vl) % 0x10 == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX) > >> > +#define sve_vq_from_vl(vl) ((vl) / 0x10) > >> > +#define sve_vl_from_vq(vq) ((vq) * 0x10) > >> > >> I got a little confused first time through over what VQ and VL where. > >> Maybe it would make sense to expand a little more from first principles? > >> > >> /* > >> * The SVE architecture defines vector registers as a multiple of 128 > >> * bit quadwords. The current architectural limit is 2048 bits (16 > >> * quadwords) but there is room for future expansion beyond that. > >> */ > > > > This comes up in several places and so I didn't want to comment it > > repeatedly everywhere. > > > > Instead, I wrote up something in section 2 (Vector length terminology) > > of Documentation/arm64/sve.txt -- see patch 25. Can you take a look and > > see whether that's adequate? > > Ahh, I hadn't got to that yet. I'm unsure to the order the kernel likes > to put things but I like to put design documents at the front of the I don't have a strong opinion on that -- I had preferred not to add a document describing stuff that doesn't exist at the time of commit. I could commit a stub document at the start of the series, and then commit the real document later. Either way, it seemed overkill. Perhaps I should have drawn more attention to the documentation in the cover letter, and encouraged reviewers to look at it first. My experience is that people don't often read cover letters... Now the series is posted, I'm minded to keep the order as-is, unless you think it's a big issue. Adding a reference to the document seems a reasonable thing to do, so I could add that. > patch queue as they are useful primers and saves you having to patch a: > > modified arch/arm64/include/uapi/asm/sigcontext.h > @@ -132,19 +132,24 @@ struct sve_context { > /* > * The SVE architecture leaves space for future expansion of the > * vector length beyond its initial architectural limit of 2048 bits > - * (16 quadwords). > + * (16 quadwords). See Documentation/arm64/sve.txt for a summary of > + * the terminology of Vector Quads (VQ) and Vector Lengths (VL). > */ > + > +#define SVE_VQ_BITS 128 /* 128 bits in one quadword */ > +#define SVE_VQ_BYTES (SVE_VQ_BITS / 8) > + I was trying to keep extraneous #defines to a minimum, since this is a uapi header, and people may depend on anything defined here. I think SVE_VQ_BYTES is reasonable to have, and this allows us to rewrite a few hard-coded 0x10s and 16s symbolically which is probably a good idea -- I'll add this. SVE_VQ_BITS looks redundant to me though. It wouldn't be used for any purpose other than defining SVE_VQ_BYTES. > #define SVE_VQ_MIN 1 > #define SVE_VQ_MAX 0x200 > > -#define SVE_VL_MIN (SVE_VQ_MIN * 0x10) > -#define SVE_VL_MAX (SVE_VQ_MAX * 0x10) > +#define SVE_VL_MIN (SVE_VQ_MIN * SVE_VQ_BYTES) > +#define SVE_VL_MAX (SVE_VQ_MAX * SVE_VQ_BYTES) > > #define SVE_NUM_ZREGS 32 > #define SVE_NUM_PREGS 16 > > #define sve_vl_valid(vl) \ > - ((vl) % 0x10 == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX) > + ((vl) % SVE_VQ_BYTES == 0 && (vl) >= SVE_VL_MIN && (vl) <= SVE_VL_MAX) > #define sve_vq_from_vl(vl) ((vl) / 0x10) > #define sve_vl_from_vq(vq) ((vq) * 0x10) [...] Cheers ---Dave