| Message ID | 1422287596-5621-1-git-send-email-leon.alrae@imgtec.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 26 Jan 2015, Leon Alrae wrote: > The test is supposed to terminate TB if the end of the page is reached. > However, with current implementation it may never succeed for microMIPS or > mips16. > > Reported-by: Richard Henderson <rth@twiddle.net> > Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> > --- I'm not sure if you need this, but just in case it helps anyhow. Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org> > diff --git a/target-mips/translate.c b/target-mips/translate.c > index e9d86b2..f33c10c 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -19103,6 +19104,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, > qemu_log("search pc %d\n", search_pc); > > pc_start = tb->pc; > + next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; As a related issue -- I don't know offhand how far we are with small page support, but we may have to revise these macros -- or specifically how TARGET_PAGE_BITS these build on has been defined -- once we get there, to avoid surprises. Just a heads-up! Maciej
On 28/01/2015 00:14, Maciej W. Rozycki wrote: > On Mon, 26 Jan 2015, Leon Alrae wrote: > >> The test is supposed to terminate TB if the end of the page is reached. >> However, with current implementation it may never succeed for microMIPS or >> mips16. >> >> Reported-by: Richard Henderson <rth@twiddle.net> >> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> >> --- > > I'm not sure if you need this, but just in case it helps anyhow. Reviewed by line is always welcome, thanks! > > Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org> > >> diff --git a/target-mips/translate.c b/target-mips/translate.c >> index e9d86b2..f33c10c 100644 >> --- a/target-mips/translate.c >> +++ b/target-mips/translate.c >> @@ -19103,6 +19104,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, >> qemu_log("search pc %d\n", search_pc); >> >> pc_start = tb->pc; >> + next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; > > As a related issue -- I don't know offhand how far we are with small page > support, but we may have to revise these macros -- or specifically how > TARGET_PAGE_BITS these build on has been defined -- once we get there, to > avoid surprises. Just a heads-up! At first glance we aren't missing much to have small pages supported in target-mips. But yes, before we change TARGET_PAGE_BITS we have also to double check that these macros are correctly used in existing code and there is no place where it was assumed that page size is always 4K. Leon
On 01/26/2015 07:53 AM, Leon Alrae wrote: > The test is supposed to terminate TB if the end of the page is reached. > However, with current implementation it may never succeed for microMIPS or > mips16. > > Reported-by: Richard Henderson <rth@twiddle.net> > Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> > --- > target-mips/translate.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <rth@twiddle.net> r~
diff --git a/target-mips/translate.c b/target-mips/translate.c index e9d86b2..f33c10c 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -19091,6 +19091,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, CPUMIPSState *env = &cpu->env; DisasContext ctx; target_ulong pc_start; + target_ulong next_page_start; uint16_t *gen_opc_end; CPUBreakpoint *bp; int j, lj = -1; @@ -19103,6 +19104,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, qemu_log("search pc %d\n", search_pc); pc_start = tb->pc; + next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; gen_opc_end = tcg_ctx.gen_opc_buf + OPC_MAX_SIZE; ctx.pc = pc_start; ctx.saved_pc = -1; @@ -19202,8 +19204,9 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, break; } - if ((ctx.pc & (TARGET_PAGE_SIZE - 1)) == 0) + if (ctx.pc >= next_page_start) { break; + } if (tcg_ctx.gen_opc_ptr >= gen_opc_end) { break;
The test is supposed to terminate TB if the end of the page is reached. However, with current implementation it may never succeed for microMIPS or mips16. Reported-by: Richard Henderson <rth@twiddle.net> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> --- target-mips/translate.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)