diff mbox

[15/77] ppc: Fix sign extension issue in mtmsr(d) emulation

Message ID 1447201710-10229-16-git-send-email-benh@kernel.crashing.org
State New
Headers show

Commit Message

Benjamin Herrenschmidt Nov. 11, 2015, 12:27 a.m. UTC 
From: Michael Neuling <mikey@neuling.org>

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

David Gibson Nov. 19, 2015, 6:26 a.m. UTC | #1 
On Wed, Nov 11, 2015 at 11:27:28AM +1100, Benjamin Herrenschmidt wrote:
> From: Michael Neuling <mikey@neuling.org>
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Looks correct, though my memory of C promotion rules is obviously a
bit stale, since I'm not immediately seeing why the original was wrong.

> ---
>  target-ppc/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index bd5df40..3974cd2 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4391,7 +4391,7 @@ static void gen_mtmsrd(DisasContext *ctx)
>          /* Special form that does not need any synchronisation */
>          TCGv t0 = tcg_temp_new();
>          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
> -        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 << MSR_EE)));
> +        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
>          tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
>          tcg_temp_free(t0);
>      } else {
> @@ -4422,7 +4422,7 @@ static void gen_mtmsr(DisasContext *ctx)
>          /* Special form that does not need any synchronisation */
>          TCGv t0 = tcg_temp_new();
>          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
> -        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 << MSR_EE)));
> +        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
>          tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
>          tcg_temp_free(t0);
>      } else {
Benjamin Herrenschmidt Nov. 19, 2015, 10:26 a.m. UTC | #2 
On Thu, 2015-11-19 at 17:26 +1100, David Gibson wrote:
> On Wed, Nov 11, 2015 at 11:27:28AM +1100, Benjamin Herrenschmidt

> wrote:

> > From: Michael Neuling <mikey@neuling.org>

> > 

> > Signed-off-by: Michael Neuling <mikey@neuling.org>

> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> 

> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 

> Looks correct, though my memory of C promotion rules is obviously a

> bit stale, since I'm not immediately seeing why the original was

> wrong.


Well, at least it makes things work :-)

> > ---

> >  target-ppc/translate.c | 4 ++--

> >  1 file changed, 2 insertions(+), 2 deletions(-)

> > 

> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c

> > index bd5df40..3974cd2 100644

> > --- a/target-ppc/translate.c

> > +++ b/target-ppc/translate.c

> > @@ -4391,7 +4391,7 @@ static void gen_mtmsrd(DisasContext *ctx)

> >          /* Special form that does not need any synchronisation */

> >          TCGv t0 = tcg_temp_new();

> >          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 <<

> > MSR_RI) | (1 << MSR_EE));

> > -        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 <<

> > MSR_EE)));

> > +        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 <<

> > MSR_RI) | (1 << MSR_EE)));

> >          tcg_gen_or_tl(cpu_msr, cpu_msr, t0);

> >          tcg_temp_free(t0);

> >      } else {

> > @@ -4422,7 +4422,7 @@ static void gen_mtmsr(DisasContext *ctx)

> >          /* Special form that does not need any synchronisation */

> >          TCGv t0 = tcg_temp_new();

> >          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 <<

> > MSR_RI) | (1 << MSR_EE));

> > -        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 <<

> > MSR_EE)));

> > +        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 <<

> > MSR_RI) | (1 << MSR_EE)));

> >          tcg_gen_or_tl(cpu_msr, cpu_msr, t0);

> >          tcg_temp_free(t0);

> >      } else {

>
diff mbox

Patch

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index bd5df40..3974cd2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4391,7 +4391,7 @@  static void gen_mtmsrd(DisasContext *ctx)
         /* Special form that does not need any synchronisation */
         TCGv t0 = tcg_temp_new();
         tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
-        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 << MSR_EE)));
+        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
         tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
         tcg_temp_free(t0);
     } else {
@@ -4422,7 +4422,7 @@  static void gen_mtmsr(DisasContext *ctx)
         /* Special form that does not need any synchronisation */
         TCGv t0 = tcg_temp_new();
         tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
-        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 << MSR_EE)));
+        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
         tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
         tcg_temp_free(t0);
     } else {