From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14718 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Khem Raj Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] correct the operand specifiers in the riscv64 CAS routines Date: Tue, 24 Sep 2019 23:43:04 -0700 Message-ID: <63beda6c-37a4-6a18-2567-14b39737e017@gmail.com> References: <20190925033015.9905-1-palmer@sifive.com> Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="------------722898F012631313D590B85B" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="54995"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 Cc: Alistair Francis To: musl@lists.openwall.com, Palmer Dabbelt Original-X-From: musl-return-14734-gllmg-musl=m.gmane.org@lists.openwall.com Wed Sep 25 08:43:23 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1iD11F-000E5T-N6 for gllmg-musl@m.gmane.org; Wed, 25 Sep 2019 08:43:21 +0200 Original-Received: (qmail 15486 invoked by uid 550); 25 Sep 2019 06:43:19 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 15465 invoked from network); 25 Sep 2019 06:43:18 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:autocrypt:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=Jlp1hZ2FQN8HBok5541/i8AFX05tRbJVDgsK8bc5oOk=; b=NpOJnL1F1wtwoYM3CBRPgWd8yxyqPPuDZYi0WzDrPdhMUj/qGWBeNdiey99ojiHCTf 8NMaosge5krgkstLinF4fU+q1IJrc0H6a+tNAqQqkI00bCgBNI5vZqJwF7M2axjusigA z2OIBAUqfjaKI88KCw/8DiKohPjX9SYvXqV+0uluZVTQl83J3cGw7tu/CATZm3gbKivD ILxjZ1HAcXHapczM5Ws/Xi6Fib3OS6S2z8WCIF+ulzrwOtYG/UhnhyKERSckfgTJK0We XvvaW2i8rEQJDyPsNDtp1fhbOu4Ef/YHl5wndv7CaVtNK9QxJwQLXz1EtUUmmh6TOqpP FBqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language; bh=Jlp1hZ2FQN8HBok5541/i8AFX05tRbJVDgsK8bc5oOk=; b=s2im2Ftd91wPSISsTKPMjh7hijLv/RUeyLSdB9KXq+GsvwFm3Rm88Lqwa7ARcos8fn AetOVDnWHOZUvUKUzfdigiOhpdx9zwLfv+qn3ETdjkPu6lLTWxF0JyrcIpE6Wk1HRBYr 5JKuK7i1v56dqd6UFlLm8GfegEA2hZEszstdoEY11PPjhZnGvvvIABFrHoeiWP3ZZRtB WFpCi/ihn1I6z1ND86dKb5guSqPunu+8eYmjZrZQ366kQxfRphKU8+3sH4JtzOXop7O8 Xa+/v8ecmzG97sI+BScJMRxlCvrei+jAJwuYAyMudGZfLMqB7JUuBMhf+l6KwN1kq2bC S0cA== X-Gm-Message-State: APjAAAWY8GMYwvh6HmzoxXmmgf5B8za2cLA2lffrwCDIeJQtzSJU9edA fcbbKO51gGYDPiIiHfeX7Lw= X-Google-Smtp-Source: APXvYqw3Y5VcknBPX2qGMT4Bed3csUDC2H7fy1ywEq+7vXu8GOQNRkgdu2wO3h5fHSXYyDY4jY4GCg== X-Received: by 2002:a62:2643:: with SMTP id m64mr8060947pfm.76.1569393785731; Tue, 24 Sep 2019 23:43:05 -0700 (PDT) Autocrypt: addr=raj.khem@gmail.com; keydata= mQGiBEqXaJERBACUvFofpD3FsxD9675wcPv+rzguIfsRWilrrpSZ61JHjLHwkUnmDLpLSdfv Zw2ZDXeaQbGU2thctxXTyYf6N1fY6P5Tww6mWKInuGU3yAv8Mg5p+Xd8itwOoVR41DOBkftV miO2G7FtXsnqonB6F43a2yvc+h9OwPRVxAFss5mSmwCglSXPWndN1Ka4M/hzt7g+FmeyEssD /1V/G8lLeAS9gQCNjS7jch+uHMFJuWgHzMXdw99e1ywlIkvXN77NPkW+FLVxKxNAHyWZZ3wO 4BQ9/GVR2y0s/rrF1lQSIcfnUmzZrh/Bh0b1wVOSLhl0Vx4MI0/MbdL3xx17JAWy+s67evuK ER4Y8ycTq3gbIGJtVrIJjWPCzWF8BACAA2u52uDmJ2pS8SIhEW0jMK/zYQ5Od9l9fM6BmS6Q jnIlzzcuMvRdZn2IrnuE/YoC8yyzBK3mN+MVc2jWN9rfSg6ml7r14Zjem9Ee0O5Ca7Jg3ZvN 6g3vSlrRJqKgsnq3vdatDF+5rd0NJ7ZKUy6x2i1Pavgtv3qaAgPGa1qQdLQdS2hlbSBSYWog PHJhai5raGVtQGdtYWlsLmNvbT6IYAQTEQIAIAUCSpdokQIbAwYLCQgHAwIEFQIIAwQWAgMB Ah4BAheAAAoJELsFM1WRnTMUhOMAoJJleT2hdT7uw9Fyn26+w+/K4i0gAJ9wRt36PqfjNe90 4evWolFf5cMmp7kEDQRKl2iREBAAjjbVQ55RAouAe0l0nPsKkWdtMuUHwBdLBkpF3rGM/f0u DlPaQzYGLjKT7xDoyRh8x2tFwWvOfiqrZFArxX8e In-Reply-To: <20190925033015.9905-1-palmer@sifive.com> Content-Language: en-US Xref: news.gmane.org gmane.linux.lib.musl.general:14718 Archived-At: This is a multi-part message in MIME format. --------------722898F012631313D590B85B Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 9/24/19 8:30 PM, Palmer Dabbelt wrote: > The operand sepcifiers in a_cas and a_casp for riscv64 were incorrect: > there's a backwards branch in the routine, so despite tmp being written > at the end of the assembly fragment it cannot be allocated in one of the > input registers because the input values may be needed for another trip > around the loop. > > For code that follows the guarnteed forward progress requirements, he > backwards branch is rarely taken: SiFive's hardware only fails a store > conditional on execptional cases (ie, instruction cache misses inside > the loop), and until recently a bug in QEMU allowed back-to-back > store conditionals to succeed. The bug has been fixed in the latest > QEMU release, but it turns out that the fix caused this latent bug in > musl to manifest. > > Full disclosure: I haven't actually even compiled musl. I just guessed > this would fix a bug introducted by the new QEMU behavior, Alistair > (CC'd) actually checked it fixes the problem. The rest is just > conjecture. thanks Palmer, I tested this fix with OpenEmbedded and it indeed fixes the problem I have tested mimimal image built with with both gcc and clang on qemu 4.1 Tested-By: Khem Raj > --- > arch/riscv64/atomic_arch.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv64/atomic_arch.h b/arch/riscv64/atomic_arch.h > index c976534284aa..41ad4d04907c 100644 > --- a/arch/riscv64/atomic_arch.h > +++ b/arch/riscv64/atomic_arch.h > @@ -14,7 +14,7 @@ static inline int a_cas(volatile int *p, int t, int s) > " sc.w.aqrl %1, %4, (%2)\n" > " bnez %1, 1b\n" > "1:" > - : "=&r"(old), "=r"(tmp) > + : "=&r"(old), "=&r"(tmp) > : "r"(p), "r"(t), "r"(s) > : "memory"); > return old; > @@ -31,7 +31,7 @@ static inline void *a_cas_p(volatile void *p, void *t, void *s) > " sc.d.aqrl %1, %4, (%2)\n" > " bnez %1, 1b\n" > "1:" > - : "=&r"(old), "=r"(tmp) > + : "=&r"(old), "=&r"(tmp) > : "r"(p), "r"(t), "r"(s) > : "memory"); > return old; --------------722898F012631313D590B85B Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit


On 9/24/19 8:30 PM, Palmer Dabbelt wrote:
The operand sepcifiers in a_cas and a_casp for riscv64 were incorrect:
there's a backwards branch in the routine, so despite tmp being written
at the end of the assembly fragment it cannot be allocated in one of the
input registers because the input values may be needed for another trip
around the loop.

For code that follows the guarnteed forward progress requirements, he
backwards branch is rarely taken: SiFive's hardware only fails a store
conditional on execptional cases (ie, instruction cache misses inside
the loop), and until recently a bug in QEMU allowed back-to-back
store conditionals to succeed.  The bug has been fixed in the latest
QEMU release, but it turns out that the fix caused this latent bug in
musl to manifest.

Full disclosure: I haven't actually even compiled musl.  I just guessed
this would fix a bug introducted by the new QEMU behavior, Alistair
(CC'd) actually checked it fixes the problem.  The rest is just
conjecture.
thanks Palmer, I tested this fix with OpenEmbedded and it indeed fixes the
problem I have tested mimimal image built with with both gcc and clang on qemu 4.1 

Tested-By: Khem Raj <raj.khem@gmail.com>

---
 arch/riscv64/atomic_arch.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv64/atomic_arch.h b/arch/riscv64/atomic_arch.h
index c976534284aa..41ad4d04907c 100644
--- a/arch/riscv64/atomic_arch.h
+++ b/arch/riscv64/atomic_arch.h
@@ -14,7 +14,7 @@ static inline int a_cas(volatile int *p, int t, int s)
 		"	sc.w.aqrl %1, %4, (%2)\n"
 		"	bnez %1, 1b\n"
 		"1:"
-		: "=&r"(old), "=r"(tmp)
+		: "=&r"(old), "=&r"(tmp)
 		: "r"(p), "r"(t), "r"(s)
 		: "memory");
 	return old;
@@ -31,7 +31,7 @@ static inline void *a_cas_p(volatile void *p, void *t, void *s)
 		"	sc.d.aqrl %1, %4, (%2)\n"
 		"	bnez %1, 1b\n"
 		"1:"
-		: "=&r"(old), "=r"(tmp)
+		: "=&r"(old), "=&r"(tmp)
 		: "r"(p), "r"(t), "r"(s)
 		: "memory");
 	return old;
--------------722898F012631313D590B85B--