All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN
@ 2017-11-07 14:55 Richard Henderson
  2017-11-07 16:00 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Richard Henderson @ 2017-11-07 14:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-s390x

We added the entry to insn-data.def, but failed to update op_risbg
to match.  No need to special-case the imask inversion, since that
is already ~0 for RISBG (and now RISBGN).

Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/translate.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index dee72a787d..85d0a6c3af 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
     /* Adjust the arguments for the specific insn.  */
     switch (s->fields->op2) {
     case 0x55: /* risbg */
+    case 0x59: /* risbgn */
         i3 &= 63;
         i4 &= 63;
         pmask = ~0;
@@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
         pmask = 0x00000000ffffffffull;
         break;
     default:
-        abort();
+        g_assert_not_reached();
     }
 
     /* MASK is the set of bits to be inserted from R2.
@@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
        insns, we need to keep the other half of the register.  */
     imask = ~mask | ~pmask;
     if (do_zero) {
-        if (s->fields->op2 == 0x55) {
-            imask = 0;
-        } else {
-            imask = ~pmask;
-        }
+        imask = ~pmask;
     }
 
     len = i4 - i3 + 1;
-- 
2.13.6

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
  2017-11-07 14:55 [Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN Richard Henderson
@ 2017-11-07 16:00 ` Thomas Huth
  2017-11-07 17:42   ` Peter Maydell
  2017-11-08 11:50 ` Cornelia Huck
  2017-11-09  9:39 ` Cornelia Huck
  2 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2017-11-07 16:00 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: peter.maydell, qemu-s390x

On 07.11.2017 15:55, Richard Henderson wrote:
> We added the entry to insn-data.def, but failed to update op_risbg
> to match.  No need to special-case the imask inversion, since that
> is already ~0 for RISBG (and now RISBGN).
> 
> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index dee72a787d..85d0a6c3af 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>      /* Adjust the arguments for the specific insn.  */
>      switch (s->fields->op2) {
>      case 0x55: /* risbg */
> +    case 0x59: /* risbgn */
>          i3 &= 63;
>          i4 &= 63;
>          pmask = ~0;
> @@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>          pmask = 0x00000000ffffffffull;
>          break;
>      default:
> -        abort();
> +        g_assert_not_reached();
>      }
>  
>      /* MASK is the set of bits to be inserted from R2.
> @@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>         insns, we need to keep the other half of the register.  */
>      imask = ~mask | ~pmask;
>      if (do_zero) {
> -        if (s->fields->op2 == 0x55) {
> -            imask = 0;
> -        } else {
> -            imask = ~pmask;
> -        }
> +        imask = ~pmask;
>      }
>  
>      len = i4 - i3 + 1;
> 

Looks good.

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
  2017-11-07 16:00 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
@ 2017-11-07 17:42   ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2017-11-07 17:42 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Richard Henderson, QEMU Developers, qemu-s390x

On 7 November 2017 at 16:00, Thomas Huth <thuth@redhat.com> wrote:
> On 07.11.2017 15:55, Richard Henderson wrote:
>> We added the entry to insn-data.def, but failed to update op_risbg
>> to match.  No need to special-case the imask inversion, since that
>> is already ~0 for RISBG (and now RISBGN).
>>
>> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/s390x/translate.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
>> index dee72a787d..85d0a6c3af 100644
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>>      /* Adjust the arguments for the specific insn.  */
>>      switch (s->fields->op2) {
>>      case 0x55: /* risbg */
>> +    case 0x59: /* risbgn */
>>          i3 &= 63;
>>          i4 &= 63;
>>          pmask = ~0;
>> @@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>>          pmask = 0x00000000ffffffffull;
>>          break;
>>      default:
>> -        abort();
>> +        g_assert_not_reached();
>>      }
>>
>>      /* MASK is the set of bits to be inserted from R2.
>> @@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>>         insns, we need to keep the other half of the register.  */
>>      imask = ~mask | ~pmask;
>>      if (do_zero) {
>> -        if (s->fields->op2 == 0x55) {
>> -            imask = 0;
>> -        } else {
>> -            imask = ~pmask;
>> -        }
>> +        imask = ~pmask;
>>      }
>>
>>      len = i4 - i3 + 1;
>>
>
> Looks good.
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Tested-by: Peter Maydell <peter.maydell@linaro.org>

...at least it fixes the simple test case from the bug report.

-- PMM

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
  2017-11-07 14:55 [Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN Richard Henderson
  2017-11-07 16:00 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
@ 2017-11-08 11:50 ` Cornelia Huck
  2017-11-09  7:39   ` Richard Henderson
  2017-11-09  9:39 ` Cornelia Huck
  2 siblings, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2017-11-08 11:50 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, peter.maydell, qemu-s390x

On Tue,  7 Nov 2017 15:55:46 +0100
Richard Henderson <richard.henderson@linaro.org> wrote:

> We added the entry to insn-data.def, but failed to update op_risbg
> to match.  No need to special-case the imask inversion, since that
> is already ~0 for RISBG (and now RISBGN).
> 
> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index dee72a787d..85d0a6c3af 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>      /* Adjust the arguments for the specific insn.  */
>      switch (s->fields->op2) {
>      case 0x55: /* risbg */
> +    case 0x59: /* risbgn */
>          i3 &= 63;
>          i4 &= 63;
>          pmask = ~0;
> @@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>          pmask = 0x00000000ffffffffull;
>          break;
>      default:
> -        abort();
> +        g_assert_not_reached();
>      }
>  
>      /* MASK is the set of bits to be inserted from R2.
> @@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>         insns, we need to keep the other half of the register.  */
>      imask = ~mask | ~pmask;
>      if (do_zero) {
> -        if (s->fields->op2 == 0x55) {
> -            imask = 0;
> -        } else {
> -            imask = ~pmask;
> -        }
> +        imask = ~pmask;
>      }
>  
>      len = i4 - i3 + 1;

I can queue this to s390-fixes (unless there are other takers).

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
  2017-11-08 11:50 ` Cornelia Huck
@ 2017-11-09  7:39   ` Richard Henderson
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2017-11-09  7:39 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-devel, peter.maydell, qemu-s390x

On 11/08/2017 12:50 PM, Cornelia Huck wrote:
> I can queue this to s390-fixes (unless there are other takers).

Please do.  Thanks,


r~

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
  2017-11-07 14:55 [Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN Richard Henderson
  2017-11-07 16:00 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
  2017-11-08 11:50 ` Cornelia Huck
@ 2017-11-09  9:39 ` Cornelia Huck
  2 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2017-11-09  9:39 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, peter.maydell, qemu-s390x

On Tue,  7 Nov 2017 15:55:46 +0100
Richard Henderson <richard.henderson@linaro.org> wrote:

> We added the entry to insn-data.def, but failed to update op_risbg
> to match.  No need to special-case the imask inversion, since that
> is already ~0 for RISBG (and now RISBGN).
> 
> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Thanks, applied to s390-fixes.

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

end of thread, other threads:[~2017-11-09  9:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 14:55 [Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN Richard Henderson
2017-11-07 16:00 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2017-11-07 17:42   ` Peter Maydell
2017-11-08 11:50 ` Cornelia Huck
2017-11-09  7:39   ` Richard Henderson
2017-11-09  9:39 ` Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.