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 <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2015-08-11 11:33:24 +02:00
parent 8fd19e6cfd
commit 9fd1a94888
2 changed files with 55 additions and 22 deletions

View File

@ -249,10 +249,10 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
tb_free(tb); tb_free(tb);
} }
static TranslationBlock *tb_find_slow(CPUState *cpu, static TranslationBlock *tb_find_physical(CPUState *cpu,
target_ulong pc, target_ulong pc,
target_ulong cs_base, target_ulong cs_base,
uint64_t flags) uint64_t flags)
{ {
CPUArchState *env = (CPUArchState *)cpu->env_ptr; CPUArchState *env = (CPUArchState *)cpu->env_ptr;
TranslationBlock *tb, **ptb1; TranslationBlock *tb, **ptb1;
@ -269,8 +269,9 @@ static TranslationBlock *tb_find_slow(CPUState *cpu,
ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h]; ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h];
for(;;) { for(;;) {
tb = *ptb1; tb = *ptb1;
if (!tb) if (!tb) {
goto not_found; return NULL;
}
if (tb->pc == pc && if (tb->pc == pc &&
tb->page_addr[0] == phys_page1 && tb->page_addr[0] == phys_page1 &&
tb->cs_base == cs_base && tb->cs_base == cs_base &&
@ -282,25 +283,59 @@ static TranslationBlock *tb_find_slow(CPUState *cpu,
virt_page2 = (pc & TARGET_PAGE_MASK) + virt_page2 = (pc & TARGET_PAGE_MASK) +
TARGET_PAGE_SIZE; TARGET_PAGE_SIZE;
phys_page2 = get_page_addr_code(env, virt_page2); phys_page2 = get_page_addr_code(env, virt_page2);
if (tb->page_addr[1] == phys_page2) if (tb->page_addr[1] == phys_page2) {
goto found; break;
}
} else { } else {
goto found; break;
} }
} }
ptb1 = &tb->phys_hash_next; ptb1 = &tb->phys_hash_next;
} }
not_found:
/* if no translated code available, then translate it now */ /* Move the TB to the head of the list */
*ptb1 = tb->phys_hash_next;
tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
return tb;
}
static TranslationBlock *tb_find_slow(CPUState *cpu,
target_ulong pc,
target_ulong cs_base,
uint64_t flags)
{
TranslationBlock *tb;
tb = tb_find_physical(cpu, pc, cs_base, flags);
if (tb) {
goto found;
}
#ifdef CONFIG_USER_ONLY
/* mmap_lock is needed by tb_gen_code, and mmap_lock must be
* taken outside tb_lock. Since we're momentarily dropping
* tb_lock, there's a chance that our desired tb has been
* translated.
*/
tb_unlock();
mmap_lock();
tb_lock();
tb = tb_find_physical(cpu, pc, cs_base, flags);
if (tb) {
mmap_unlock();
goto found;
}
#endif
/* if no translated code available, then translate it now */
tb = tb_gen_code(cpu, pc, cs_base, flags, 0); tb = tb_gen_code(cpu, pc, cs_base, flags, 0);
found: #ifdef CONFIG_USER_ONLY
/* Move the last found TB to the head of the list */ mmap_unlock();
if (likely(*ptb1)) { #endif
*ptb1 = tb->phys_hash_next;
tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h]; found:
tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
}
/* we add the TB in the virtual pc hash table */ /* we add the TB in the virtual pc hash table */
cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)] = tb; cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)] = tb;
return tb; return tb;

View File

@ -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 /* 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.
*
* Called with mmap_lock held for user-mode emulation.
*/ */
static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc,
tb_page_addr_t phys_page2) 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; unsigned int h;
TranslationBlock **ptb; 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 */ /* add in the physical hash table */
h = tb_phys_hash_func(phys_pc); h = tb_phys_hash_func(phys_pc);
ptb = &tcg_ctx.tb_ctx.tb_phys_hash[h]; 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 #ifdef DEBUG_TB_CHECK
tb_page_check(); tb_page_check();
#endif #endif
mmap_unlock();
} }
/* find the TB 'tb' such that tb[0].tc_ptr <= tc_ptr < /* find the TB 'tb' such that tb[0].tc_ptr <= tc_ptr <