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=-10.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 CE52DC48BDF for ; Tue, 15 Jun 2021 21:47:13 +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 79DC361159 for ; Tue, 15 Jun 2021 21:47:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 79DC361159 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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.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=q+ec4twB7aJvtwzpbWBSrsULpYT0os0eb4GAjTpsm9k=; b=x3XCbOawr1EH36 b66CRHkcZaxgowOqsZvTmU/92cb+Trgls5Q9c2rXXTyaZ9eMij2wWHow+DfZeIgQu1Q4RSE9kN+Xn 6QfhGwR/PnehZVjRnLDfY02NKkx8J+Hm949KbsHLQQXnpo4VzVPL2QN9h17trjMhXGLTmmgVU0/wa bHY8TxU+uDMp6+3BVZz5e+ekgiJndrcbtEfz2WBZg34HjZqiotRJTDMzWcyJKqww0WfvOn6ngJg/O HJie+40db94GcYGNFiiMxIw0Zzvt3XCo9WG9w5/HL5UZueXA42+lVXn9l+RjtP+7dOKkNI0OsZzgP aVCWA50aY3phJxy8lgTw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ltGrt-003Hdk-Ap; Tue, 15 Jun 2021 21:45:09 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ltDOP-001wus-NQ for linux-arm-kernel@lists.infradead.org; Tue, 15 Jun 2021 18:02:31 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id E454161042; Tue, 15 Jun 2021 18:02:27 +0000 (UTC) Date: Tue, 15 Jun 2021 19:02:25 +0100 From: Catalin Marinas To: Peter Collingbourne Cc: Vincenzo Frascino , Will Deacon , Evgenii Stepanov , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis Message-ID: <20210615180224.GC13005@arm.com> References: <20210615015728.3232519-1-pcc@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210615015728.3232519-1-pcc@google.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210615_110229_872485_E8C1F08D X-CRM114-Status: GOOD ( 29.47 ) 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 On Mon, Jun 14, 2021 at 06:57:28PM -0700, Peter Collingbourne wrote: > @@ -120,6 +120,25 @@ in the ``PR_MTE_TAG_MASK`` bit-field. > interface provides an include mask. An include mask of ``0`` (exclusion > mask ``0xffff``) results in the CPU always generating tag ``0``. > > +Upgrading to stricter tag checking modes > +---------------------------------------- > + > +On some CPUs the performance of MTE in stricter tag checking modes > +is similar to that of less strict tag checking modes. This makes it > +worthwhile to enable stricter checks on those CPUs when a less strict > +checking mode is requested, in order to gain the error detection > +benefits of the stricter checks without the performance downsides. To > +opt into upgrading to a stricter checking mode on those CPUs, the user > +can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument > +to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call. > + > +This feature is currently only supported for upgrading from > +asynchronous mode. To configure a CPU to upgrade from asynchronous mode > +to synchronous mode, a privileged user may write the value ``1`` to > +``/sys/devices/system/cpu/cpu/mte_upgrade_async``, and to disable > +upgrading they may write the value ``2``. By default the feature is > +disabled on all CPUs. This needs updated as well to for 0 as disabled. I wonder whether we could generalise this to something like mte_tcf_upgrade and allow asymmetric to be expanded to sync. Otherwise we'd have to add another interface when we know that if a CPU can handle sync as fast as async, the asymmetric mode should also be upgraded. So a more generic mte_tcf_upgrade just holds the strictest that the CPU can handle without significant performance degradation. The mte_upgrade_async can be confusing as well for the asymmetric mode where the writes are asynchronous. > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c > index 125a10e413e9..ad85e8519669 100644 > --- a/arch/arm64/kernel/mte.c > +++ b/arch/arm64/kernel/mte.c > @@ -4,6 +4,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -26,6 +27,9 @@ u64 gcr_kernel_excl __ro_after_init; > > static bool report_fault_once = true; > > +DEFINE_PER_CPU_READ_MOSTLY(u64, mte_upgrade_async); > +EXPORT_PER_CPU_SYMBOL(mte_upgrade_async); I think this should be static and not exported, unless I missed its use elsewhere. > @@ -216,15 +210,33 @@ void mte_thread_init_user(void) > dsb(ish); > write_sysreg_s(0, SYS_TFSRE0_EL1); > clear_thread_flag(TIF_MTE_ASYNC_FAULT); > - /* disable tag checking */ > - set_task_sctlr_el1((current->thread.sctlr_user & ~SCTLR_EL1_TCF0_MASK) | > - SCTLR_EL1_TCF0_NONE); > - /* reset tag generation mask */ > - set_gcr_el1_excl(SYS_GCR_EL1_EXCL_MASK); > + /* disable tag checking and reset tag generation mask */ > + current->thread.mte_ctrl = > + MTE_CTRL_GCR_USER_EXCL_MASK | MTE_CTRL_TCF_NONE; > + mte_update_sctlr_user(current); > + set_task_sctlr_el1(current->thread.sctlr_user); > +} > + > +void mte_update_sctlr_user(struct task_struct *task) > +{ > + unsigned long sctlr = task->thread.sctlr_user; > + > + sctlr &= ~SCTLR_EL1_TCF0_MASK; > + if ((task->thread.mte_ctrl & MTE_CTRL_DYNAMIC_TCF) && > + (task->thread.mte_ctrl & MTE_CTRL_TCF_MASK) == MTE_CTRL_TCF_ASYNC) { > + sctlr |= __this_cpu_read(mte_upgrade_async); If we consider 0 to mean "disable upgrade", you'd just need another check here before the write. But it may simplify some of the sysfs code to avoid the switch statement and the pre-initialisation of mte_upgrade_async. > + } else { > + sctlr |= ((task->thread.mte_ctrl & MTE_CTRL_TCF_MASK) >> > + MTE_CTRL_TCF_SHIFT) > + << SCTLR_EL1_TCF0_SHIFT; Nitpick: we tend to place the operator on the previous line you can probably place the shift constant on that line as well. > + } > + task->thread.sctlr_user = sctlr; I think on the "else" path, we shouldn't bother updating sctlr_user, though it probably needs some tweaking of the prctl() path. Otherwise the patch looks in the right direction. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel