Re: [Qemu-devel] [PATCH v2 10/11] tcg/mips: Make direct jump patching thread-safe

[Sergey Fedorov <serge.fdrv@gmail.com>]

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