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=-6.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_NEOMUTT 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 A04C0C10F11 for ; Wed, 24 Apr 2019 07:33:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 75BA32089F for ; Wed, 24 Apr 2019 07:33:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730215AbfDXHdH (ORCPT ); Wed, 24 Apr 2019 03:33:07 -0400 Received: from foss.arm.com ([217.140.101.70]:38972 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729772AbfDXHdG (ORCPT ); Wed, 24 Apr 2019 03:33:06 -0400 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 6EC5980D; Wed, 24 Apr 2019 00:33:06 -0700 (PDT) Received: from blommer (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 87ED53F238; Wed, 24 Apr 2019 00:33:03 -0700 (PDT) Date: Wed, 24 Apr 2019 08:33:01 +0100 From: Mark Rutland To: Kees Cook Cc: Catalin Marinas , Will Deacon , Arnd Bergmann , Alex Matveev , Ard Biesheuvel , Nick Desaulniers , Yury Norov , Matthias Kaehlcke , Sami Tolvanen , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4] arm64: sysreg: Make mrs_s and msr_s macros work with Clang and LTO Message-ID: <20190424073258.d5gv7s7ij5zujd2l@blommer> References: <20190423232622.GA11776@beast> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190423232622.GA11776@beast> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Kees, On Tue, Apr 23, 2019 at 04:26:22PM -0700, Kees Cook wrote: > Clang's integrated assembler does not allow assembly macros defined > in one inline asm block using the .macro directive to be used across > separate asm blocks. LLVM developers consider this a feature and not a > bug, recommending code refactoring: > > https://bugs.llvm.org/show_bug.cgi?id=19749 > > As binutils doesn't allow macros to be redefined, this change uses > UNDEFINE_MRS_S and UNDEFINE_MSR_S to define corresponding macros > in-place and workaround gcc and clang limitations on redefining macros > across different assembler blocks. [...] > diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h > index 43d8366c1e87..d359f28765cb 100644 > --- a/arch/arm64/include/asm/irqflags.h > +++ b/arch/arm64/include/asm/irqflags.h > @@ -43,7 +43,7 @@ static inline void arch_local_irq_enable(void) > asm volatile(ALTERNATIVE( > "msr daifclr, #2 // arch_local_irq_enable\n" > "nop", > - "msr_s " __stringify(SYS_ICC_PMR_EL1) ",%0\n" > + __msr_s(SYS_ICC_PMR_EL1) Seeing the __stringify() go is nice! Our assembly happens to use argument 0 by coincidence rather than by deliberate design, so please have the macro take the template, e.g. __msr_s(SYS_ICC_PMR_EL1, "%x0") ... that ways it's obvious as to which asm parameter is used, and this won't complicate refactoring of the assembly (which may shuffle or rename the parameters). Likewise for __mrs_s(), e.g. __mrs_s("%x[va]", SYS_WHATEVER); [...] > +#define __mrs_s(r) \ > + DEFINE_MRS_S \ > +" mrs_s %0, " __stringify(r) "\n" \ > + UNDEFINE_MRS_S > + > +#define __msr_s(r) \ > + DEFINE_MSR_S \ > +" msr_s " __stringify(r) ", %0\n" \ > + UNDEFINE_MSR_S These can be: #define __mrs_s(v, r) \ DEFINE_MRS_S \ " mrs_s " v ", " __stringify(r) "\n" \ UNDEFINE_MRS_S #define __msr_s(r, v) \ DEFINE_MSR_S \ " msr_s " __stringify(r) ", " v "\n" \ UNDEFINE_MSR_S > + > +/* For use with "rZ" constraints. */ > +#define __msr_s_x0(r) \ > + DEFINE_MSR_S \ > +" msr_s " __stringify(r) ", %x0\n" \ > + UNDEFINE_MSR_S I don't think we need this. If we pass the template in, this is solved by the caller, and we could consistently use the x form regardless. Otherwise, I think this is as clean as we can make this short of bumping our minimum binutils version and using the S_<...> names. Thanks, Mark. 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.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_NEOMUTT autolearn=unavailable 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 45689C10F11 for ; Wed, 24 Apr 2019 07:33:12 +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 155722089F for ; Wed, 24 Apr 2019 07:33:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="J8C2HoOs" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 155722089F 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=g+ODLhjymYByPT3TDfUi8KqjnOYCOzCKU2LHpOctbN0=; b=J8C2HoOsvlV4UZ gClX0mp5/BXE8FmBOk7htSfZoyxON7fxEh+H7wTYaNO0oDQd9nPAtHS0PXJeymh7xuC/pvBUHhWfT w2Wa0zg9z/AcGTv3YQNvpkA5IxC/fjeIqoODTYeL/VMCjeD3eLWHkfIQeuViSURDBsWXEnOSE6ZYA ktC/eOGvKtyAN4+goEZA3AhbPE3Y0Dr9uwu4sfrYGFfAntA2Vej6uQHXBczXw03hPnQvrLYAJeOeM 362l96QUg3D5QFWIPJh6AVz7Y+bePdPw3lUw9Buj0aV0IfES8f+yVlNicjSlojvT59aocHAUrVPuO 7x8HBZnA2AUTTWkRQEJQ==; 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 1hJCP1-0004rC-8v; Wed, 24 Apr 2019 07:33:11 +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 1hJCOx-0004qV-2Q for linux-arm-kernel@lists.infradead.org; Wed, 24 Apr 2019 07:33:08 +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 6EC5980D; Wed, 24 Apr 2019 00:33:06 -0700 (PDT) Received: from blommer (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 87ED53F238; Wed, 24 Apr 2019 00:33:03 -0700 (PDT) Date: Wed, 24 Apr 2019 08:33:01 +0100 From: Mark Rutland To: Kees Cook Subject: Re: [PATCH v4] arm64: sysreg: Make mrs_s and msr_s macros work with Clang and LTO Message-ID: <20190424073258.d5gv7s7ij5zujd2l@blommer> References: <20190423232622.GA11776@beast> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190423232622.GA11776@beast> User-Agent: NeoMutt/20170113 (1.7.2) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190424_003307_113035_44D25174 X-CRM114-Status: GOOD ( 17.57 ) 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: Arnd Bergmann , Ard Biesheuvel , Catalin Marinas , Nick Desaulniers , linux-kernel@vger.kernel.org, Will Deacon , Yury Norov , Sami Tolvanen , Alex Matveev , Matthias Kaehlcke , 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 Hi Kees, On Tue, Apr 23, 2019 at 04:26:22PM -0700, Kees Cook wrote: > Clang's integrated assembler does not allow assembly macros defined > in one inline asm block using the .macro directive to be used across > separate asm blocks. LLVM developers consider this a feature and not a > bug, recommending code refactoring: > > https://bugs.llvm.org/show_bug.cgi?id=19749 > > As binutils doesn't allow macros to be redefined, this change uses > UNDEFINE_MRS_S and UNDEFINE_MSR_S to define corresponding macros > in-place and workaround gcc and clang limitations on redefining macros > across different assembler blocks. [...] > diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h > index 43d8366c1e87..d359f28765cb 100644 > --- a/arch/arm64/include/asm/irqflags.h > +++ b/arch/arm64/include/asm/irqflags.h > @@ -43,7 +43,7 @@ static inline void arch_local_irq_enable(void) > asm volatile(ALTERNATIVE( > "msr daifclr, #2 // arch_local_irq_enable\n" > "nop", > - "msr_s " __stringify(SYS_ICC_PMR_EL1) ",%0\n" > + __msr_s(SYS_ICC_PMR_EL1) Seeing the __stringify() go is nice! Our assembly happens to use argument 0 by coincidence rather than by deliberate design, so please have the macro take the template, e.g. __msr_s(SYS_ICC_PMR_EL1, "%x0") ... that ways it's obvious as to which asm parameter is used, and this won't complicate refactoring of the assembly (which may shuffle or rename the parameters). Likewise for __mrs_s(), e.g. __mrs_s("%x[va]", SYS_WHATEVER); [...] > +#define __mrs_s(r) \ > + DEFINE_MRS_S \ > +" mrs_s %0, " __stringify(r) "\n" \ > + UNDEFINE_MRS_S > + > +#define __msr_s(r) \ > + DEFINE_MSR_S \ > +" msr_s " __stringify(r) ", %0\n" \ > + UNDEFINE_MSR_S These can be: #define __mrs_s(v, r) \ DEFINE_MRS_S \ " mrs_s " v ", " __stringify(r) "\n" \ UNDEFINE_MRS_S #define __msr_s(r, v) \ DEFINE_MSR_S \ " msr_s " __stringify(r) ", " v "\n" \ UNDEFINE_MSR_S > + > +/* For use with "rZ" constraints. */ > +#define __msr_s_x0(r) \ > + DEFINE_MSR_S \ > +" msr_s " __stringify(r) ", %x0\n" \ > + UNDEFINE_MSR_S I don't think we need this. If we pass the template in, this is solved by the caller, and we could consistently use the x form regardless. Otherwise, I think this is as clean as we can make this short of bumping our minimum binutils version and using the S_<...> names. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel