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.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no 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 D7889C07E95 for ; Tue, 13 Jul 2021 17:29:23 +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 9D14A6127C for ; Tue, 13 Jul 2021 17:29:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9D14A6127C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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=jFeZlE7xcwH6zWWb23KXEui3qRGAjnhNQvg9jDXxdCQ=; b=J+18rrtZk8ebUw 05BoWvujnKQfL2t8BdcJmnHDDdcHUi+brjU+HMGXe7yZq/0/oRafXYm8Xwi7O0HefYYObvU9K2ZzN lQ7BF+sz0+n8UGJ+44ax9voj6iRF2iraVVf29SC7BUkVp3ac73XW3g0rjQIqOPH5+ueaWdK/99od8 PP+0pfmU4gNRjruCS1Hma8bUgpBKbhadbeO1sS9RkJjp8yKCZ+TX5e0RWUPyiS9R19r389kgawYsZ 2rji5a7E4zEuElUugyToZXsB5BUdJHZomTBUDFYoZZ5tZpelvU+FHYfdT0k06os3u8IfkPyGkIIAs p2E7D5Y+rEVX+XMl7qbw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m3MCM-00B2qV-5w; Tue, 13 Jul 2021 17:27:58 +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 1m3MCI-00B2q5-DS for linux-arm-kernel@lists.infradead.org; Tue, 13 Jul 2021 17:27:55 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id A7C226127C; Tue, 13 Jul 2021 17:27:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1626197274; bh=JqI2ATacaYbNNJghgErZN4k/U0+XwNWxO0zs9p3KQaU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XStAXDKyqvE2BS41bTLlkZeQ5A/m+YLBz85v6X4IoMSzou8bVHqeDR0ZuZUBGHi4L smOkgUr3+jlBgcVZ+susk3wA+ap6AyAOJ9iR/pB7yzs+9YBstKTVKUyKewZwhzAUjW ibq30cvWWfIAJFPjaZlUFyqKZnAlaFSWt54wqqtf61Q4bsfm24ahbfuHWOZVUMcML+ m8w7HxjsFbu4SgGCiBr+LWu55Em6EbklMVTFqOUTto6LY+YjWQ483E2EYjPl3V8sGJ QFMHilCfEPjKKKsSMvsoRQpAlvSLun1/hWGAfS8dAxUSSf6OkqSOmrS2EcT3TWkOEy nQ7dTT/IitDMw== Date: Tue, 13 Jul 2021 18:27:49 +0100 From: Will Deacon To: Peter Collingbourne Cc: Catalin Marinas , Vincenzo Frascino , Evgenii Stepanov , Szabolcs Nagy , Tejas Belagod , linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman Subject: Re: [PATCH v9 2/4] arm64: mte: change ASYNC and SYNC TCF settings into bitfields Message-ID: <20210713172748.GB30304@willie-the-truck> References: <20210702194110.2045282-1-pcc@google.com> <20210702194110.2045282-3-pcc@google.com> <20210707111054.GA21926@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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-20210713_102754_532783_257314A8 X-CRM114-Status: GOOD ( 19.52 ) 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, Jul 12, 2021 at 12:04:39PM -0700, Peter Collingbourne wrote: > On Wed, Jul 7, 2021 at 4:11 AM Will Deacon wrote: > > On Fri, Jul 02, 2021 at 12:41:08PM -0700, Peter Collingbourne wrote: > > > long set_mte_ctrl(struct task_struct *task, unsigned long arg) > > > { > > > - u64 sctlr = task->thread.sctlr_user & ~SCTLR_EL1_TCF0_MASK; > > > u64 mte_ctrl = (~((arg & PR_MTE_TAG_MASK) >> PR_MTE_TAG_SHIFT) & > > > SYS_GCR_EL1_EXCL_MASK) << MTE_CTRL_GCR_USER_EXCL_SHIFT; > > > > > > if (!system_supports_mte()) > > > return 0; > > > > > > - switch (arg & PR_MTE_TCF_MASK) { > > > - case PR_MTE_TCF_NONE: > > > - sctlr |= SCTLR_EL1_TCF0_NONE; > > > - break; > > > - case PR_MTE_TCF_SYNC: > > > - sctlr |= SCTLR_EL1_TCF0_SYNC; > > > - break; > > > - case PR_MTE_TCF_ASYNC: > > > - sctlr |= SCTLR_EL1_TCF0_ASYNC; > > > - break; > > > - default: > > > - return -EINVAL; > > > - } > > > + if (arg & PR_MTE_TCF_ASYNC) > > > + mte_ctrl |= MTE_CTRL_TCF_ASYNC; > > > + if (arg & PR_MTE_TCF_SYNC) > > > + mte_ctrl |= MTE_CTRL_TCF_SYNC; > > > > > > - if (task != current) { > > > - task->thread.sctlr_user = sctlr; > > > - task->thread.mte_ctrl = mte_ctrl; > > > - } else { > > > - set_task_sctlr_el1(sctlr); > > > - set_gcr_el1_excl(mte_ctrl); > > > + task->thread.mte_ctrl = mte_ctrl; > > > + if (task == current) { > > > + mte_update_sctlr_user(task); > > > > In conjunction with the next patch, what happens if we migrate at this > > point? I worry that we can install a stale sctlr_user value. > > > > > + set_task_sctlr_el1(task->thread.sctlr_user); > > In this case, we will call mte_update_sctlr_user when scheduled onto > the new CPU as a result of the change to mte_thread_switch, and both > the scheduler and prctl will set SCTLR_EL1 to the new (correct) value > for the current CPU. Doesn't that rely on task->thread.sctlr_user being explicitly read on the new CPU? For example, the following rough sequence is what I'm worried about: CPU x (prefer ASYNC) set_mte_ctrl(ASYNC | SYNC) current->thread.mte_ctrl = ASYNC | SYNC; mte_update_sctlr_user current->thread.sctlr_user = ASYNC; Register Xn = current->thread.sctlr_user; // ASYNC CPU y (prefer SYNC) mte_thread_switch mte_update_sctlr_user next->thread.sctlr_user = SYNC; update_sctlr_el1 SCTLR_EL1 = SYNC; set_task_sctlr_el1(Xn); // ASYNC current->thread.sctlr_user = Xn; // ASYNC XXX: also superfluous? SCTLR_EL1 = ASYNC; Does that make sense? I'm thinking set_mte_ctrl() should be using update_sctlr_el1() and disabling preemption around the whole thing, which would make it a lot closer to the context-switch path. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel