[Qemu-devel] [PATCH v2] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x

Mark Cave-Ayland posted 1 patch 6 years, 8 months ago
Test FreeBSD passed
Test docker-clang@ubuntu passed
Test s390x passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190524065345.25591-1-mark.cave-ayland@ilande.co.uk
Maintainers: David Gibson <david@gibson.dropbear.id.au>
target/ppc/translate/vsx-impl.inc.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH v2] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x
Posted by Mark Cave-Ayland 6 years, 8 months ago
From: Anton Blanchard <anton@ozlabs.org>

During the conversion these instructions were incorrectly treated as
stores. We need to use set_cpu_vsr* and not get_cpu_vsr*.

Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers for VSR register access")
Signed-off-by: Anton Blanchard <anton@ozlabs.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Tested-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/translate/vsx-impl.inc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c
index 199d22da97..cdb44b8b70 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -102,8 +102,7 @@ static void gen_lxvw4x(DisasContext *ctx)
     }
     xth = tcg_temp_new_i64();
     xtl = tcg_temp_new_i64();
-    get_cpu_vsrh(xth, xT(ctx->opcode));
-    get_cpu_vsrl(xtl, xT(ctx->opcode));
+
     gen_set_access_type(ctx, ACCESS_INT);
     EA = tcg_temp_new();
 
@@ -126,6 +125,8 @@ static void gen_lxvw4x(DisasContext *ctx)
         tcg_gen_addi_tl(EA, EA, 8);
         tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
     }
+    set_cpu_vsrh(xT(ctx->opcode), xth);
+    set_cpu_vsrl(xT(ctx->opcode), xtl);
     tcg_temp_free(EA);
     tcg_temp_free_i64(xth);
     tcg_temp_free_i64(xtl);
@@ -185,8 +186,6 @@ static void gen_lxvh8x(DisasContext *ctx)
     }
     xth = tcg_temp_new_i64();
     xtl = tcg_temp_new_i64();
-    get_cpu_vsrh(xth, xT(ctx->opcode));
-    get_cpu_vsrl(xtl, xT(ctx->opcode));
     gen_set_access_type(ctx, ACCESS_INT);
 
     EA = tcg_temp_new();
@@ -197,6 +196,8 @@ static void gen_lxvh8x(DisasContext *ctx)
     if (ctx->le_mode) {
         gen_bswap16x8(xth, xtl, xth, xtl);
     }
+    set_cpu_vsrh(xT(ctx->opcode), xth);
+    set_cpu_vsrl(xT(ctx->opcode), xtl);
     tcg_temp_free(EA);
     tcg_temp_free_i64(xth);
     tcg_temp_free_i64(xtl);
@@ -214,14 +215,14 @@ static void gen_lxvb16x(DisasContext *ctx)
     }
     xth = tcg_temp_new_i64();
     xtl = tcg_temp_new_i64();
-    get_cpu_vsrh(xth, xT(ctx->opcode));
-    get_cpu_vsrl(xtl, xT(ctx->opcode));
     gen_set_access_type(ctx, ACCESS_INT);
     EA = tcg_temp_new();
     gen_addr_reg_index(ctx, EA);
     tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_BEQ);
     tcg_gen_addi_tl(EA, EA, 8);
     tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
+    set_cpu_vsrh(xT(ctx->opcode), xth);
+    set_cpu_vsrl(xT(ctx->opcode), xtl);
     tcg_temp_free(EA);
     tcg_temp_free_i64(xth);
     tcg_temp_free_i64(xtl);
-- 
2.11.0


Re: [Qemu-devel] [PATCH v2] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x
Posted by David Gibson 6 years, 8 months ago
On Fri, May 24, 2019 at 07:53:45AM +0100, Mark Cave-Ayland wrote:
> From: Anton Blanchard <anton@ozlabs.org>
> 
> During the conversion these instructions were incorrectly treated as
> stores. We need to use set_cpu_vsr* and not get_cpu_vsr*.
> 
> Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers for VSR register access")
> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Tested-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>

Applied, thanks.

> ---
>  target/ppc/translate/vsx-impl.inc.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c
> index 199d22da97..cdb44b8b70 100644
> --- a/target/ppc/translate/vsx-impl.inc.c
> +++ b/target/ppc/translate/vsx-impl.inc.c
> @@ -102,8 +102,7 @@ static void gen_lxvw4x(DisasContext *ctx)
>      }
>      xth = tcg_temp_new_i64();
>      xtl = tcg_temp_new_i64();
> -    get_cpu_vsrh(xth, xT(ctx->opcode));
> -    get_cpu_vsrl(xtl, xT(ctx->opcode));
> +
>      gen_set_access_type(ctx, ACCESS_INT);
>      EA = tcg_temp_new();
>  
> @@ -126,6 +125,8 @@ static void gen_lxvw4x(DisasContext *ctx)
>          tcg_gen_addi_tl(EA, EA, 8);
>          tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
>      }
> +    set_cpu_vsrh(xT(ctx->opcode), xth);
> +    set_cpu_vsrl(xT(ctx->opcode), xtl);
>      tcg_temp_free(EA);
>      tcg_temp_free_i64(xth);
>      tcg_temp_free_i64(xtl);
> @@ -185,8 +186,6 @@ static void gen_lxvh8x(DisasContext *ctx)
>      }
>      xth = tcg_temp_new_i64();
>      xtl = tcg_temp_new_i64();
> -    get_cpu_vsrh(xth, xT(ctx->opcode));
> -    get_cpu_vsrl(xtl, xT(ctx->opcode));
>      gen_set_access_type(ctx, ACCESS_INT);
>  
>      EA = tcg_temp_new();
> @@ -197,6 +196,8 @@ static void gen_lxvh8x(DisasContext *ctx)
>      if (ctx->le_mode) {
>          gen_bswap16x8(xth, xtl, xth, xtl);
>      }
> +    set_cpu_vsrh(xT(ctx->opcode), xth);
> +    set_cpu_vsrl(xT(ctx->opcode), xtl);
>      tcg_temp_free(EA);
>      tcg_temp_free_i64(xth);
>      tcg_temp_free_i64(xtl);
> @@ -214,14 +215,14 @@ static void gen_lxvb16x(DisasContext *ctx)
>      }
>      xth = tcg_temp_new_i64();
>      xtl = tcg_temp_new_i64();
> -    get_cpu_vsrh(xth, xT(ctx->opcode));
> -    get_cpu_vsrl(xtl, xT(ctx->opcode));
>      gen_set_access_type(ctx, ACCESS_INT);
>      EA = tcg_temp_new();
>      gen_addr_reg_index(ctx, EA);
>      tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_BEQ);
>      tcg_gen_addi_tl(EA, EA, 8);
>      tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
> +    set_cpu_vsrh(xT(ctx->opcode), xth);
> +    set_cpu_vsrl(xT(ctx->opcode), xtl);
>      tcg_temp_free(EA);
>      tcg_temp_free_i64(xth);
>      tcg_temp_free_i64(xtl);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH v2] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x
Posted by Mark Cave-Ayland 6 years, 8 months ago
On 28/05/2019 02:09, David Gibson wrote:

> On Fri, May 24, 2019 at 07:53:45AM +0100, Mark Cave-Ayland wrote:
>> From: Anton Blanchard <anton@ozlabs.org>
>>
>> During the conversion these instructions were incorrectly treated as
>> stores. We need to use set_cpu_vsr* and not get_cpu_vsr*.
>>
>> Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers for VSR register access")
>> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Tested-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> Applied, thanks.

I'm in the process of preparing a VSX fixes branch to send over to qemu-stable@ so
that Anton's patches make the next 4.0 stable release, however I can't find this
patch in your ppc-for-4.1 branch? Did it get missed somehow?


ATB,

Mark.

>> ---
>>  target/ppc/translate/vsx-impl.inc.c | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c
>> index 199d22da97..cdb44b8b70 100644
>> --- a/target/ppc/translate/vsx-impl.inc.c
>> +++ b/target/ppc/translate/vsx-impl.inc.c
>> @@ -102,8 +102,7 @@ static void gen_lxvw4x(DisasContext *ctx)
>>      }
>>      xth = tcg_temp_new_i64();
>>      xtl = tcg_temp_new_i64();
>> -    get_cpu_vsrh(xth, xT(ctx->opcode));
>> -    get_cpu_vsrl(xtl, xT(ctx->opcode));
>> +
>>      gen_set_access_type(ctx, ACCESS_INT);
>>      EA = tcg_temp_new();
>>  
>> @@ -126,6 +125,8 @@ static void gen_lxvw4x(DisasContext *ctx)
>>          tcg_gen_addi_tl(EA, EA, 8);
>>          tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
>>      }
>> +    set_cpu_vsrh(xT(ctx->opcode), xth);
>> +    set_cpu_vsrl(xT(ctx->opcode), xtl);
>>      tcg_temp_free(EA);
>>      tcg_temp_free_i64(xth);
>>      tcg_temp_free_i64(xtl);
>> @@ -185,8 +186,6 @@ static void gen_lxvh8x(DisasContext *ctx)
>>      }
>>      xth = tcg_temp_new_i64();
>>      xtl = tcg_temp_new_i64();
>> -    get_cpu_vsrh(xth, xT(ctx->opcode));
>> -    get_cpu_vsrl(xtl, xT(ctx->opcode));
>>      gen_set_access_type(ctx, ACCESS_INT);
>>  
>>      EA = tcg_temp_new();
>> @@ -197,6 +196,8 @@ static void gen_lxvh8x(DisasContext *ctx)
>>      if (ctx->le_mode) {
>>          gen_bswap16x8(xth, xtl, xth, xtl);
>>      }
>> +    set_cpu_vsrh(xT(ctx->opcode), xth);
>> +    set_cpu_vsrl(xT(ctx->opcode), xtl);
>>      tcg_temp_free(EA);
>>      tcg_temp_free_i64(xth);
>>      tcg_temp_free_i64(xtl);
>> @@ -214,14 +215,14 @@ static void gen_lxvb16x(DisasContext *ctx)
>>      }
>>      xth = tcg_temp_new_i64();
>>      xtl = tcg_temp_new_i64();
>> -    get_cpu_vsrh(xth, xT(ctx->opcode));
>> -    get_cpu_vsrl(xtl, xT(ctx->opcode));
>>      gen_set_access_type(ctx, ACCESS_INT);
>>      EA = tcg_temp_new();
>>      gen_addr_reg_index(ctx, EA);
>>      tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_BEQ);
>>      tcg_gen_addi_tl(EA, EA, 8);
>>      tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_BEQ);
>> +    set_cpu_vsrh(xT(ctx->opcode), xth);
>> +    set_cpu_vsrl(xT(ctx->opcode), xtl);
>>      tcg_temp_free(EA);
>>      tcg_temp_free_i64(xth);
>>      tcg_temp_free_i64(xtl);
> 


Re: [Qemu-devel] [PATCH v2] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x
Posted by David Gibson 6 years, 8 months ago
On Sun, Jun 02, 2019 at 01:13:44PM +0100, Mark Cave-Ayland wrote:
> On 28/05/2019 02:09, David Gibson wrote:
> 
> > On Fri, May 24, 2019 at 07:53:45AM +0100, Mark Cave-Ayland wrote:
> >> From: Anton Blanchard <anton@ozlabs.org>
> >>
> >> During the conversion these instructions were incorrectly treated as
> >> stores. We need to use set_cpu_vsr* and not get_cpu_vsr*.
> >>
> >> Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() helpers for VSR register access")
> >> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
> >> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> Tested-by: Greg Kurz <groug@kaod.org>
> >> Reviewed-by: Greg Kurz <groug@kaod.org>
> > 
> > Applied, thanks.
> 
> I'm in the process of preparing a VSX fixes branch to send over to qemu-stable@ so
> that Anton's patches make the next 4.0 stable release, however I can't find this
> patch in your ppc-for-4.1 branch? Did it get missed somehow?

Oops.  I think I must have botched a rebase and removed it
accidentally.  I've re-applied it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson