On 19/04/16 14:37, Alex Bennée wrote: > Sergey Fedorov <sergey.fedo...@linaro.org> writes: (snip) >> diff --git a/cpu-exec.c b/cpu-exec.c >> index bbfcbfb54385..065cc9159477 100644 >> --- a/cpu-exec.c >> +++ b/cpu-exec.c >> @@ -508,11 +508,8 @@ int cpu_exec(CPUState *cpu) >> next_tb = 0; >> tcg_ctx.tb_ctx.tb_invalidated_flag = 0; >> } >> - /* see if we can patch the calling TB. When the TB >> - spans two pages, we cannot safely do a direct >> - jump. */ >> - if (next_tb != 0 && tb->page_addr[1] == -1 >> - && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { >> + /* See if we can patch the calling TB. */ >> + if (next_tb != 0 && >> !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { >> tb_add_jump((TranslationBlock *)(next_tb & >> ~TB_EXIT_MASK), >> next_tb & TB_EXIT_MASK, tb); >> } > We've already discussed on IRC the confusion of next_tb ;-)
Yes, I'd rather add a patch to clean up 'next_tb' naming to "tcg: Misc clean-up patches" series. This series is to deal with direct block chaining only. >> diff --git a/target-alpha/translate.c b/target-alpha/translate.c >> index 5b86992dd367..5fa66309ce2e 100644 >> --- a/target-alpha/translate.c >> +++ b/target-alpha/translate.c >> @@ -464,7 +464,10 @@ static bool use_goto_tb(DisasContext *ctx, uint64_t >> dest) >> if (in_superpage(ctx, dest)) { >> return true; >> } >> - /* Check for the dest on the same page as the start of the TB. */ >> + /* Direct jumps with goto_tb are only safe within the page this TB >> resides >> + * in because we don't take care of direct jumps when address mapping >> + * changes. >> + */ > I'm all for harmonising the comments but I think for the common case we > could refer to a central location for the page linking rules and only > expand the comment for subtle differences between the front ends. You mean, it'd be better to put a detailed explanation in a comment for e.g. tlb_flush() and refer to it in every place like this? >> return ((ctx->tb->pc ^ dest) & TARGET_PAGE_MASK) == 0; >> } (snip) >> diff --git a/target-arm/translate.c b/target-arm/translate.c >> index 940ec8d981d1..aeb3e84e8d40 100644 >> --- a/target-arm/translate.c >> +++ b/target-arm/translate.c >> @@ -4054,7 +4054,12 @@ static inline void gen_goto_tb(DisasContext *s, int >> n, target_ulong dest) >> TranslationBlock *tb; >> >> tb = s->tb; >> - if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)) { >> + /* Direct jumps with goto_tb are only safe within the pages this TB >> resides >> + * in because we don't take care of direct jumps when address mapping >> + * changes. >> + */ >> + if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) || >> + ((s->pc - 1) & TARGET_PAGE_MASK) == (dest & >> TARGET_PAGE_MASK)) { > Isn't this check avoided by the fact the translate loop bails on end_of_page? A TB can straddle a page boundary in Thumb mode. A TB can begin with an T32 instruction straddling a page boundary, thus this check should be useful :) Kind regards, Sergey