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=-5.5 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,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 0E171C43381 for ; Wed, 27 Mar 2019 14:03:26 +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 B7A9D2146F for ; Wed, 27 Mar 2019 14:03:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="S3HHzRMn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B7A9D2146F 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=2SP5C9Gn/BzXHk9N+pAnT/VgY0c93ZRgY29e6oz0/tg=; b=S3HHzRMnjqcpYR HuqAtOQmDjcuFI1dIO3ftCVdBSuxetiT3DN4aEaCqCY52CwXMGX6/o/rZKw3F/Hxcbz0TwG4ER17L Gsrc9RUZ5yA7HY2JmxaeK8eWxkMTH7tVruY2ZULQ6mi+z7yVgJDqvPqiZWLD71INJV/lviRupcXEj HtggOG1vJjKJD/Du6g7HGhbPtri2ybMZtIy79cirY20uiOx0Taf7dCyCK3PCoD+UK5IBPm+z71whe y6xLWGM5wVWps9dqwPhaiWMQ9RpClzAv8fAyH9+QPyCpWCvA4dRy6QEpdKp9ck7eun7mQnv4BDu8P UcUPKMETuNdi2WQcq3Hw==; 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 1h999E-0008Ab-NQ; Wed, 27 Mar 2019 14:03:20 +0000 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70] helo=foss.arm.com) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h999A-00088Y-Mu for linux-arm-kernel@lists.infradead.org; Wed, 27 Mar 2019 14:03:19 +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 151F680D; Wed, 27 Mar 2019 07:03:14 -0700 (PDT) Received: from localhost (unknown [10.37.6.20]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9F1293F59C; Wed, 27 Mar 2019 07:03:13 -0700 (PDT) Date: Wed, 27 Mar 2019 14:03:11 +0000 From: Andrew Murray To: Dave P Martin Subject: Re: [PATCH v2 3/6] arm64: HWCAP: encapsulate elf_hwcap Message-ID: <20190327140311.GC43527@e119886-lin.cambridge.arm.com> References: <1550751657-30252-1-git-send-email-andrew.murray@arm.com> <1550751657-30252-4-git-send-email-andrew.murray@arm.com> <20190221184506.GP16031@e103592.cambridge.arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190221184506.GP16031@e103592.cambridge.arm.com> User-Agent: Mutt/1.10.1+81 (426a6c1) (2018-08-26) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190327_070316_791722_0D2D6606 X-CRM114-Status: GOOD ( 22.97 ) 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: Szabolcs Nagy , Catalin Marinas , Will Deacon , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Feb 21, 2019 at 06:45:08PM +0000, Dave P Martin wrote: > On Thu, Feb 21, 2019 at 12:20:54PM +0000, Andrew Murray wrote: > > The introduction of AT_HWCAP2 introduced accessors which ensure that > > hwcap features are set and tested appropriately. > > > > Let's now mandate access to elf_hwcap via these accessors by making > > elf_hwcap static within cpufeature.c. > > Since elf_hwcap now survives and retains a compatible encoding for > HWCAP_foo, I'm wondering whether it would be simpler to drop this patch. > > Although this will help push people to use the new helpers, the need to > do that seems reduced now. > > People falling off the end of the hwcaps will discover that there is no > HWCAP_foo for the feature they want, only HWCAP2_foo (but no elf_hwcap2 > to look for it in), or KERNEL_HWCAP_foo (which should get them > thinking). > > What do you think? You're right that people will find the appropiate functions to set the relevant hwcaps. But I think my motivation for this comes from the perspective of code maintainability... There is absolutely no functional benefit to exposing the elf_hwcap variable to the rest of the kernel - yet doing so will result in users using elf_hwcap instead of the helpers. If we later change the type of elf_hwcap, or split the bits into multiple variables (e.g. elf_hwcap2) or even change the mapping from UAPI to kernel, then modifications need to be made at each call site. Whereas if we reduce the visibility of elf_hwcap then we encapsulate all the bit fiddling in one place. Also, taking this approach forces us into this ugly suitation... +#ifdef CONFIG_ARM64 + if (cpu_have_feature_name(EVTSTRM)) +#else if (elf_hwcap & HWCAP_EVTSTRM) +#endif It's not nice, but it's a lot less fragile than expecting cross-platform agreement on the bit value of particular hwcaps. Is this a good enough reason to keep it? Thanks, Andrew Murray > > [...] > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index 6a477a3..d57a179 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -35,8 +35,7 @@ > > #include > > #include > > > > -unsigned long elf_hwcap __read_mostly; > > -EXPORT_SYMBOL_GPL(elf_hwcap); > > +static unsigned long elf_hwcap __read_mostly; > > > > #ifdef CONFIG_COMPAT > > #define COMPAT_ELF_HWCAP_DEFAULT \ > > @@ -1909,6 +1908,30 @@ bool this_cpu_has_cap(unsigned int n) > > return false; > > } > > > > +void cpu_set_feature(unsigned int num) > > +{ > > + WARN_ON(num >= MAX_CPU_FEATURES); > > + elf_hwcap |= BIT(num); > > +} > > +EXPORT_SYMBOL_GPL(cpu_set_feature); > > + > > +bool cpu_have_feature(unsigned int num) > > +{ > > + WARN_ON(num >= MAX_CPU_FEATURES); > > + return elf_hwcap & BIT(num); > > +} > > +EXPORT_SYMBOL_GPL(cpu_have_feature); > > + > > +unsigned long cpu_get_elf_hwcap(void) > > +{ > > + return lower_32_bits(elf_hwcap); > > +} > > + > > +unsigned long cpu_get_elf_hwcap2(void) > > +{ > > + return upper_32_bits(elf_hwcap); > > +} > > + > > Similarly, pushing all this out of line to enable elf_hwcap to be hidden > may be more effort than it is really worth (?) > > Cheers > ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel