From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46898) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZOs0e-0000Nu-2U for qemu-devel@nongnu.org; Mon, 10 Aug 2015 14:41:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZOs0a-0007FT-O8 for qemu-devel@nongnu.org; Mon, 10 Aug 2015 14:41:20 -0400 Received: from mail-wi0-f169.google.com ([209.85.212.169]:34379) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZOs0a-0007F9-FF for qemu-devel@nongnu.org; Mon, 10 Aug 2015 14:41:16 -0400 Received: by wicne3 with SMTP id ne3so147265228wic.1 for ; Mon, 10 Aug 2015 11:41:15 -0700 (PDT) References: <1439220437-23957-1-git-send-email-fred.konrad@greensocs.com> <1439220437-23957-14-git-send-email-fred.konrad@greensocs.com> <55C8D6D2.4030908@redhat.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <55C8D6D2.4030908@redhat.com> Date: Mon, 10 Aug 2015 19:41:13 +0100 Message-ID: <877fp3gf5i.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH V7 13/19] add a callback when tb_invalidate is called. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: mttcg@listserver.greensocs.com, mark.burton@greensocs.com, a.rigo@virtualopensystems.com, qemu-devel@nongnu.org, guillaume.delbergue@greensocs.com, fred.konrad@greensocs.com Paolo Bonzini writes: > On 10/08/2015 17:27, fred.konrad@greensocs.com wrote: >> From: KONRAD Frederic >> >> Instead of doing the jump cache invalidation directly in tb_invalidate delay it >> after the exit so we don't have an other CPU trying to execute the code being >> invalidated. >> >> Signed-off-by: KONRAD Frederic >> --- >> translate-all.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 59 insertions(+), 2 deletions(-) > > If you take the easy way and avoid the optimizations in patch 7, this is > not necessary: tb_find_fast and tb_add_jump are only called from within > tb_lock, so all of tb_jmp_cache/jmp_first/jmp_next are protected by tb_lock. > > Let's get everything in and then optimize; the order should be: > > - Alvise's LL/SC implementation > > - conversion of atomics to LL/SC for all front-ends > > - the main MTTCG series, reusing the locking already in-place for > user-mode emulation (with some audit...) - including dropping the cmpxchg fix and including Alvise's MTTCG aware patches that build on top of LL/SC work. > > - any further push-downs of tb_lock > > Paolo > >> diff --git a/translate-all.c b/translate-all.c >> index 954c67a..fc5162a 100644 >> --- a/translate-all.c >> +++ b/translate-all.c >> @@ -62,6 +62,7 @@ >> #include "translate-all.h" >> #include "qemu/bitmap.h" >> #include "qemu/timer.h" >> +#include "sysemu/cpus.h" >> >> //#define DEBUG_TB_INVALIDATE >> //#define DEBUG_FLUSH >> @@ -967,14 +968,58 @@ static inline void tb_reset_jump(TranslationBlock *tb, int n) >> tb_set_jmp_target(tb, n, (uintptr_t)(tb->tc_ptr + tb->tb_next_offset[n])); >> } >> >> +struct CPUDiscardTBParams { >> + CPUState *cpu; >> + TranslationBlock *tb; >> +}; >> + >> +static void cpu_discard_tb_from_jmp_cache(void *opaque) >> +{ >> + unsigned int h; >> + struct CPUDiscardTBParams *params = opaque; >> + >> + h = tb_jmp_cache_hash_func(params->tb->pc); >> + if (params->cpu->tb_jmp_cache[h] == params->tb) { >> + params->cpu->tb_jmp_cache[h] = NULL; >> + } >> + >> + g_free(opaque); >> +} >> + >> +static void tb_invalidate_jmp_remove(void *opaque) >> +{ >> + TranslationBlock *tb = opaque; >> + TranslationBlock *tb1, *tb2; >> + unsigned int n1; >> + >> + /* suppress this TB from the two jump lists */ >> + tb_jmp_remove(tb, 0); >> + tb_jmp_remove(tb, 1); >> + >> + /* suppress any remaining jumps to this TB */ >> + tb1 = tb->jmp_first; >> + for (;;) { >> + n1 = (uintptr_t)tb1 & 3; >> + if (n1 == 2) { >> + break; >> + } >> + tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3); >> + tb2 = tb1->jmp_next[n1]; >> + tb_reset_jump(tb1, n1); >> + tb1->jmp_next[n1] = NULL; >> + tb1 = tb2; >> + } >> + tb->jmp_first = (TranslationBlock *)((uintptr_t)tb | 2); /* fail safe */ >> +} >> + >> /* invalidate one TB */ >> void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) >> { >> CPUState *cpu; >> PageDesc *p; >> - unsigned int h, n1; >> + unsigned int h; >> tb_page_addr_t phys_pc; >> - TranslationBlock *tb1, *tb2; >> + struct CPUDiscardTBParams *params; >> >> tb_lock(); >> >> @@ -997,6 +1042,9 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) >> >> tcg_ctx.tb_ctx.tb_invalidated_flag = 1; >> >> +#if 0 /*MTTCG*/ >> + TranslationBlock *tb1, *tb2; >> + unsigned int n1; >> /* remove the TB from the hash list */ >> h = tb_jmp_cache_hash_func(tb->pc); >> CPU_FOREACH(cpu) { >> @@ -1023,6 +1071,15 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) >> tb1 = tb2; >> } >> tb->jmp_first = (TranslationBlock *)((uintptr_t)tb | 2); /* fail safe */ >> +#else >> + CPU_FOREACH(cpu) { >> + params = g_malloc(sizeof(struct CPUDiscardTBParams)); >> + params->cpu = cpu; >> + params->tb = tb; >> + async_run_on_cpu(cpu, cpu_discard_tb_from_jmp_cache, params); >> + } >> + async_run_safe_work_on_cpu(first_cpu, tb_invalidate_jmp_remove, tb); >> +#endif /* MTTCG */ >> >> tcg_ctx.tb_ctx.tb_phys_invalidate_count++; >> tb_unlock(); >> -- Alex Bennée