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=-17.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT, USER_IN_DEF_DKIM_WL 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 690D3C43603 for ; Fri, 20 Dec 2019 01:49:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 16D6F2465E for ; Fri, 20 Dec 2019 01:49:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="QnD2X8ug" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 16D6F2465E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A5BA58E0187; Thu, 19 Dec 2019 20:49:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A0C028E0184; Thu, 19 Dec 2019 20:49:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9215C8E0187; Thu, 19 Dec 2019 20:49:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0176.hostedemail.com [216.40.44.176]) by kanga.kvack.org (Postfix) with ESMTP id 7D1848E0184 for ; Thu, 19 Dec 2019 20:49:21 -0500 (EST) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id 27BAD181AC9B6 for ; Fri, 20 Dec 2019 01:49:21 +0000 (UTC) X-FDA: 76283837322.04.fight94_5199c45a27a40 X-HE-Tag: fight94_5199c45a27a40 X-Filterd-Recvd-Size: 9531 Received: from mail-yw1-f74.google.com (mail-yw1-f74.google.com [209.85.161.74]) by imf04.hostedemail.com (Postfix) with ESMTP for ; Fri, 20 Dec 2019 01:49:20 +0000 (UTC) Received: by mail-yw1-f74.google.com with SMTP id j137so5545850ywa.8 for ; Thu, 19 Dec 2019 17:49:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=HZMnRQf2RzyKu0EQDfD1pRubT2qoNtln9gX9VZl7+Lw=; b=QnD2X8ugKAM+uICpb7uuNo+E46l8sDMOx9E2HllXNNfZ3iUHNx7MEf5Cn0GjUwfRTd OKLau0tl6dua5duaimUEqxFDkRJkuWXPc1v3yiTigDeXisZZhWPRv1M51ulShTut74/0 ma4/kaNbkejRaCDbNXrYqFSVIMW6S7OmDHydlpWtXpRIAnu59Prcrx0HfLn2aunMypwl V/jocBvh3oyonQOMr0+IlqC4PufPaZsPxYCPTQvKCuf8MX3m8fwh0Yhq6AU39rgKPAN0 9TRM3nrbL70rMxgLnlQHhKSO+5WtC9JO6YIi9gIZROJcL/Yc3wkGv5PENac6siscxMHp TN9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc:content-transfer-encoding; bh=HZMnRQf2RzyKu0EQDfD1pRubT2qoNtln9gX9VZl7+Lw=; b=olTrCWNrJMcFRs2iu2/MkwBblePbvO1CqZF7tKhN/0H0rt5jrtPztip8xylU9qp8D5 nJD7bWCRTrca15zDNe/iXybEQR0wY53Lf9D5QNLO6XS4sBEhZrHnCLJv2z0/0Se5f40t HIaSEszmd4hsD4Do/6IQnPRZANe0qsq0T3I0N/2TizpFBLkF3FLeNCA9wAABEHul2ekm N+yOHjXIUgGR5kvH9cYtP3++FnG6e7dkSHHheaTKHRO6kwmmxk8kLe4+XlyOimiJFykI IeThnlL7+OcLAvEd8IANpQV2FLy6kwqrfyHQVCSZECYa4JXUHtBH575ZRCo8KW2/O9aN NnUg== X-Gm-Message-State: APjAAAVRbm4kTtGZk/MXVc9+u7ZN8Ev9LflqDcZwlW82z/OBEemkalVc KAgGzzxo7yXsckfmo1QhHT2+X3c= X-Google-Smtp-Source: APXvYqzIKCMr5RBHVty3PxKkUeb+z/z35I8hFPiVh8D2zq2tXjZ8EsxBxsgexdyHh/iBIMSJ8f2vGqE= X-Received: by 2002:a81:618a:: with SMTP id v132mr9055009ywb.388.1576806560029; Thu, 19 Dec 2019 17:49:20 -0800 (PST) Date: Thu, 19 Dec 2019 17:48:53 -0800 In-Reply-To: Message-Id: <20191220014853.223389-1-pcc@google.com> Mime-Version: 1.0 References: X-Mailer: git-send-email 2.24.1.735.g03f4e72817-goog Subject: [PATCH] arm64: mte: Clear SCTLR_EL1.TCF0 on exec From: Peter Collingbourne To: Catalin Marinas , Evgenii Stepanov , Kostya Serebryany Cc: Peter Collingbourne , Linux ARM , linux-arch@vger.kernel.org, Richard Earnshaw , Szabolcs Nagy , Marc Zyngier , Kevin Brodsky , linux-mm@kvack.org, Andrey Konovalov , Vincenzo Frascino , Will Deacon Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Signed-off-by: Peter Collingbourne --- On Thu, Dec 19, 2019 at 12:32 PM Peter Collingbourne wrote= : > > On Wed, Dec 11, 2019 at 10:45 AM Catalin Marinas > wrote: > > + =C2=A0 =C2=A0 =C2=A0 if (current->thread.sctlr_tcf0 !=3D next->thread= .sctlr_tcf0) > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 update_sctlr_el1_tcf= 0(next->thread.sctlr_tcf0); > > I don't entirely understand why yet, but I've found that this check is > insufficient for ensuring consistency between SCTLR_EL1.TCF0 and > sctlr_tcf0. In my Android test environment with some processes having > sctlr_tcf0=3DSCTLR_EL1_TCF0_SYNC and others having sctlr_tcf0=3D0, I am > seeing intermittent tag failures coming from the sctlr_tcf0=3D0 > processes. With this patch: > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index ef3bfa2bf2b1..4e5d02520a51 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -663,6 +663,8 @@ static int do_sea(unsigned long addr, unsigned int > esr, struct pt_regs *regs) > =C2=A0static int do_tag_check_fault(unsigned long addr, unsigned int esr, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 struct pt_regs *regs) > =C2=A0{ > + =C2=A0 =C2=A0 =C2=A0 printk(KERN_ERR "do_tag_check_fault %lx %lx\n", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0current->thread.sctlr_t= cf0, read_sysreg(sctlr_el1)); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 do_bad_area(addr, esr, regs); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0; > =C2=A0} > > I see dmesg output like this: > > [ =C2=A0 15.249216] do_tag_check_fault 0 c60fc64791d > > showing that SCTLR_EL1.TCF0 became inconsistent with sctlr_tcf0. This > patch fixes the problem for me: > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index fba89c9f070b..fb012f0baa12 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -518,9 +518,7 @@ static void mte_thread_switch(struct task_struct *nex= t) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!system_supports_mte()) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; > > - =C2=A0 =C2=A0 =C2=A0 /* avoid expensive SCTLR_EL1 accesses if no change= */ > - =C2=A0 =C2=A0 =C2=A0 if (current->thread.sctlr_tcf0 !=3D next->thread.s= ctlr_tcf0) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 update_sctlr_el1_tcf0(= next->thread.sctlr_tcf0); > + =C2=A0 =C2=A0 =C2=A0 update_sctlr_el1_tcf0(next->thread.sctlr_tcf0); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 update_gcr_el1_excl(next->thread.gcr_excl); > =C2=A0} > =C2=A0#else > @@ -643,15 +641,8 @@ static long set_mte_ctrl(unsigned long arg) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EINVAL; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > - =C2=A0 =C2=A0 =C2=A0 /* > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* mte_thread_switch() checks current->thread= .sctlr_tcf0 as an > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* optimisation. Disable preemption so that i= t does not see > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* the variable update before the SCTLR_EL1.T= CF0 one. > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*/ > - =C2=A0 =C2=A0 =C2=A0 preempt_disable(); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 current->thread.sctlr_tcf0 =3D tcf0; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 update_sctlr_el1_tcf0(tcf0); > - =C2=A0 =C2=A0 =C2=A0 preempt_enable(); > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 current->thread.gcr_excl =3D (arg & PR_MTE_EX= CL_MASK) >> > PR_MTE_EXCL_SHIFT; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 update_gcr_el1_excl(current->thread.gcr_excl)= ; > > Since sysreg_clear_set only sets the sysreg if it ended up changing, I > wouldn't expect this to cause a significant performance hit unless > just reading SCTLR_EL1 is expensive. That being said, if the > inconsistency is indicative of a deeper problem, we should probably > address that. I tracked it down to the flush_mte_state() function setting sctlr_tcf0 but failing to update SCTLR_EL1.TCF0. With this patch I am not seeing any more inconsistencies. Peter arch/arm64/kernel/process.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index fba89c9f070b..07e8e7bd3bec 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -319,6 +319,25 @@ static void flush_tagged_addr_state(void) } =20 #ifdef CONFIG_ARM64_MTE +static void update_sctlr_el1_tcf0(u64 tcf0) +{ + /* no need for ISB since this only affects EL0, implicit with ERET */ + sysreg_clear_set(sctlr_el1, SCTLR_EL1_TCF0_MASK, tcf0); +} + +static void set_sctlr_el1_tcf0(u64 tcf0) +{ + /* + * mte_thread_switch() checks current->thread.sctlr_tcf0 as an + * optimisation. Disable preemption so that it does not see + * the variable update before the SCTLR_EL1.TCF0 one. + */ + preempt_disable(); + current->thread.sctlr_tcf0 =3D tcf0; + update_sctlr_el1_tcf0(tcf0); + preempt_enable(); +} + static void flush_mte_state(void) { if (!system_supports_mte()) @@ -327,7 +346,7 @@ static void flush_mte_state(void) /* clear any pending asynchronous tag fault */ clear_thread_flag(TIF_MTE_ASYNC_FAULT); /* disable tag checking */ - current->thread.sctlr_tcf0 =3D 0; + set_sctlr_el1_tcf0(0); } #else static void flush_mte_state(void) @@ -497,12 +516,6 @@ static void ssbs_thread_switch(struct task_struct *nex= t) } =20 #ifdef CONFIG_ARM64_MTE -static void update_sctlr_el1_tcf0(u64 tcf0) -{ - /* no need for ISB since this only affects EL0, implicit with ERET */ - sysreg_clear_set(sctlr_el1, SCTLR_EL1_TCF0_MASK, tcf0); -} - static void update_gcr_el1_excl(u64 excl) { /* @@ -643,15 +656,7 @@ static long set_mte_ctrl(unsigned long arg) return -EINVAL; } =20 - /* - * mte_thread_switch() checks current->thread.sctlr_tcf0 as an - * optimisation. Disable preemption so that it does not see - * the variable update before the SCTLR_EL1.TCF0 one. - */ - preempt_disable(); - current->thread.sctlr_tcf0 =3D tcf0; - update_sctlr_el1_tcf0(tcf0); - preempt_enable(); + set_sctlr_el1_tcf0(tcf0); =20 current->thread.gcr_excl =3D (arg & PR_MTE_EXCL_MASK) >> PR_MTE_EXCL_SHIF= T; update_gcr_el1_excl(current->thread.gcr_excl); --=20 2.24.1.735.g03f4e72817-goog