From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55414)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <serge.fdrv@gmail.com>) id 1ateNP-0006PX-8B
	for qemu-devel@nongnu.org; Fri, 22 Apr 2016 12:56:20 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <serge.fdrv@gmail.com>) id 1ateNM-0002hT-1s
	for qemu-devel@nongnu.org; Fri, 22 Apr 2016 12:56:19 -0400
Received: from mail-lf0-x242.google.com ([2a00:1450:4010:c07::242]:34025)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <serge.fdrv@gmail.com>) id 1ateNL-0002hP-N9
	for qemu-devel@nongnu.org; Fri, 22 Apr 2016 12:56:15 -0400
Received: by mail-lf0-x242.google.com with SMTP id e190so10541081lfe.1
	for <qemu-devel@nongnu.org>; Fri, 22 Apr 2016 09:56:15 -0700 (PDT)
References: <1461341333-19646-1-git-send-email-sergey.fedorov@linaro.org>
	<1461341333-19646-11-git-send-email-sergey.fedorov@linaro.org>
	<20160422164743.GA23711@aurel32.net>
From: Sergey Fedorov <serge.fdrv@gmail.com>
Message-ID: <571A57AC.3070303@gmail.com>
Date: Fri, 22 Apr 2016 19:56:12 +0300
MIME-Version: 1.0
In-Reply-To: <20160422164743.GA23711@aurel32.net>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 10/11] tcg/mips: Make direct jump
 patching thread-safe
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Aurelien Jarno <aurelien@aurel32.net>, Sergey Fedorov <sergey.fedorov@linaro.org>
Cc: qemu-devel@nongnu.org, =?UTF-8?Q?Alex_Benn=c3=a9e?= <alex.bennee@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Crosthwaite <crosthwaite.peter@gmail.com>, Richard Henderson <rth@twiddle.net>

On 22/04/16 19:47, Aurelien Jarno wrote:
> On 2016-04-22 19:08, Sergey Fedorov wrote:
>> From: Sergey Fedorov <serge.fdrv@gmail.com>
>>
>> Ensure direct jump patching in MIPS is atomic by using
>> atomic_read()/atomic_set() for code patching.
>>
>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>> ---
>>
>> Changes in v2:
>>  * s/atomic_write/atomic_set/
>>
>>  tcg/mips/tcg-target.inc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
>> index 682e19897db0..cefc0398018a 100644
>> --- a/tcg/mips/tcg-target.inc.c
>> +++ b/tcg/mips/tcg-target.inc.c
>> @@ -1886,6 +1886,7 @@ static void tcg_target_init(TCGContext *s)
>>  void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
>>  {
>>      uint32_t *ptr = (uint32_t *)jmp_addr;
>> -    *ptr = deposit32(*ptr, 0, 26, addr >> 2);
>> +    uint32_t insn = atomic_read(ptr);
>> +    atomic_set(ptr, deposit32(insn, 0, 26, addr >> 2));
>>      flush_icache_range(jmp_addr, jmp_addr + 4);
> Does it really make sense to read and write the value atomically? The
> resulting operation is still not atomic, something can happen in
> between.

Actually, it's not important to read it atomically because it's always
the target address part of the instruction gets only changed. So
whatever can happen in between is overwritten by subsequent deposit32().

Kind regards,
Sergey