All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0
@ 2015-09-14 12:45 Leon Alrae
  2015-09-14 12:45 ` [Qemu-devel] [PATCH 1/2] target-mips: correct MTC0 instruction on MIPS64 Leon Alrae
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Leon Alrae @ 2015-09-14 12:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: aurelien

This patchset removes the gen_mtc0_store64() which is actually incorrect
as MTC0 instruction in MIPS64 is supposed to move entire content (if
dst CP0 register is 64-bit) without sign extending. It also removes the
gen_mfc0_load64() and replaces the pair of tcg_gen_ld_tl() +
tcg_gen_ext32s_tl() with single tcg_gen_ld32s_tl().

Leon Alrae (2):
  target-mips: correct MTC0 instruction on MIPS64
  target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl()

 target-mips/translate.c | 61 +++++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 37 deletions(-)

-- 
2.1.0

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

• * [Qemu-devel] [PATCH 1/2] target-mips: correct MTC0 instruction on MIPS64
    2015-09-14 12:45 [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Leon Alrae
  @ 2015-09-14 12:45 ` Leon Alrae
    2015-09-14 12:45 ` [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl() Leon Alrae
    2015-09-16  4:46 ` [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Aurelien Jarno
    2 siblings, 0 replies; 6+ messages in thread
  From: Leon Alrae @ 2015-09-14 12:45 UTC (permalink / raw)
    To: qemu-devel; +Cc: aurelien
  
  MTC0 on a 64-bit processor should move entire 64-bit GPR content to CP0
  register.
  
  Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
  ---
   target-mips/translate.c | 18 +++++++-----------
   1 file changed, 7 insertions(+), 11 deletions(-)
  
  diff --git a/target-mips/translate.c b/target-mips/translate.c
  index cd0cf8b..8bb45df 100644
  --- a/target-mips/translate.c
  +++ b/target-mips/translate.c
  @@ -4765,12 +4765,6 @@ static inline void gen_mtc0_store32 (TCGv arg, target_ulong off)
       tcg_temp_free_i32(t0);
   }
   
  -static inline void gen_mtc0_store64 (TCGv arg, target_ulong off)
  -{
  -    tcg_gen_ext32s_tl(arg, arg);
  -    tcg_gen_st_tl(arg, cpu_env, off);
  -}
  -
   static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
   {
       const char *rn = "invalid";
  @@ -5629,12 +5623,14 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
               break;
           case 5:
               CP0_CHECK(ctx->insn_flags & ASE_MT);
  -            gen_mtc0_store64(arg, offsetof(CPUMIPSState, CP0_VPESchedule));
  +            tcg_gen_st_tl(arg, cpu_env,
  +                          offsetof(CPUMIPSState, CP0_VPESchedule));
               rn = "VPESchedule";
               break;
           case 6:
               CP0_CHECK(ctx->insn_flags & ASE_MT);
  -            gen_mtc0_store64(arg, offsetof(CPUMIPSState, CP0_VPEScheFBack));
  +            tcg_gen_st_tl(arg, cpu_env,
  +                          offsetof(CPUMIPSState, CP0_VPEScheFBack));
               rn = "VPEScheFBack";
               break;
           case 7:
  @@ -5884,7 +5880,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 14:
           switch (sel) {
           case 0:
  -            gen_mtc0_store64(arg, offsetof(CPUMIPSState, CP0_EPC));
  +            tcg_gen_st_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EPC));
               rn = "EPC";
               break;
           default:
  @@ -6057,7 +6053,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
           switch (sel) {
           case 0:
               /* EJTAG support */
  -            gen_mtc0_store64(arg, offsetof(CPUMIPSState, CP0_DEPC));
  +            tcg_gen_st_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_DEPC));
               rn = "DEPC";
               break;
           default:
  @@ -6160,7 +6156,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 30:
           switch (sel) {
           case 0:
  -            gen_mtc0_store64(arg, offsetof(CPUMIPSState, CP0_ErrorEPC));
  +            tcg_gen_st_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_ErrorEPC));
               rn = "ErrorEPC";
               break;
           default:
  -- 
  2.1.0
  
  ^ permalink raw reply related	[flat|nested] 6+ messages in thread

• * [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl()
    2015-09-14 12:45 [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Leon Alrae
    2015-09-14 12:45 ` [Qemu-devel] [PATCH 1/2] target-mips: correct MTC0 instruction on MIPS64 Leon Alrae
  @ 2015-09-14 12:45 ` Leon Alrae
    2015-09-16 18:14   ` Richard Henderson
    2015-09-16  4:46 ` [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Aurelien Jarno
    2 siblings, 1 reply; 6+ messages in thread
  From: Leon Alrae @ 2015-09-14 12:45 UTC (permalink / raw)
    To: qemu-devel; +Cc: aurelien
  
  Remove misleading gen_mfc0_load64() which actually loads 32 or 64 bits
  depending whether MIPS32 or MIPS64 and also replace the pair of
  tcg_gen_ld_tl() + tcg_gen_ext32s_tl() with single tcg_gen_ld32s_tl().
  
  Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
  ---
   target-mips/translate.c | 43 +++++++++++++++++--------------------------
   1 file changed, 17 insertions(+), 26 deletions(-)
  
  diff --git a/target-mips/translate.c b/target-mips/translate.c
  index 8bb45df..7fb7c01 100644
  --- a/target-mips/translate.c
  +++ b/target-mips/translate.c
  @@ -4750,12 +4750,6 @@ static inline void gen_mfc0_load32 (TCGv arg, target_ulong off)
       tcg_temp_free_i32(t0);
   }
   
  -static inline void gen_mfc0_load64 (TCGv arg, target_ulong off)
  -{
  -    tcg_gen_ld_tl(arg, cpu_env, off);
  -    tcg_gen_ext32s_tl(arg, arg);
  -}
  -
   static inline void gen_mtc0_store32 (TCGv arg, target_ulong off)
   {
       TCGv_i32 t0 = tcg_temp_new_i32();
  @@ -4972,17 +4966,19 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
               break;
           case 4:
               CP0_CHECK(ctx->insn_flags & ASE_MT);
  -            gen_mfc0_load64(arg, offsetof(CPUMIPSState, CP0_YQMask));
  +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_YQMask));
               rn = "YQMask";
               break;
           case 5:
               CP0_CHECK(ctx->insn_flags & ASE_MT);
  -            gen_mfc0_load64(arg, offsetof(CPUMIPSState, CP0_VPESchedule));
  +            tcg_gen_ld32s_tl(arg, cpu_env,
  +                             offsetof(CPUMIPSState, CP0_VPESchedule));
               rn = "VPESchedule";
               break;
           case 6:
               CP0_CHECK(ctx->insn_flags & ASE_MT);
  -            gen_mfc0_load64(arg, offsetof(CPUMIPSState, CP0_VPEScheFBack));
  +            tcg_gen_ld32s_tl(arg, cpu_env,
  +                             offsetof(CPUMIPSState, CP0_VPEScheFBack));
               rn = "VPEScheFBack";
               break;
           case 7:
  @@ -5078,8 +5074,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 4:
           switch (sel) {
           case 0:
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_Context));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_Context));
               rn = "Context";
               break;
           case 1:
  @@ -5161,8 +5156,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 8:
           switch (sel) {
           case 0:
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env,
  +                             offsetof(CPUMIPSState, CP0_BadVAddr));
               rn = "BadVAddr";
               break;
           case 1:
  @@ -5203,8 +5198,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 10:
           switch (sel) {
           case 0:
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EntryHi));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EntryHi));
               rn = "EntryHi";
               break;
           default:
  @@ -5260,8 +5254,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 14:
           switch (sel) {
           case 0:
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EPC));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EPC));
               rn = "EPC";
               break;
           default:
  @@ -5357,8 +5350,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
           case 0:
   #if defined(TARGET_MIPS64)
               check_insn(ctx, ISA_MIPS3);
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_XContext));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env,
  +                             offsetof(CPUMIPSState, CP0_XContext));
               rn = "XContext";
               break;
   #endif
  @@ -5412,8 +5405,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
           switch (sel) {
           case 0:
               /* EJTAG support */
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_DEPC));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_DEPC));
               rn = "DEPC";
               break;
           default:
  @@ -5520,8 +5512,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
       case 30:
           switch (sel) {
           case 0:
  -            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_ErrorEPC));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env,
  +                             offsetof(CPUMIPSState, CP0_ErrorEPC));
               rn = "ErrorEPC";
               break;
           default:
  @@ -5537,9 +5529,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
               break;
           case 2 ... 7:
               CP0_CHECK(ctx->kscrexist & (1 << sel));
  -            tcg_gen_ld_tl(arg, cpu_env,
  -                          offsetof(CPUMIPSState, CP0_KScratch[sel-2]));
  -            tcg_gen_ext32s_tl(arg, arg);
  +            tcg_gen_ld32s_tl(arg, cpu_env,
  +                             offsetof(CPUMIPSState, CP0_KScratch[sel-2]));
               rn = "KScratch";
               break;
           default:
  -- 
  2.1.0
  
  ^ permalink raw reply related	[flat|nested] 6+ messages in thread

• • * Re: [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl()
      2015-09-14 12:45 ` [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl() Leon Alrae
    @ 2015-09-16 18:14   ` Richard Henderson
      2015-09-16 19:00     ` Leon Alrae
      0 siblings, 1 reply; 6+ messages in thread
    From: Richard Henderson @ 2015-09-16 18:14 UTC (permalink / raw)
      To: Leon Alrae, qemu-devel; +Cc: aurelien
    
    On 09/14/2015 05:45 AM, Leon Alrae wrote:
    > -static inline void gen_mfc0_load64 (TCGv arg, target_ulong off)
    > -{
    > -    tcg_gen_ld_tl(arg, cpu_env, off);
    > -    tcg_gen_ext32s_tl(arg, arg);
    > -}
    > -
    >  static inline void gen_mtc0_store32 (TCGv arg, target_ulong off)
    >  {
    >      TCGv_i32 t0 = tcg_temp_new_i32();
    > @@ -4972,17 +4966,19 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
    >              break;
    >          case 4:
    >              CP0_CHECK(ctx->insn_flags & ASE_MT);
    > -            gen_mfc0_load64(arg, offsetof(CPUMIPSState, CP0_YQMask));
    > +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_YQMask));
    >              rn = "YQMask";
    >              break;
    
    This change is broken for 64-bit guest and big-endian host -- one has to adjust
    the offset in that case.  I suspect that's why the extension was separate to
    begin with...
    
    
    r~
    
    ^ permalink raw reply	[flat|nested] 6+ messages in thread

  • • * Re: [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl()
        2015-09-16 18:14   ` Richard Henderson
      @ 2015-09-16 19:00     ` Leon Alrae
        0 siblings, 0 replies; 6+ messages in thread
      From: Leon Alrae @ 2015-09-16 19:00 UTC (permalink / raw)
        To: Richard Henderson, qemu-devel; +Cc: aurelien
      
      On 16/09/15 19:14, Richard Henderson wrote:
      > On 09/14/2015 05:45 AM, Leon Alrae wrote:
      >> -static inline void gen_mfc0_load64 (TCGv arg, target_ulong off)
      >> -{
      >> -    tcg_gen_ld_tl(arg, cpu_env, off);
      >> -    tcg_gen_ext32s_tl(arg, arg);
      >> -}
      >> -
      >>  static inline void gen_mtc0_store32 (TCGv arg, target_ulong off)
      >>  {
      >>      TCGv_i32 t0 = tcg_temp_new_i32();
      >> @@ -4972,17 +4966,19 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
      >>              break;
      >>          case 4:
      >>              CP0_CHECK(ctx->insn_flags & ASE_MT);
      >> -            gen_mfc0_load64(arg, offsetof(CPUMIPSState, CP0_YQMask));
      >> +            tcg_gen_ld32s_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_YQMask));
      >>              rn = "YQMask";
      >>              break;
      > 
      > This change is broken for 64-bit guest and big-endian host -- one has to adjust
      > the offset in that case.  I suspect that's why the extension was separate to
      > begin with...
      
      Uh, I think I wasn't expecting that ld32s_i64 actually does 32-bit load.
      It means that CP0_UserLocal needs fixing in current code.
      
      Thanks,
      Leon
      
      ^ permalink raw reply	[flat|nested] 6+ messages in thread

• * Re: [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0
    2015-09-14 12:45 [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Leon Alrae
    2015-09-14 12:45 ` [Qemu-devel] [PATCH 1/2] target-mips: correct MTC0 instruction on MIPS64 Leon Alrae
    2015-09-14 12:45 ` [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl() Leon Alrae
  @ 2015-09-16  4:46 ` Aurelien Jarno
    2 siblings, 0 replies; 6+ messages in thread
  From: Aurelien Jarno @ 2015-09-16  4:46 UTC (permalink / raw)
    To: Leon Alrae; +Cc: qemu-devel
  
  On 2015-09-14 13:45, Leon Alrae wrote:
  > This patchset removes the gen_mtc0_store64() which is actually incorrect
  > as MTC0 instruction in MIPS64 is supposed to move entire content (if
  > dst CP0 register is 64-bit) without sign extending. It also removes the
  > gen_mfc0_load64() and replaces the pair of tcg_gen_ld_tl() +
  > tcg_gen_ext32s_tl() with single tcg_gen_ld32s_tl().
  > 
  > Leon Alrae (2):
  >   target-mips: correct MTC0 instruction on MIPS64
  >   target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl()
  
  The existence of tcg_gen_ld32s_tl is relatively recent, that's why we
  had it opened coded up to now.
  
  Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
  
  -- 
  Aurelien Jarno                          GPG: 4096R/1DDD8C9B
  aurelien@aurel32.net                 http://www.aurel32.net
  
  ^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-09-16 19:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 12:45 [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Leon Alrae
2015-09-14 12:45 ` [Qemu-devel] [PATCH 1/2] target-mips: correct MTC0 instruction on MIPS64 Leon Alrae
2015-09-14 12:45 ` [Qemu-devel] [PATCH 2/2] target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl() Leon Alrae
2015-09-16 18:14   ` Richard Henderson
2015-09-16 19:00     ` Leon Alrae
2015-09-16  4:46 ` [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0 Aurelien Jarno

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.