Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-04-14 Thread Sergey Fedorov
On 14/04/16 18:13, Paolo Bonzini wrote: > On 14/04/2016 16:45, Sergey Fedorov wrote: >> Personally, I'm not so >> happy trying to use pc/cs_base/flags to mark an invalid TB. Are my >> worries unreasonable? :) > Can you explain your worries? > > The advantages are that it's O(1) and it obviously doe

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-04-14 Thread Sergey Fedorov
On 14/04/16 18:13, Paolo Bonzini wrote: > This is very similar to the current code. From 10,000 feet, because > tb_find_fast calls tb_find_slow, this could indeed work, but I'm a bit > concerned about how to order the removal of the jump lists. As long as we always link/unlink TBs under tb_lock t

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-04-14 Thread Paolo Bonzini
On 14/04/2016 17:36, Sergey Fedorov wrote: > IIUC we always modify tb_jmp_cache/tb_phys_hash under tb_lock. We're > just gonna do tb_jmp_cache lookup outside of tb_lock. I think write > memory barrier in tb_phys_invalidate() paired with read memory memory > barrier in tb_find_fast()/tb_find_slow(

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-04-14 Thread Sergey Fedorov
On 14/04/16 18:13, Paolo Bonzini wrote: > This is very similar to the current code. From 10,000 feet, because > tb_find_fast calls tb_find_slow, this could indeed work, but I'm a bit > concerned about how to order the removal of the jump lists. The usage > of "tcg_ctx.tb_ctx.tb_invalidated_flag =

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-04-14 Thread Paolo Bonzini
On 14/04/2016 16:45, Sergey Fedorov wrote: > So what would you suggest to use for x86? I can't think of something > that looks like a really compelling combination when I look at > cpu_get_tb_cpu_state() in target-i386/cpu.h. On x86 I think we should define HF_INVALID_TB to an invalid flag combi

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-04-14 Thread Sergey Fedorov
On 29/03/16 13:37, Paolo Bonzini wrote: Your observation that tb->pc==-1 is not necessarily safe still holds of >> > course. Probably the best thing is an inline that can do one of: >> > >> > 1) set cs_base to an invalid value (anything nonzero is enough except >> > on >>

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-29 Thread Alex Bennée
Sergey Fedorov writes: > On 29/03/16 13:37, Paolo Bonzini wrote: >> cpu_exec_nocache is not used in user-mode emulation, so it's okay if >> qemu.git doesn't take the lock yet. (This kind of misunderstanding >> about which code is thread-safe is going to be common until we have >> MTTCG. This w

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-29 Thread Sergey Fedorov
On 29/03/16 13:37, Paolo Bonzini wrote: > cpu_exec_nocache is not used in user-mode emulation, so it's okay if > qemu.git doesn't take the lock yet. (This kind of misunderstanding > about which code is thread-safe is going to be common until we have > MTTCG. This was the reason for the patch "cpu

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-29 Thread Paolo Bonzini
On 29/03/2016 12:03, Sergey Fedorov wrote: >>> >> [...] I would suggest the following solution: >>> >> (1) Use 'tb->pc' as an indicator of whether TB is valid; check for it >>> >> in cpu_exec() when deciding on whether to patch the last executed >>> >> TB or not >>> >> (2) Use 'tcg_ct

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-29 Thread Sergey Fedorov
On 29/03/16 00:21, Paolo Bonzini wrote: > > On 28/03/2016 17:18, Sergey Fedorov wrote: >> The use pattern of 'tb_invalidated_flag' is a bit intricate; correct me, >> if I'm wrong about the following. Basically, 'tb_invalidated_flag' was >> meant to catch two events: >> * some TB has been invalidat

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-28 Thread Richard Henderson
On 03/28/2016 01:58 PM, Paolo Bonzini wrote: On 28/03/2016 20:42, Sergey Fedorov wrote: On 17/03/16 16:46, sergey.fedo...@linaro.org wrote: First the translation block is invalidated, for which a simple write to tb->pc is enough. This means that cpu-exec will not pick up anymore the block, t

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-28 Thread Paolo Bonzini
On 28/03/2016 17:18, Sergey Fedorov wrote: > The use pattern of 'tb_invalidated_flag' is a bit intricate; correct me, > if I'm wrong about the following. Basically, 'tb_invalidated_flag' was > meant to catch two events: > * some TB has been invalidated by tb_phys_invalidate(); This is patch 4.

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-28 Thread Paolo Bonzini
On 28/03/2016 20:42, Sergey Fedorov wrote: > On 17/03/16 16:46, sergey.fedo...@linaro.org wrote: >> First the translation block is invalidated, for which a simple write >> to tb->pc is enough. This means that cpu-exec will not pick up anymore >> the block, though it may still execute it through

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-28 Thread Sergey Fedorov
On 17/03/16 16:46, sergey.fedo...@linaro.org wrote: > First the translation block is invalidated, for which a simple write > to tb->pc is enough. This means that cpu-exec will not pick up anymore > the block, though it may still execute it through chained jumps. This > also replaces the NULLing o

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-28 Thread Sergey Fedorov
On 17/03/16 18:14, Sergey Fedorov wrote: > On 17/03/16 18:09, Paolo Bonzini wrote: >> >> On 17/03/2016 14:46, sergey.fedo...@linaro.org wrote: >>> void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t >>> page_addr) >>> { >>> -CPUState *cpu; >>> PageDesc *p; >>> unsigned

[Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-19 Thread sergey . fedorov
From: Paolo Bonzini First the translation block is invalidated, for which a simple write to tb->pc is enough. This means that cpu-exec will not pick up anymore the block, though it may still execute it through chained jumps. This also replaces the NULLing out of the pointer in the CPUs' local c

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-19 Thread Paolo Bonzini
On 17/03/2016 14:46, sergey.fedo...@linaro.org wrote: > void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) > { > -CPUState *cpu; > PageDesc *p; > unsigned int h, n1; > +tb_page_addr_t pc; > tb_page_addr_t phys_pc; > TranslationBlock *tb1, *tb2; >

Re: [Qemu-devel] [PATCH 4/5] tcg: reorder removal from lists in tb_phys_invalidate

2016-03-18 Thread Sergey Fedorov
On 17/03/16 18:09, Paolo Bonzini wrote: On 17/03/2016 14:46, sergey.fedo...@linaro.org wrote: void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) { -CPUState *cpu; PageDesc *p; unsigned int h, n1; +tb_page_addr_t pc; tb_page_addr_t phys_pc;