[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set
From: |
Sergey Fedorov |
Subject: |
Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set |
Date: |
Mon, 18 Jul 2016 20:25:58 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 |
On 18/07/16 20:22, Paolo Bonzini wrote:
>
> On 18/07/2016 19:17, Sergey Fedorov wrote:
>> On 18/07/16 20:11, Paolo Bonzini wrote:
>>> On 18/07/2016 19:07, Sergey Fedorov wrote:
>>>> On 18/07/16 20:00, Paolo Bonzini wrote:
>>>>> On 18/07/2016 18:57, Sergey Fedorov wrote:
>>>>>> On 18/07/16 19:53, Paolo Bonzini wrote:
>>>>>>> On 18/07/2016 18:52, Sergey Fedorov wrote:
>>>>>>>> So how are we going to use them?
>>>>>>> Instead of atomic_read/atomic_set when marking invalid TBs.
>>>>>> But shouldn't they be atomic to avoid reading torn writes?
>>>>> A torn write would probably fail to match anyway, but even if it doesn't
>>>>> it is indistinguishable from a race, isn't it?
>>>> I'm afraid, torn write can happen to be a false match against a wrong
>>>> TB. In case of a race with atomic access we either get the right TB or
>>>> an invalid one which couldn't match any valid CPU state. Probably, we
>>>> have to make sure (and document this) that TB invalidation process
>>>> cannot make a partially invalidated TB which can match any meaningful
>>>> CPU state.
>>> x86 is atomic (because flags are 32-bit); those that have cs_base==0 are
>>> safe against torn writes too. Only SPARC perhaps could use
>>> "tb->cs_base|=1" instead in case 0xffffffff........ matches another TB.
>> That could really work but needs some comment, of course.
> Yup. A simpler possibility is this:
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index e327a35..3278d8a 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -753,14 +753,14 @@ static inline void
> cpu_get_invalid_tb_cpu_state(target_ulong *pc,
> target_ulong *cs_base,
> uint32_t *flags)
> {
> - *cs_base = -1; /* npc must be a multible of 4 */
> + *flags = TB_FLAG_MMU_MASK;
> }
Hmm, not sure if it is really simpler to follow. Maybe " |= 1;" anyway?
>
> static inline bool cpu_tb_cpu_state_is_invalidated(target_ulong pc,
> target_ulong cs_base,
> uint32_t flags)
> {
> - return cs_base == -1;
> + return flags == TB_FLAG_MMU_MASK;
> }
>
> static inline bool tb_fpu_enabled(int tb_flags)
>
>
> I'll send a fixup patch now, ack it and I'll send another pull request. :)
>
>> BTW, what is
>> the main point of such change? A bit more performance on some 32-bit hosts?
> No, succeeding to compile on 32-bit hosts. :)
- [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Sergey Fedorov, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Sergey Fedorov, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Sergey Fedorov, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Sergey Fedorov, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set,
Sergey Fedorov <=
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Sergey Fedorov, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Sergey Fedorov, 2016/07/18
- Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set, Paolo Bonzini, 2016/07/18