diff mbox series

[5/7] target/hppa: copy new_spc to iasq_f on be,n instruction

Message ID 20240317221431.251515-6-svens@stackframe.org
State New
Headers show
Series few fixes for hppa target | expand

Commit Message

Sven Schnelle March 17, 2024, 10:14 p.m. UTC 
Otherwise the first instruction at the new location gets executed from
the old space.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/translate.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Richard Henderson March 18, 2024, 9:27 p.m. UTC | #1 
On 3/17/24 12:14, Sven Schnelle wrote:
> Otherwise the first instruction at the new location gets executed from
> the old space.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/translate.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 58d7ec1ade..a09112e4ae 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -3777,6 +3777,9 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>           }
>           copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
>           tcg_gen_mov_i64(cpu_iasq_b, new_spc);
> +        if (a->n) {
> +            tcg_gen_mov_i64(cpu_iasq_f, new_spc);
> +        }
>           nullify_set(ctx, a->n);
>       }
>       tcg_gen_lookup_and_goto_ptr();


Without use_nullify_skip(), we're going to execute the next instruction even if we know it 
is nullified (a->n).  This is usually because there's a page crossing or breakpoint, and 
we need to take the exception that might be raised there.

So, we advance the queue:

         copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
         if (ctx->iaoq_b == -1) {
             tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
         }

then put the branch destination at the back of the queue:

         copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
         tcg_gen_mov_i64(cpu_iasq_b, new_spc);

Note that iaoq_b is always -1 on a space change.

So your change does not look correct.
What is the issue that you saw?


r~
Sven Schnelle March 19, 2024, 7:08 a.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/17/24 12:14, Sven Schnelle wrote:
>> Otherwise the first instruction at the new location gets executed from
>> the old space.
>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>> ---
>>   target/hppa/translate.c | 3 +++
>>   1 file changed, 3 insertions(+)
>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>> index 58d7ec1ade..a09112e4ae 100644
>> --- a/target/hppa/translate.c
>> +++ b/target/hppa/translate.c
>> @@ -3777,6 +3777,9 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>>           }
>>           copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
>>           tcg_gen_mov_i64(cpu_iasq_b, new_spc);
>> +        if (a->n) {
>> +            tcg_gen_mov_i64(cpu_iasq_f, new_spc);
>> +        }
>>           nullify_set(ctx, a->n);
>>       }
>>       tcg_gen_lookup_and_goto_ptr();
>
>
> Without use_nullify_skip(), we're going to execute the next
> instruction even if we know it is nullified (a->n).  This is usually
> because there's a page crossing or breakpoint, and we need to take the
> exception that might be raised there.
>
> So, we advance the queue:
>
>         copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
>         if (ctx->iaoq_b == -1) {
>             tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
>         }
>
> then put the branch destination at the back of the queue:
>
>         copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
>         tcg_gen_mov_i64(cpu_iasq_b, new_spc);
>
> Note that iaoq_b is always -1 on a space change.
>
> So your change does not look correct.
> What is the issue that you saw?

I was running the CPU instruction tests from HP Offline Diagnostics, and
some be instruction was ending up in the wrong space. I don't have the
details anymore. I'd say we drop the patch for now and I test it again
later and provide more details.
diff mbox series

Patch

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 58d7ec1ade..a09112e4ae 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3777,6 +3777,9 @@  static bool trans_be(DisasContext *ctx, arg_be *a)
         }
         copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
         tcg_gen_mov_i64(cpu_iasq_b, new_spc);
+        if (a->n) {
+            tcg_gen_mov_i64(cpu_iasq_f, new_spc);
+        }
         nullify_set(ctx, a->n);
     }
     tcg_gen_lookup_and_goto_ptr();