qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Fix missing TRACE exception
@ 2012-10-19 10:17 Julio Guerra
  2012-11-22  9:26 ` Julio Guerra
  2012-11-23 16:06 ` Alexander Graf
  0 siblings, 2 replies; 4+ messages in thread
From: Julio Guerra @ 2012-10-19 10:17 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Alexander Graf

This patch fixes bug 1031698 :
https://bugs.launchpad.net/qemu/+bug/1031698

If we look at the (truncated) translation of the conditional branch
instruction in the test submitted in the bug post, the call to the
exception helper is missing in the "bne-false" chunk of translated
code :

IN:
bne-    0x1800278

OUT:
0xb544236d:  jne    0xb5442396

0xb5442373:  mov    %ebp,(%esp)
0xb5442376:  mov    $0x44,%ebx
0xb544237b:  mov    %ebx,0x4(%esp)
0xb544237f:  mov    $0x1800278,%ebx
0xb5442384:  mov    %ebx,0x25c(%ebp)
0xb544238a:  call   0x827475a
                     ^^^^^^^^^^^^^^^^^^
# OK : call the exception helper function

0xb5442396:  mov    %ebp,(%esp)
0xb5442399:  mov    $0x44,%ebx
0xb544239e:  mov    %ebx,0x4(%esp)
0xb54423a2:  mov    $0x1800270,%ebx
0xb54423a7:  mov    %ebx,0x25c(%ebp)
# KO : missing "call 0x827475a"


Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by
gen_bcond) changes ctx->exception's value to excp's :

gen_bcond()
{
  gen_goto_tb(ctx, 0, ctx->nip + li - 4);
  /* ctx->exception value is POWERPC_EXCP_BRANCH */

  gen_goto_tb(ctx, 1, ctx->nip);
  /* ctx->exception now value is POWERPC_EXCP_TRACE */
}


Making the following gen_goto_tb()'s test false during the second call :

if ((ctx->singlestep_enabled &
    (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
    ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) {
         target_ulong tmp = ctx->nip;
         ctx->nip = dest;
         /* ... and this is the missing call */
         gen_exception(ctx, POWERPC_EXCP_TRACE);
         ctx->nip = tmp;
}

So the patch simply adds the missing matching case, fixing our problem.

Signed-off-by: Julio Guerra <guerr@julio.in>
---
 target-ppc/translate.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -up a/target-ppc/translate.c b/target-ppc/translate.c
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3466,7 +3466,8 @@ static inline void gen_goto_tb(DisasCont
         if (unlikely(ctx->singlestep_enabled)) {
             if ((ctx->singlestep_enabled &
                 (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
-                ctx->exception == POWERPC_EXCP_BRANCH) {
+                (ctx->exception == POWERPC_EXCP_BRANCH ||
+                 ctx->exception == POWERPC_EXCP_TRACE)) {
                 target_ulong tmp = ctx->nip;
                 ctx->nip = dest;
                 gen_exception(ctx, POWERPC_EXCP_TRACE);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix missing TRACE exception
  2012-10-19 10:17 [Qemu-devel] [PATCH] Fix missing TRACE exception Julio Guerra
@ 2012-11-22  9:26 ` Julio Guerra
  2012-11-23 16:06 ` Alexander Graf
  1 sibling, 0 replies; 4+ messages in thread
From: Julio Guerra @ 2012-11-22  9:26 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Alexander Graf

2012/10/19 Julio Guerra <guerr@julio.in>:
> This patch fixes bug 1031698 :
> https://bugs.launchpad.net/qemu/+bug/1031698
>
> If we look at the (truncated) translation of the conditional branch
> instruction in the test submitted in the bug post, the call to the
> exception helper is missing in the "bne-false" chunk of translated
> code :
>
> IN:
> bne-    0x1800278
>
> OUT:
> 0xb544236d:  jne    0xb5442396
>
> 0xb5442373:  mov    %ebp,(%esp)
> 0xb5442376:  mov    $0x44,%ebx
> 0xb544237b:  mov    %ebx,0x4(%esp)
> 0xb544237f:  mov    $0x1800278,%ebx
> 0xb5442384:  mov    %ebx,0x25c(%ebp)
> 0xb544238a:  call   0x827475a
>                      ^^^^^^^^^^^^^^^^^^
> # OK : call the exception helper function
>
> 0xb5442396:  mov    %ebp,(%esp)
> 0xb5442399:  mov    $0x44,%ebx
> 0xb544239e:  mov    %ebx,0x4(%esp)
> 0xb54423a2:  mov    $0x1800270,%ebx
> 0xb54423a7:  mov    %ebx,0x25c(%ebp)
> # KO : missing "call 0x827475a"
>
>
> Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by
> gen_bcond) changes ctx->exception's value to excp's :
>
> gen_bcond()
> {
>   gen_goto_tb(ctx, 0, ctx->nip + li - 4);
>   /* ctx->exception value is POWERPC_EXCP_BRANCH */
>
>   gen_goto_tb(ctx, 1, ctx->nip);
>   /* ctx->exception now value is POWERPC_EXCP_TRACE */
> }
>
>
> Making the following gen_goto_tb()'s test false during the second call :
>
> if ((ctx->singlestep_enabled &
>     (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
>     ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) {
>          target_ulong tmp = ctx->nip;
>          ctx->nip = dest;
>          /* ... and this is the missing call */
>          gen_exception(ctx, POWERPC_EXCP_TRACE);
>          ctx->nip = tmp;
> }
>
> So the patch simply adds the missing matching case, fixing our problem.
>
> Signed-off-by: Julio Guerra <guerr@julio.in>
> ---
>  target-ppc/translate.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff -up a/target-ppc/translate.c b/target-ppc/translate.c
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3466,7 +3466,8 @@ static inline void gen_goto_tb(DisasCont
>          if (unlikely(ctx->singlestep_enabled)) {
>              if ((ctx->singlestep_enabled &
>                  (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
> -                ctx->exception == POWERPC_EXCP_BRANCH) {
> +                (ctx->exception == POWERPC_EXCP_BRANCH ||
> +                 ctx->exception == POWERPC_EXCP_TRACE)) {
>                  target_ulong tmp = ctx->nip;
>                  ctx->nip = dest;
>                  gen_exception(ctx, POWERPC_EXCP_TRACE);


ping

--
Julio Guerra

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix missing TRACE exception
  2012-10-19 10:17 [Qemu-devel] [PATCH] Fix missing TRACE exception Julio Guerra
  2012-11-22  9:26 ` Julio Guerra
@ 2012-11-23 16:06 ` Alexander Graf
  2012-11-23 16:35   ` Andreas Färber
  1 sibling, 1 reply; 4+ messages in thread
From: Alexander Graf @ 2012-11-23 16:06 UTC (permalink / raw)
  To: Julio Guerra; +Cc: qemu-ppc, qemu-devel


On 19.10.2012, at 12:17, Julio Guerra wrote:

> This patch fixes bug 1031698 :
> https://bugs.launchpad.net/qemu/+bug/1031698
> 
> If we look at the (truncated) translation of the conditional branch
> instruction in the test submitted in the bug post, the call to the
> exception helper is missing in the "bne-false" chunk of translated
> code :
> 
> IN:
> bne-    0x1800278
> 
> OUT:
> 0xb544236d:  jne    0xb5442396
> 
> 0xb5442373:  mov    %ebp,(%esp)
> 0xb5442376:  mov    $0x44,%ebx
> 0xb544237b:  mov    %ebx,0x4(%esp)
> 0xb544237f:  mov    $0x1800278,%ebx
> 0xb5442384:  mov    %ebx,0x25c(%ebp)
> 0xb544238a:  call   0x827475a
>                     ^^^^^^^^^^^^^^^^^^
> # OK : call the exception helper function
> 
> 0xb5442396:  mov    %ebp,(%esp)
> 0xb5442399:  mov    $0x44,%ebx
> 0xb544239e:  mov    %ebx,0x4(%esp)
> 0xb54423a2:  mov    $0x1800270,%ebx
> 0xb54423a7:  mov    %ebx,0x25c(%ebp)
> # KO : missing "call 0x827475a"
> 
> 
> Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by
> gen_bcond) changes ctx->exception's value to excp's :
> 
> gen_bcond()
> {
>  gen_goto_tb(ctx, 0, ctx->nip + li - 4);
>  /* ctx->exception value is POWERPC_EXCP_BRANCH */
> 
>  gen_goto_tb(ctx, 1, ctx->nip);
>  /* ctx->exception now value is POWERPC_EXCP_TRACE */
> }
> 
> 
> Making the following gen_goto_tb()'s test false during the second call :
> 
> if ((ctx->singlestep_enabled &
>    (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
>    ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) {
>         target_ulong tmp = ctx->nip;
>         ctx->nip = dest;
>         /* ... and this is the missing call */
>         gen_exception(ctx, POWERPC_EXCP_TRACE);
>         ctx->nip = tmp;
> }
> 
> So the patch simply adds the missing matching case, fixing our problem.
> 
> Signed-off-by: Julio Guerra <guerr@julio.in>

Thanks, applied to ppc-next :)

Alex

> ---
> target-ppc/translate.c |    4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff -up a/target-ppc/translate.c b/target-ppc/translate.c
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3466,7 +3466,8 @@ static inline void gen_goto_tb(DisasCont
>         if (unlikely(ctx->singlestep_enabled)) {
>             if ((ctx->singlestep_enabled &
>                 (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
> -                ctx->exception == POWERPC_EXCP_BRANCH) {
> +                (ctx->exception == POWERPC_EXCP_BRANCH ||
> +                 ctx->exception == POWERPC_EXCP_TRACE)) {
>                 target_ulong tmp = ctx->nip;
>                 ctx->nip = dest;
>                 gen_exception(ctx, POWERPC_EXCP_TRACE);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix missing TRACE exception
  2012-11-23 16:06 ` Alexander Graf
@ 2012-11-23 16:35   ` Andreas Färber
  0 siblings, 0 replies; 4+ messages in thread
From: Andreas Färber @ 2012-11-23 16:35 UTC (permalink / raw)
  To: Alexander Graf; +Cc: Julio Guerra, qemu-ppc, qemu-devel

Am 23.11.2012 17:06, schrieb Alexander Graf:
> 
> On 19.10.2012, at 12:17, Julio Guerra wrote:
> 
>> This patch fixes bug 1031698 :
>> https://bugs.launchpad.net/qemu/+bug/1031698
>>
>> If we look at the (truncated) translation of the conditional branch
>> instruction in the test submitted in the bug post, the call to the
>> exception helper is missing in the "bne-false" chunk of translated
>> code :
>>
>> IN:
>> bne-    0x1800278
>>
>> OUT:
>> 0xb544236d:  jne    0xb5442396
>>
>> 0xb5442373:  mov    %ebp,(%esp)
>> 0xb5442376:  mov    $0x44,%ebx
>> 0xb544237b:  mov    %ebx,0x4(%esp)
>> 0xb544237f:  mov    $0x1800278,%ebx
>> 0xb5442384:  mov    %ebx,0x25c(%ebp)
>> 0xb544238a:  call   0x827475a
>>                     ^^^^^^^^^^^^^^^^^^
>> # OK : call the exception helper function
>>
>> 0xb5442396:  mov    %ebp,(%esp)
>> 0xb5442399:  mov    $0x44,%ebx
>> 0xb544239e:  mov    %ebx,0x4(%esp)
>> 0xb54423a2:  mov    $0x1800270,%ebx
>> 0xb54423a7:  mov    %ebx,0x25c(%ebp)
>> # KO : missing "call 0x827475a"
>>
>>
>> Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by
>> gen_bcond) changes ctx->exception's value to excp's :
>>
>> gen_bcond()
>> {
>>  gen_goto_tb(ctx, 0, ctx->nip + li - 4);
>>  /* ctx->exception value is POWERPC_EXCP_BRANCH */
>>
>>  gen_goto_tb(ctx, 1, ctx->nip);
>>  /* ctx->exception now value is POWERPC_EXCP_TRACE */
>> }
>>
>>
>> Making the following gen_goto_tb()'s test false during the second call :
>>
>> if ((ctx->singlestep_enabled &
>>    (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
>>    ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) {
>>         target_ulong tmp = ctx->nip;
>>         ctx->nip = dest;
>>         /* ... and this is the missing call */
>>         gen_exception(ctx, POWERPC_EXCP_TRACE);
>>         ctx->nip = tmp;
>> }
>>
>> So the patch simply adds the missing matching case, fixing our problem.
>>
>> Signed-off-by: Julio Guerra <guerr@julio.in>
> 
> Thanks, applied to ppc-next :)

Please don't forget to add the missing "target-ppc: ". :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-11-23 16:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-19 10:17 [Qemu-devel] [PATCH] Fix missing TRACE exception Julio Guerra
2012-11-22  9:26 ` Julio Guerra
2012-11-23 16:06 ` Alexander Graf
2012-11-23 16:35   ` Andreas Färber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).