| Message ID | 1459280892-8789-3-git-send-email-sergey.fedorov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 03/29/2016 12:48 PM, Sergey Fedorov wrote: > + while (tb) { > if (tb->pc == pc && > tb->page_addr[0] == phys_page1 && > tb->cs_base == cs_base && > tb->flags == flags) { > - /* check next page if needed */ > - if (tb->page_addr[1] != -1) { > - tb_page_addr_t phys_page2; > > - virt_page2 = (pc & TARGET_PAGE_MASK) + > - TARGET_PAGE_SIZE; > - phys_page2 = get_page_addr_code(env, virt_page2); > + if (tb->page_addr[1] == -1) { > + /* done, we have a match */ > + break; > + } else { > + /* check next page if needed */ > + target_ulong virt_page2 = (pc & TARGET_PAGE_MASK) + > + TARGET_PAGE_SIZE; > + tb_page_addr_t phys_page2 = get_page_addr_code(env, virt_page2); > + > if (tb->page_addr[1] == phys_page2) { > break; > } > - } else { > - break; > } > } FYI, There's an issue here before and after this patch: calling get_page_addr_code will cause an exception to be thrown if the page isn't mapped. Except this is a search routine looking for matching TB's. We shouldn't be raising an exception within this loop. We need a variant of get_page_addr_code that reloads the TLB, if possible, but without generating a fault. Something that I don't think we can actually do with the current tlb_fill target hooks. So not something for this patch, but something to put on someone's radar, hopefully... r~
On 03/29/2016 12:48 PM, Sergey Fedorov wrote: > From: Alex Bennée <alex.bennee@linaro.org> > > Put some comments and improve code structure. This should help reading > the code. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > [Sergey Fedorov: provide commit message; bring back resetting of > tb_invalidated_flag] > Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org> Oh, right. Reviewed-by: Richard Henderson <rth@twiddle.net> r~
diff --git a/cpu-exec.c b/cpu-exec.c index fd92452f16f6..44116f180859 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -214,10 +214,9 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, uint64_t flags) { CPUArchState *env = (CPUArchState *)cpu->env_ptr; - TranslationBlock *tb, **ptb1; + TranslationBlock *tb, **tb_hash_head, **ptb1; unsigned int h; tb_page_addr_t phys_pc, phys_page1; - target_ulong virt_page2; tcg_ctx.tb_ctx.tb_invalidated_flag = 0; @@ -225,37 +224,42 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, phys_pc = get_page_addr_code(env, pc); phys_page1 = phys_pc & TARGET_PAGE_MASK; h = tb_phys_hash_func(phys_pc); - ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h]; - for(;;) { - tb = *ptb1; - if (!tb) { - return NULL; - } + + /* Start at head of the hash entry */ + ptb1 = tb_hash_head = &tcg_ctx.tb_ctx.tb_phys_hash[h]; + tb = *ptb1; + + while (tb) { if (tb->pc == pc && tb->page_addr[0] == phys_page1 && tb->cs_base == cs_base && tb->flags == flags) { - /* check next page if needed */ - if (tb->page_addr[1] != -1) { - tb_page_addr_t phys_page2; - virt_page2 = (pc & TARGET_PAGE_MASK) + - TARGET_PAGE_SIZE; - phys_page2 = get_page_addr_code(env, virt_page2); + if (tb->page_addr[1] == -1) { + /* done, we have a match */ + break; + } else { + /* check next page if needed */ + target_ulong virt_page2 = (pc & TARGET_PAGE_MASK) + + TARGET_PAGE_SIZE; + tb_page_addr_t phys_page2 = get_page_addr_code(env, virt_page2); + if (tb->page_addr[1] == phys_page2) { break; } - } else { - break; } } + ptb1 = &tb->phys_hash_next; + tb = *ptb1; } - /* 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; + if (tb) { + /* Move the TB to the head of the list */ + *ptb1 = tb->phys_hash_next; + tb->phys_hash_next = *tb_hash_head; + *tb_hash_head = tb; + } return tb; }