1. Patchew
  2. QEMU
  3. View series

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

Giuseppe Musacchio posted 1 patch 2 years, 11 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210518133020.58927-1-thatlemon@gmail.com
Maintainers: Greg Kurz <groug@kaod.org>, David Gibson <david@gibson.dropbear.id.au>
target/ppc/translate/vsx-impl.c.inc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx
Posted by Giuseppe Musacchio 2 years, 11 months ago 
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
Posted by Paul A. Clarke 2 years, 11 months ago 
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
Posted by David Gibson 2 years, 11 months ago 
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
Re: [PATCH v2] target/ppc: Fix load endianness for lxvwsx/lxvdsx
Posted by David Gibson 2 years, 11 months ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.