[PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx

[PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx

[Giuseppe Musacchio]

TARGET_WORDS_BIGENDIAN may not match the machine endianness if that's a
runtime-configurable parameter.

Fixes: bcb0b7b1a1c05707304f80ca6f523d557816f85c
Fixes: afae37d98ae991c0792c867dbd9f32f988044318
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/212

Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
 target/ppc/translate/vsx-impl.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index b817d31260..57a7f73bba 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -139,7 +139,7 @@ static void gen_lxvwsx(DisasContext *ctx)
     gen_addr_reg_index(ctx, EA);
 
     data = tcg_temp_new_i32();
-    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, MO_TEUL);
+    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, DEF_MEMOP(MO_UL));
     tcg_gen_gvec_dup_i32(MO_UL, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
 
     tcg_temp_free(EA);
@@ -162,7 +162,7 @@ static void gen_lxvdsx(DisasContext *ctx)
     gen_addr_reg_index(ctx, EA);
 
     data = tcg_temp_new_i64();
-    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_TEQ);
+    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, DEF_MEMOP(MO_Q));
     tcg_gen_gvec_dup_i64(MO_Q, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
 
     tcg_temp_free(EA);
-- 
2.30.2

Re: [PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx

[Paul A. Clarke]

Thanks, all!  My original patch which addressed this issue
for me probably should've been labeled as an RFC.  Thanks for your
willingness to review it, in spite of its problems.  It was a bit
of a stab in the dark. I hope it at least helped get to this solution.

Tested-by: Paul A. Clarke <pc@us.ibm.com>

Do I need to do anything about the GitLab issue?
https://gitlab.com/qemu-project/qemu/-/issues/212

(I couldn't even figure out how to subscribe to my own issue,
or if I already was, or who might have been notified of its
existence, or if discussion should happen there or here.)

On Tue, May 18, 2021 at 03:30:20PM +0200, Giuseppe Musacchio wrote:
> TARGET_WORDS_BIGENDIAN may not match the machine endianness if that's a
> runtime-configurable parameter.
> 
> Fixes: bcb0b7b1a1c05707304f80ca6f523d557816f85c
> Fixes: afae37d98ae991c0792c867dbd9f32f988044318
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/212
> 
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
>  target/ppc/translate/vsx-impl.c.inc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
> index b817d31260..57a7f73bba 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -139,7 +139,7 @@ static void gen_lxvwsx(DisasContext *ctx)
>      gen_addr_reg_index(ctx, EA);
>  
>      data = tcg_temp_new_i32();
> -    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, MO_TEUL);
> +    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, DEF_MEMOP(MO_UL));
>      tcg_gen_gvec_dup_i32(MO_UL, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
>  
>      tcg_temp_free(EA);
> @@ -162,7 +162,7 @@ static void gen_lxvdsx(DisasContext *ctx)
>      gen_addr_reg_index(ctx, EA);
>  
>      data = tcg_temp_new_i64();
> -    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_TEQ);
> +    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, DEF_MEMOP(MO_Q));
>      tcg_gen_gvec_dup_i64(MO_Q, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
>  
>      tcg_temp_free(EA);
> -- 
> 2.30.2

PC

Re: [PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

On Tue, May 18, 2021 at 03:30:20PM +0200, Giuseppe Musacchio wrote:
> TARGET_WORDS_BIGENDIAN may not match the machine endianness if that's a
> runtime-configurable parameter.
> 
> Fixes: bcb0b7b1a1c05707304f80ca6f523d557816f85c
> Fixes: afae37d98ae991c0792c867dbd9f32f988044318
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/212
> 
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>

That looks more like it.  Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/translate/vsx-impl.c.inc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
> index b817d31260..57a7f73bba 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -139,7 +139,7 @@ static void gen_lxvwsx(DisasContext *ctx)
>      gen_addr_reg_index(ctx, EA);
>  
>      data = tcg_temp_new_i32();
> -    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, MO_TEUL);
> +    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, DEF_MEMOP(MO_UL));
>      tcg_gen_gvec_dup_i32(MO_UL, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
>  
>      tcg_temp_free(EA);
> @@ -162,7 +162,7 @@ static void gen_lxvdsx(DisasContext *ctx)
>      gen_addr_reg_index(ctx, EA);
>  
>      data = tcg_temp_new_i64();
> -    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_TEQ);
> +    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, DEF_MEMOP(MO_Q));
>      tcg_gen_gvec_dup_i64(MO_Q, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
>  
>      tcg_temp_free(EA);

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]

On Tue, May 18, 2021 at 10:42:09AM -0500, Paul A. Clarke wrote:
> Thanks, all!  My original patch which addressed this issue
> for me probably should've been labeled as an RFC.  Thanks for your
> willingness to review it, in spite of its problems.  It was a bit
> of a stab in the dark. I hope it at least helped get to this solution.
> 
> Tested-by: Paul A. Clarke <pc@us.ibm.com>
> 
> Do I need to do anything about the GitLab issue?
> https://gitlab.com/qemu-project/qemu/-/issues/212

If you could retest and close the issue once the fix is merged, that
would be helpful, thanks.

> 
> (I couldn't even figure out how to subscribe to my own issue,
> or if I already was, or who might have been notified of its
> existence, or if discussion should happen there or here.)
> 
> On Tue, May 18, 2021 at 03:30:20PM +0200, Giuseppe Musacchio wrote:
> > TARGET_WORDS_BIGENDIAN may not match the machine endianness if that's a
> > runtime-configurable parameter.
> > 
> > Fixes: bcb0b7b1a1c05707304f80ca6f523d557816f85c
> > Fixes: afae37d98ae991c0792c867dbd9f32f988044318
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/212
> > 
> > Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> > ---
> >  target/ppc/translate/vsx-impl.c.inc | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
> > index b817d31260..57a7f73bba 100644
> > --- a/target/ppc/translate/vsx-impl.c.inc
> > +++ b/target/ppc/translate/vsx-impl.c.inc
> > @@ -139,7 +139,7 @@ static void gen_lxvwsx(DisasContext *ctx)
> >      gen_addr_reg_index(ctx, EA);
> >  
> >      data = tcg_temp_new_i32();
> > -    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, MO_TEUL);
> > +    tcg_gen_qemu_ld_i32(data, EA, ctx->mem_idx, DEF_MEMOP(MO_UL));
> >      tcg_gen_gvec_dup_i32(MO_UL, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
> >  
> >      tcg_temp_free(EA);
> > @@ -162,7 +162,7 @@ static void gen_lxvdsx(DisasContext *ctx)
> >      gen_addr_reg_index(ctx, EA);
> >  
> >      data = tcg_temp_new_i64();
> > -    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, MO_TEQ);
> > +    tcg_gen_qemu_ld_i64(data, EA, ctx->mem_idx, DEF_MEMOP(MO_Q));
> >      tcg_gen_gvec_dup_i64(MO_Q, vsr_full_offset(xT(ctx->opcode)), 16, 16, data);
> >  
> >      tcg_temp_free(EA);
> 
> PC
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]