From d1142fb83efdcf8a6c2dee825569892203e16d2c Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Sun, 23 Aug 2015 20:23:39 -0400 Subject: translate-all: remove obsolete comment about l1_map l1_map is based on physical addresses in full-system mode, as pointed out in an earlier comment. Said comment also mentions that virtual addresses are only used in l1_map in user-only mode. Signed-off-by: Emilio G. Cota Message-Id: <1440375847-17603-11-git-send-email-cota@braap.org> Signed-off-by: Paolo Bonzini --- translate-all.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'translate-all.c') diff --git a/translate-all.c b/translate-all.c index 2a40530bba..a75aeed538 100644 --- a/translate-all.c +++ b/translate-all.c @@ -122,8 +122,7 @@ uintptr_t qemu_real_host_page_mask; uintptr_t qemu_host_page_size; uintptr_t qemu_host_page_mask; -/* This is a multi-level map on the virtual address space. - The bottom level has pointers to PageDesc. */ +/* The bottom level has pointers to PageDesc */ static void *l1_map[V_L1_SIZE]; /* code generation context */ -- cgit v1.2.1 From 677ef6230b603571ae05125db469f7b4c8912a77 Mon Sep 17 00:00:00 2001 From: KONRAD Frederic Date: Mon, 10 Aug 2015 17:27:02 +0200 Subject: replace spinlock by QemuMutex. spinlock is only used in two cases: * cpu-exec.c: to protect TranslationBlock * mem_helper.c: for lock helper in target-i386 (which seems broken). It's a pthread_mutex_t in user-mode, so we can use QemuMutex directly, with an #ifdef. The #ifdef will be removed when multithreaded TCG will need the mutex as well. Signed-off-by: KONRAD Frederic Message-Id: <1439220437-23957-5-git-send-email-fred.konrad@greensocs.com> Signed-off-by: Emilio G. Cota [Merge Emilio G. Cota's patch to remove volatile. - Paolo] Signed-off-by: Paolo Bonzini --- translate-all.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'translate-all.c') diff --git a/translate-all.c b/translate-all.c index a75aeed538..37bb56ca42 100644 --- a/translate-all.c +++ b/translate-all.c @@ -128,6 +128,39 @@ static void *l1_map[V_L1_SIZE]; /* code generation context */ TCGContext tcg_ctx; +/* translation block context */ +#ifdef CONFIG_USER_ONLY +__thread int have_tb_lock; +#endif + +void tb_lock(void) +{ +#ifdef CONFIG_USER_ONLY + assert(!have_tb_lock); + qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock); + have_tb_lock++; +#endif +} + +void tb_unlock(void) +{ +#ifdef CONFIG_USER_ONLY + assert(have_tb_lock); + have_tb_lock--; + qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock); +#endif +} + +void tb_lock_reset(void) +{ +#ifdef CONFIG_USER_ONLY + if (have_tb_lock) { + qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock); + have_tb_lock = 0; + } +#endif +} + static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, tb_page_addr_t phys_page2); static TranslationBlock *tb_find_pc(uintptr_t tc_ptr); @@ -675,6 +708,7 @@ static inline void code_gen_alloc(size_t tb_size) CODE_GEN_AVG_BLOCK_SIZE; tcg_ctx.tb_ctx.tbs = g_malloc(tcg_ctx.code_gen_max_blocks * sizeof(TranslationBlock)); + qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock); } /* Must be called before using the QEMU cpus. 'tb_size' is the size -- cgit v1.2.1 From 6940fab84b826175cf90d48d0e3da1b76518f5b4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 12 Aug 2015 09:41:40 +0200 Subject: tcg: add memory barriers in page_find_alloc accesses page_find is reading the radix tree outside all locks, so it has to use the RCU primitives. It does not need RCU critical sections because the PageDescs are never removed, so there is never a need to wait for the end of code sections that use a PageDesc. Signed-off-by: Paolo Bonzini --- translate-all.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'translate-all.c') diff --git a/translate-all.c b/translate-all.c index 37bb56ca42..5329982518 100644 --- a/translate-all.c +++ b/translate-all.c @@ -431,26 +431,26 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc) /* Level 2..N-1. */ for (i = V_L1_SHIFT / V_L2_BITS - 1; i > 0; i--) { - void **p = *lp; + void **p = atomic_rcu_read(lp); if (p == NULL) { if (!alloc) { return NULL; } p = g_new0(void *, V_L2_SIZE); - *lp = p; + atomic_rcu_set(lp, p); } lp = p + ((index >> (i * V_L2_BITS)) & (V_L2_SIZE - 1)); } - pd = *lp; + pd = atomic_rcu_read(lp); if (pd == NULL) { if (!alloc) { return NULL; } pd = g_new0(PageDesc, V_L2_SIZE); - *lp = pd; + atomic_rcu_set(lp, pd); } return pd + (index & (V_L2_SIZE - 1)); -- cgit v1.2.1 From 756920876f60829fad0d15df4f3fa205077a8131 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Aug 2015 10:59:50 +0200 Subject: tcg: comment on which functions have to be called with mmap_lock held Reviewed-by: Peter Maydell Signed-off-by: Paolo Bonzini --- translate-all.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) (limited to 'translate-all.c') diff --git a/translate-all.c b/translate-all.c index 5329982518..a12139ba01 100644 --- a/translate-all.c +++ b/translate-all.c @@ -171,11 +171,13 @@ void cpu_gen_init(void) } /* return non zero if the very first instruction is invalid so that - the virtual CPU can trigger an exception. - - '*gen_code_size_ptr' contains the size of the generated code (host - code). -*/ + * the virtual CPU can trigger an exception. + * + * '*gen_code_size_ptr' contains the size of the generated code (host + * code). + * + * Called with mmap_lock held for user-mode emulation. + */ int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, int *gen_code_size_ptr) { TCGContext *s = &tcg_ctx; @@ -420,6 +422,9 @@ static void page_init(void) #endif } +/* If alloc=1: + * Called with mmap_lock held for user-mode emulation. + */ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc) { PageDesc *pd; @@ -1029,6 +1034,7 @@ static void build_page_bitmap(PageDesc *p) } } +/* Called with mmap_lock held for user mode emulation. */ TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc, target_ulong cs_base, int flags, int cflags) @@ -1076,6 +1082,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu, * 'is_cpu_write_access' should be true if called from a real cpu write * access: the virtual CPU will exit the current TB if code is modified inside * this TB. + * + * Called with mmap_lock held for user-mode emulation */ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end) { @@ -1092,6 +1100,8 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end) * 'is_cpu_write_access' should be true if called from a real cpu write * access: the virtual CPU will exit the current TB if code is modified inside * this TB. + * + * Called with mmap_lock held for user-mode emulation */ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end, int is_cpu_write_access) @@ -1240,6 +1250,7 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len) } #if !defined(CONFIG_SOFTMMU) +/* Called with mmap_lock held. */ static void tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc, void *puc, bool locked) @@ -1309,7 +1320,10 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr, } #endif -/* add the tb in the target page and protect it if necessary */ +/* add the tb in the target page and protect it if necessary + * + * Called with mmap_lock held for user-mode emulation. + */ static inline void tb_alloc_page(TranslationBlock *tb, unsigned int n, tb_page_addr_t page_addr) { @@ -1365,7 +1379,8 @@ static inline void tb_alloc_page(TranslationBlock *tb, } /* add a new TB and link it to the physical page tables. phys_page2 is - (-1) to indicate that only one page contains the TB. */ + * (-1) to indicate that only one page contains the TB. + */ static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, tb_page_addr_t phys_page2) { -- cgit v1.2.1 From 8fd19e6cfd5b6cdf028c6ac2ff4157ed831ea3a6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Aug 2015 10:57:52 +0200 Subject: exec: make mmap_lock/mmap_unlock globally available There is some iffy lock hierarchy going on in translate-all.c. To fix it, we need to take the mmap_lock in cpu-exec.c. Make the functions globally available. Reviewed-by: Peter Maydell Signed-off-by: Paolo Bonzini --- translate-all.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'translate-all.c') diff --git a/translate-all.c b/translate-all.c index a12139ba01..e3c5c5e20a 100644 --- a/translate-all.c +++ b/translate-all.c @@ -466,11 +466,6 @@ static inline PageDesc *page_find(tb_page_addr_t index) return page_find_alloc(index, 0); } -#if !defined(CONFIG_USER_ONLY) -#define mmap_lock() do { } while (0) -#define mmap_unlock() do { } while (0) -#endif - #if defined(CONFIG_USER_ONLY) /* Currently it is not recommended to allocate big chunks of data in user mode. It will change when a dedicated libc will be used. */ -- cgit v1.2.1 From 9fd1a94888cd6a559f95c3596ec1ac28b74838c1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Aug 2015 11:33:24 +0200 Subject: cpu-exec: fix lock hierarchy for user-mode emulation tb_lock has to be taken inside the mmap_lock (example: tb_invalidate_phys_range is called by target_mmap), but tb_link_page is taking the mmap_lock and it is called with the tb_lock held. To fix this, take the mmap_lock in tb_find_slow, not in tb_link_page. Reviewed-by: Peter Maydell Signed-off-by: Paolo Bonzini --- translate-all.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'translate-all.c') diff --git a/translate-all.c b/translate-all.c index e3c5c5e20a..d9c7aac8e6 100644 --- a/translate-all.c +++ b/translate-all.c @@ -1375,6 +1375,8 @@ static inline void tb_alloc_page(TranslationBlock *tb, /* add a new TB and link it to the physical page tables. phys_page2 is * (-1) to indicate that only one page contains the TB. + * + * Called with mmap_lock held for user-mode emulation. */ static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, tb_page_addr_t phys_page2) @@ -1382,9 +1384,6 @@ static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, unsigned int h; TranslationBlock **ptb; - /* Grab the mmap lock to stop another thread invalidating this TB - before we are done. */ - mmap_lock(); /* add in the physical hash table */ h = tb_phys_hash_func(phys_pc); ptb = &tcg_ctx.tb_ctx.tb_phys_hash[h]; @@ -1414,7 +1413,6 @@ static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, #ifdef DEBUG_TB_CHECK tb_page_check(); #endif - mmap_unlock(); } /* find the TB 'tb' such that tb[0].tc_ptr <= tc_ptr < -- cgit v1.2.1