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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 3E9FCC433F5 for ; Thu, 21 Apr 2022 09:35:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=/MW+Fl+m+hDCy5e0c8AFCeIIJxS9vE1OOX3dHcTmRr4=; b=NpObZCp9RG6FVA uEDBulDTY28UDrh7/aO6BYI8EZM+sQF6TokUmtFrh0vuHpoUESJafdhi1tEfyQITSjHrJKIDWsRRf hc073gmlZjzaAL0Xkxga3/VeMZNaFl3fG3seFH5DbZCbaK0aTdeue/zlX7ohV/ArmDiwhsAlrbwS4 /6N0ejjthk0HJMPEfNZZUr1e0cuc54ITqz9bW+yvd03K9s0u9+ckhaYweiJvjA6EpV7xeBGXusERh I+ut3xC60k9myXkoRBzEhFH3WQ0KVSxYN6rl3qFMxUWE8LS77n4baQwVgbzB8ao27cXJ5HzWGlk5A GUgMVVKLvaJgeZvsdRSw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nhTCM-00CmF5-JZ; Thu, 21 Apr 2022 09:34:02 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nhTCH-00CmC7-Qc for linux-arm-kernel@lists.infradead.org; Thu, 21 Apr 2022 09:33:59 +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 64E9A1477; Thu, 21 Apr 2022 02:33:56 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.76.146]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3CDDB3F766; Thu, 21 Apr 2022 02:33:55 -0700 (PDT) Date: Thu, 21 Apr 2022 10:33:51 +0100 From: Mark Rutland To: Mark Brown Cc: Catalin Marinas , Will Deacon , Marc Zyngier , Suzuki K Poulose , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 1/8] arm64/mte: Move shift from definition of TCF0 enumeration values Message-ID: References: <20220419104329.188489-1-broonie@kernel.org> <20220419104329.188489-2-broonie@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220419104329.188489-2-broonie@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220421_023357_994865_F5BAB8E1 X-CRM114-Status: GOOD ( 22.02 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mark, I've obviously in favour of scripting, and the changes below look sound to me, so: Acked-by: Mark Rutland I have a couple of minor comments on the commit message, and one suggestion on the actual code, but the ack stands either way. On Tue, Apr 19, 2022 at 11:43:22AM +0100, Mark Brown wrote: > In preparation for automatic generation of SCTLR_EL1 register definitions > move the shifting of the enumeration values for the TCF0 field from the > defines in the header to the point where they are used. It might be worth saying explicitly that this is because the scripting consistently define enum values *without* shifting. Likewise saying that the MASK definition is deliberately left shifted as that defines the bit position of the field, and can be used with FIELD_GET() / FIELD_PREP(). You can also add something like: There should be no functional change as a result of this patch ... to make it clear that this is just a refactoring. > > Signed-off-by: Mark Brown > --- > arch/arm64/include/asm/sysreg.h | 8 ++++---- > arch/arm64/kernel/mte.c | 6 +++--- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index fbf5f8bb9055..ff7693902686 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -678,10 +678,10 @@ > #define SCTLR_EL1_ATA0 (BIT(42)) > > #define SCTLR_EL1_TCF0_SHIFT 38 > -#define SCTLR_EL1_TCF0_NONE (UL(0x0) << SCTLR_EL1_TCF0_SHIFT) > -#define SCTLR_EL1_TCF0_SYNC (UL(0x1) << SCTLR_EL1_TCF0_SHIFT) > -#define SCTLR_EL1_TCF0_ASYNC (UL(0x2) << SCTLR_EL1_TCF0_SHIFT) > -#define SCTLR_EL1_TCF0_ASYMM (UL(0x3) << SCTLR_EL1_TCF0_SHIFT) > +#define SCTLR_EL1_TCF0_NONE UL(0x0) > +#define SCTLR_EL1_TCF0_SYNC UL(0x1) > +#define SCTLR_EL1_TCF0_ASYNC UL(0x2) > +#define SCTLR_EL1_TCF0_ASYMM UL(0x3) > #define SCTLR_EL1_TCF0_MASK (UL(0x3) << SCTLR_EL1_TCF0_SHIFT) > > #define SCTLR_EL1_BT1 (BIT(36)) > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c > index 78b3e0f8e997..77614f8bc463 100644 > --- a/arch/arm64/kernel/mte.c > +++ b/arch/arm64/kernel/mte.c > @@ -216,11 +216,11 @@ static void mte_update_sctlr_user(struct task_struct *task) > * default order. > */ > if (resolved_mte_tcf & MTE_CTRL_TCF_ASYMM) > - sctlr |= SCTLR_EL1_TCF0_ASYMM; > + sctlr |= SCTLR_EL1_TCF0_ASYMM << SCTLR_EL1_TCF0_SHIFT; Generally we should probably use FIELD_PREP()/FIELD_GET() in C code, so maybe we should make this: sctlr |= FIELD_PREP(SCTLR_EL1_TCF0_MASK, SCTLR_EL1_TCF0_ASYMM); Going forward, it might be worth having some helpers that allow us to avoid the redundant bits, and save humans having to care about how we namespace the definitions, e.g. sctlr |= SYS_FIELD_PREP(SCTLR_EL1, TCF0, 0x0); is: sctlr |= FIELD_PREP(SCTLR_EL1_TCF0_MASK, 0x0); sctlr |= SYS_FIELD_PREP_ENUM(SCTLR_EL1, TCF0, ASYMM); is sctlr |= FIELD_PREP(SCTLR_EL1_TCF0_MASK, SCTLR_EL1_TCF0_ASYMM); Thanks, Mark. > else if (resolved_mte_tcf & MTE_CTRL_TCF_ASYNC) > - sctlr |= SCTLR_EL1_TCF0_ASYNC; > + sctlr |= SCTLR_EL1_TCF0_ASYNC << SCTLR_EL1_TCF0_SHIFT; > else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC) > - sctlr |= SCTLR_EL1_TCF0_SYNC; > + sctlr |= SCTLR_EL1_TCF0_SYNC << SCTLR_EL1_TCF0_SHIFT; > task->thread.sctlr_user = sctlr; > } > > -- > 2.30.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel