[Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation

[Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation

[Leon Alrae]

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(-)

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;
-- 
2.1.0

Re: [Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation

[Maciej W. Rozycki]

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

Re: [Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation

[Leon Alrae]

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

Re: [Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation

[Richard Henderson]

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~