From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 808B9C4360F for ; Fri, 5 Apr 2019 09:36:14 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 52371217D4 for ; Fri, 5 Apr 2019 09:36:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UgPInySg" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 52371217D4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=1Hom/L/w8v5p2CvYLHUZ8DXpoApipt1M8OlxVvczd/w=; b=UgPInySg8wqguR z1s/woKI33izXkOwwGjjZAJHBQt04Wxj+Li1XOG7ET3aow2orHBMcA7CTs328Lq9JDicQhw2b7jUr fZ/OSOw02cNaM3kEz4b6TD9jSVZCJrqlnh8D3UgdUlaBorFBF2jxheUHk/YYqBAk4dVzXn+sUIYAO W4c81nT4LRyKiOMcz4qhgPMt1YZlptwLnUdTPjpJL1qDKtCXenRYiA6xTotwDNfkMapg7uNSsmsb7 TTnOjdydjmjF9nksnWbA0+o+VxDVQkh3QrVHbHSjlJ4xgmFj5YSglQhhEk2g2jG45vKwCndSDl66X i7xgdhoRqBNxBnU6BnOg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hCLGb-00031q-AR; Fri, 05 Apr 2019 09:36:09 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hCLGS-0002rV-Hk for linux-arm-kernel@lists.infradead.org; Fri, 05 Apr 2019 09:36:05 +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 59AF2168F; Fri, 5 Apr 2019 02:36:00 -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 68FBC3F721; Fri, 5 Apr 2019 02:35:58 -0700 (PDT) Date: Fri, 5 Apr 2019 10:35:55 +0100 From: Dave Martin To: Andrew Jones Subject: Re: [PATCH v7 20/27] arm64/sve: In-kernel vector length availability query interface Message-ID: <20190405093555.GL3567@e103592.cambridge.arm.com> References: <1553864452-15080-1-git-send-email-Dave.Martin@arm.com> <1553864452-15080-21-git-send-email-Dave.Martin@arm.com> <20190404142034.ikkn7lkgbnntsziq@kamzik.brq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190404142034.ikkn7lkgbnntsziq@kamzik.brq.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190405_023601_333588_9A9A172F X-CRM114-Status: GOOD ( 27.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Okamoto Takayuki , Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , Zhang Lei , Julien Grall , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 04, 2019 at 04:20:34PM +0200, Andrew Jones wrote: > On Fri, Mar 29, 2019 at 01:00:45PM +0000, Dave Martin wrote: > > KVM will need to interrogate the set of SVE vector lengths > > available on the system. > > = > > This patch exposes the relevant bits to the kernel, along with a > > sve_vq_available() helper to check whether a particular vector > > length is supported. > > = > > __vq_to_bit() and __bit_to_vq() are not intended for use outside > > these functions: now that these are exposed outside fpsimd.c, they > > are prefixed with __ in order to provide an extra hint that they > > are not intended for general-purpose use. > > = > > Signed-off-by: Dave Martin > > Reviewed-by: Alex Benn=E9e > > Tested-by: zhang.lei > > --- > > arch/arm64/include/asm/fpsimd.h | 29 +++++++++++++++++++++++++++++ > > arch/arm64/kernel/fpsimd.c | 35 ++++++++-------------------------= -- > > 2 files changed, 37 insertions(+), 27 deletions(-) > > = > > diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/f= psimd.h > > index df7a143..ad6d2e4 100644 > > --- a/arch/arm64/include/asm/fpsimd.h > > +++ b/arch/arm64/include/asm/fpsimd.h > > @@ -24,10 +24,13 @@ > > = > > #ifndef __ASSEMBLY__ > > = > > +#include > > #include > > +#include > > #include > > #include > > #include > > +#include > > = > > #if defined(__KERNEL__) && defined(CONFIG_COMPAT) > > /* Masks for extracting the FPSR and FPCR from the FPSCR */ > > @@ -89,6 +92,32 @@ extern u64 read_zcr_features(void); > > = > > extern int __ro_after_init sve_max_vl; > > extern int __ro_after_init sve_max_virtualisable_vl; > > +/* Set of available vector lengths, as vq_to_bit(vq): */ > = > s/as/for use with/ ? Not exactly. Does the following work for you: /* * Set of available vector lengths * Vector length vq is encoded as bit __vq_to_bit(vq): */ > s/vq_to_bit/__vq_to_bit/ Ack: that got renamed when I moved it to fpsimd.h, bit I clearly didn't update the comment as I pasted it across. > = > > +extern __ro_after_init DECLARE_BITMAP(sve_vq_map, SVE_VQ_MAX); > > + > > +/* > > + * Helpers to translate bit indices in sve_vq_map to VQ values (and > > + * vice versa). This allows find_next_bit() to be used to find the > > + * _maximum_ VQ not exceeding a certain value. > > + */ > > +static inline unsigned int __vq_to_bit(unsigned int vq) > > +{ > = > Why not have the same WARN_ON and clamping here as we do > in __bit_to_vq. Here a vq > SVE_VQ_MAX will wrap around > to a super high bit. > = > > + return SVE_VQ_MAX - vq; > > +} > > + > > +static inline unsigned int __bit_to_vq(unsigned int bit) > > +{ > > + if (WARN_ON(bit >=3D SVE_VQ_MAX)) > > + bit =3D SVE_VQ_MAX - 1; > > + > > + return SVE_VQ_MAX - bit; > > +} > > + > > +/* Ensure vq >=3D SVE_VQ_MIN && vq <=3D SVE_VQ_MAX before calling this= function */ > = > Are we avoiding putting these tests and WARN_ONs in this function to > keep it fast? These are intended as backend for use only by fpsimd.c and this header, so peppering them with WARN_ON() felt excessive. I don't expect a lot of new calls to these (or any, probably). I don't recall why I kept the WARN_ON() just in __bit_to_vq(), except that the way that gets called is a bit more complex in some places. Are you happy to replace these with comments? e.g.: /* Only valid when vq >=3D SVE_VQ_MIN && vq <=3D SVE_VQ_MAX */ __vq_to_bit() /* Only valid when bit < SVE_VQ_MAX */ __bit_to_vq() OTOH, these are not used on fast paths, so maybe having both as WARN_ON() would be better. Part of the problem is knowing what to clamp to: these are generally used in conjunction with looping or bitmap find operations, so the caller may be making assumptions about the return value that may wrong when the value is clamped. Alternatively, these could be BUG() -- but that seems heavy. What do you think? [...] Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel