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

* 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

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.