From patchwork Fri Sep 30 17:40:57 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bdegraaf@codeaurora.org X-Patchwork-Id: 9358669 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 01501600C8 for ; Fri, 30 Sep 2016 17:43:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EB3FA2A10E for ; Fri, 30 Sep 2016 17:43:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DD8A72A10F; Fri, 30 Sep 2016 17:43:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 454EC2A10C for ; Fri, 30 Sep 2016 17:43:38 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bq1p9-0005XC-3u; Fri, 30 Sep 2016 17:42:15 +0000 Received: from smtp.codeaurora.org ([198.145.29.96]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bq1p5-0005SA-1F for linux-arm-kernel@lists.infradead.org; Fri, 30 Sep 2016 17:42:12 +0000 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 7AF976185F; Fri, 30 Sep 2016 17:41:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1475257308; bh=6CVvDiadgLUTiz+Jl0nJNWuhSf4vgapugz7wgtYL6Qs=; h=From:To:Cc:Subject:Date:From; b=aVhbZofUZPdTTp/8TTt6OAJJnrctnvT1E5MaZaaacXyFV4EofhePjxI2Dshf052DU 5Kfzvfz3H3kz/AXNRfqqvZJ4uul+VKsLVOZ9Vd/PmjrrDmGJqo13XxZY4JVwywWG4W 7Zeq20uNDFr0PPc0+gJfeiOJ7EV9gMAc/bL5vZLE= Received: from southpoint.qualcomm.com.qualcomm.com (global_nat1_iad_fw.qualcomm.com [129.46.232.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: bdegraaf@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 497D161816; Fri, 30 Sep 2016 17:41:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1475257307; bh=6CVvDiadgLUTiz+Jl0nJNWuhSf4vgapugz7wgtYL6Qs=; h=From:To:Cc:Subject:Date:From; b=IyxlFodgDvY2RrGctKPFjOVzRbuNdaJ9CTDw5qBVYTqmhrKX/qIuDavEIVX1Om8OZ YOM1gaA1Jy3EOOwRb+dH6oHb2sQE3BpECOIbydtZ3bXvVCA6zkDQVAh2+m+YoBGk/O /897ZQcr5lFTHbYWaYThXIrIbQ0QPM7VWT4kmkW8= DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org 497D161816 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=bdegraaf@codeaurora.org From: Brent DeGraaf To: Peter Zijlstra , Ingo Molnar , Catalin Marinas , Will Deacon Subject: [RFC] arm64: Enforce observed order for spinlock and data Date: Fri, 30 Sep 2016 13:40:57 -0400 Message-Id: <1475257257-23072-1-git-send-email-bdegraaf@codeaurora.org> X-Mailer: git-send-email 1.9.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160930_104211_175322_79B994BA X-CRM114-Status: GOOD ( 17.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Brent DeGraaf , Timur Tabi , Nathan Lynch , linux-kernel@vger.kernel.org, Christopher Covington , linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Prior spinlock code solely used load-acquire and store-release semantics to ensure ordering of the spinlock lock and the area it protects. However, store-release semantics and ordinary stores do not protect against accesses to the protected area being observed prior to the access that locks the lock itself. While the load-acquire and store-release ordering is sufficient when the spinlock routines themselves are strictly used, other kernel code that references the lock values directly (e.g. lockrefs) could observe changes to the area protected by the spinlock prior to observance of the lock itself being in a locked state, despite the fact that the spinlock logic itself is correct. Barriers were added to all the locking routines wherever necessary to ensure that outside observers which read the lock values directly will not observe changes to the protected data before the lock itself is observed. Signed-off-by: Brent DeGraaf --- arch/arm64/include/asm/spinlock.h | 59 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/spinlock.h b/arch/arm64/include/asm/spinlock.h index 89206b5..4dd0977 100644 --- a/arch/arm64/include/asm/spinlock.h +++ b/arch/arm64/include/asm/spinlock.h @@ -106,7 +106,20 @@ static inline void arch_spin_lock(arch_spinlock_t *lock) /* Did we get the lock? */ " eor %w1, %w0, %w0, ror #16\n" -" cbz %w1, 3f\n" +" cbnz %w1, 4f\n" + /* + * Yes: The store done on this cpu was the one that locked the lock. + * Store-release one-way barrier on LL/SC means that accesses coming + * after this could be reordered into the critical section of the + * load-acquire/store-release, where we did not own the lock. On LSE, + * even the one-way barrier of the store-release semantics is missing, + * so LSE needs an explicit barrier here as well. Without this, the + * changed contents of the area protected by the spinlock could be + * observed prior to the lock. + */ +" dmb ish\n" +" b 3f\n" +"4:\n" /* * No: spin on the owner. Send a local event to avoid missing an * unlock before the exclusive load. @@ -116,7 +129,15 @@ static inline void arch_spin_lock(arch_spinlock_t *lock) " ldaxrh %w2, %4\n" " eor %w1, %w2, %w0, lsr #16\n" " cbnz %w1, 2b\n" - /* We got the lock. Critical section starts here. */ + /* + * We got the lock and have observed the prior owner's store-release. + * In this case, the one-way barrier of the prior owner that we + * observed combined with the one-way barrier of our load-acquire is + * enough to ensure accesses to the protected area coming after this + * are not accessed until we own the lock. In this case, other + * observers will not see our changes prior to observing the lock + * itself. Critical locked section starts here. + */ "3:" : "=&r" (lockval), "=&r" (newval), "=&r" (tmp), "+Q" (*lock) : "Q" (lock->owner), "I" (1 << TICKET_SHIFT) @@ -137,6 +158,13 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock) " add %w0, %w0, %3\n" " stxr %w1, %w0, %2\n" " cbnz %w1, 1b\n" + /* + * We got the lock with a successful store-release: Store-release + * one-way barrier means accesses coming after this could be observed + * before the lock is observed as locked. + */ + " dmb ish\n" + " nop\n" "2:", /* LSE atomics */ " ldr %w0, %2\n" @@ -146,6 +174,13 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock) " casa %w0, %w1, %2\n" " and %w1, %w1, #0xffff\n" " eor %w1, %w1, %w0, lsr #16\n" + " cbnz %w1, 1f\n" + /* + * We got the lock with the LSE casa store. + * A barrier is required to ensure accesses coming from the + * critical section of the lock are not observed before our lock. + */ + " dmb ish\n" "1:") : "=&r" (lockval), "=&r" (tmp), "+Q" (*lock) : "I" (1 << TICKET_SHIFT) @@ -212,6 +247,12 @@ static inline void arch_write_lock(arch_rwlock_t *rw) " cbnz %w0, 1b\n" " stxr %w0, %w2, %1\n" " cbnz %w0, 2b\n" + /* + * Lock is not ours until the store, which has no implicit barrier. + * Barrier is needed so our writes to the protected area are not + * observed before our lock ownership is observed. + */ + " dmb ish\n" " nop", /* LSE atomics */ "1: mov %w0, wzr\n" @@ -221,7 +262,12 @@ static inline void arch_write_lock(arch_rwlock_t *rw) " cbz %w0, 2b\n" " wfe\n" " b 1b\n" - "3:") + /* + * Casa doesn't use store-release semantics. Even if it did, + * it would not protect us from our writes being observed before + * our ownership is observed. Barrier is required. + */ + "3: dmb ish") : "=&r" (tmp), "+Q" (rw->lock) : "r" (0x80000000) : "memory"); @@ -299,7 +345,12 @@ static inline void arch_read_lock(arch_rwlock_t *rw) " tbnz %w1, #31, 1b\n" " casa %w0, %w1, %2\n" " sbc %w0, %w1, %w0\n" - " cbnz %w0, 2b") + " cbnz %w0, 2b\n" + /* + * Need to ensure that our reads of the area protected by the lock + * are not observed before our lock ownership is observed. + */ + " dmb ish\n") : "=&r" (tmp), "=&r" (tmp2), "+Q" (rw->lock) : : "cc", "memory");