1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v3 0/8] target/mips: Maintenance and misc fixes and improvements
  4. View patch

[Qemu-devel] [PATCH v3 5/8] target/mips: Add CP0 BadInstrX register

Aleksandar Markovic posted 8 patches 7 years, 7 months ago
[Qemu-devel] [PATCH v3 5/8] target/mips: Add CP0 BadInstrX register
Posted by Aleksandar Markovic 7 years, 7 months ago 
From: Stefan Markovic <smarkovic@wavecomp.com>

Add CP0 BadInstrX register. This register will be used in nanoMIPS.

Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
Signed-off-by: Yongbok Kim <yongbok.kim@mips.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/cpu.h       |  1 +
 target/mips/machine.c   |  5 +++--
 target/mips/translate.c | 20 +++++++++++++++++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index edbb66d..8ccbc21 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -323,6 +323,7 @@ struct CPUMIPSState {
     target_ulong CP0_BadVAddr;
     uint32_t CP0_BadInstr;
     uint32_t CP0_BadInstrP;
+    uint32_t CP0_BadInstrX;
     int32_t CP0_Count;
     target_ulong CP0_EntryHi;
 #define CP0EnHi_EHINV 10
diff --git a/target/mips/machine.c b/target/mips/machine.c
index 20100d5..5ba78ac 100644
--- a/target/mips/machine.c
+++ b/target/mips/machine.c
@@ -212,8 +212,8 @@ const VMStateDescription vmstate_tlb = {
 
 const VMStateDescription vmstate_mips_cpu = {
     .name = "cpu",
-    .version_id = 10,
-    .minimum_version_id = 10,
+    .version_id = 11,
+    .minimum_version_id = 11,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
         /* Active TC */
@@ -266,6 +266,7 @@ const VMStateDescription vmstate_mips_cpu = {
         VMSTATE_UINTTL(env.CP0_BadVAddr, MIPSCPU),
         VMSTATE_UINT32(env.CP0_BadInstr, MIPSCPU),
         VMSTATE_UINT32(env.CP0_BadInstrP, MIPSCPU),
+        VMSTATE_UINT32(env.CP0_BadInstrX, MIPSCPU),
         VMSTATE_INT32(env.CP0_Count, MIPSCPU),
         VMSTATE_UINTTL(env.CP0_EntryHi, MIPSCPU),
         VMSTATE_INT32(env.CP0_Compare, MIPSCPU),
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 88699ae..0562851 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -5315,7 +5315,12 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
             gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
             rn = "BadInstrP";
             break;
-        default:
+        case 3:
+            CP0_CHECK(ctx->bi);
+            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
+            rn = "BadInstrX";
+            break;
+       default:
             goto cp0_unimplemented;
         }
         break;
@@ -6006,6 +6011,10 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
             /* ignored */
             rn = "BadInstrP";
             break;
+        case 3:
+            /* ignored */
+            rn = "BadInstrX";
+            break;
         default:
             goto cp0_unimplemented;
         }
@@ -6711,6 +6720,11 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
             gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
             rn = "BadInstrP";
             break;
+        case 3:
+            CP0_CHECK(ctx->bi);
+            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
+            rn = "BadInstrX";
+            break;
         default:
             goto cp0_unimplemented;
         }
@@ -7385,6 +7399,10 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
             /* ignored */
             rn = "BadInstrP";
             break;
+        case 3:
+            /* ignored */
+            rn = "BadInstrX";
+            break;
         default:
             goto cp0_unimplemented;
         }
-- 
2.7.4
Re: [Qemu-devel] [PATCH v3 5/8] target/mips: Add CP0 BadInstrX register
Posted by Philippe Mathieu-Daudé 7 years, 7 months ago 
Hi Aleksandar,

On 07/04/2018 04:30 PM, Aleksandar Markovic wrote:
> From: Stefan Markovic <smarkovic@wavecomp.com>
> 
> Add CP0 BadInstrX register. This register will be used in nanoMIPS.
> 
> Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> Signed-off-by: Yongbok Kim <yongbok.kim@mips.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/cpu.h       |  1 +
>  target/mips/machine.c   |  5 +++--
>  target/mips/translate.c | 20 +++++++++++++++++++-
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index edbb66d..8ccbc21 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -323,6 +323,7 @@ struct CPUMIPSState {
>      target_ulong CP0_BadVAddr;
>      uint32_t CP0_BadInstr;
>      uint32_t CP0_BadInstrP;
> +    uint32_t CP0_BadInstrX;
>      int32_t CP0_Count;
>      target_ulong CP0_EntryHi;
>  #define CP0EnHi_EHINV 10
> diff --git a/target/mips/machine.c b/target/mips/machine.c
> index 20100d5..5ba78ac 100644
> --- a/target/mips/machine.c
> +++ b/target/mips/machine.c
> @@ -212,8 +212,8 @@ const VMStateDescription vmstate_tlb = {
>  
>  const VMStateDescription vmstate_mips_cpu = {
>      .name = "cpu",
> -    .version_id = 10,
> -    .minimum_version_id = 10,
> +    .version_id = 11,
> +    .minimum_version_id = 11,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
>          /* Active TC */
> @@ -266,6 +266,7 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_UINTTL(env.CP0_BadVAddr, MIPSCPU),
>          VMSTATE_UINT32(env.CP0_BadInstr, MIPSCPU),
>          VMSTATE_UINT32(env.CP0_BadInstrP, MIPSCPU),
> +        VMSTATE_UINT32(env.CP0_BadInstrX, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Count, MIPSCPU),
>          VMSTATE_UINTTL(env.CP0_EntryHi, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Compare, MIPSCPU),
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 88699ae..0562851 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -5315,7 +5315,12 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
>              rn = "BadInstrP";
>              break;
> -        default:
> +        case 3:
> +            CP0_CHECK(ctx->bi);
> +            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
> +            rn = "BadInstrX";
> +            break;
> +       default:
>              goto cp0_unimplemented;
>          }
>          break;
> @@ -6006,6 +6011,10 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              /* ignored */
>              rn = "BadInstrP";
>              break;
> +        case 3:
> +            /* ignored */
> +            rn = "BadInstrX";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -6711,6 +6720,11 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
>              rn = "BadInstrP";
>              break;
> +        case 3:
> +            CP0_CHECK(ctx->bi);
> +            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));

I'm unsure re-using gen_mfc0_load32() is enough, shouldn't we zero the
16 lower bits?

> +            rn = "BadInstrX";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -7385,6 +7399,10 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              /* ignored */
>              rn = "BadInstrP";
>              break;
> +        case 3:
> +            /* ignored */
> +            rn = "BadInstrX";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
>
Re: [Qemu-devel] [PATCH v3 5/8] target/mips: Add CP0 BadInstrX register
Posted by Stefan Markovic 7 years, 7 months ago 
Hi Philippe,

Following fix will be added in v4:


diff --git a/target-mips/translate.c b/target-mips/translate.c
index 79a59fd..98ff8d0 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -5354,6 +5354,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         case 3:
             CP0_CHECK(ctx->bi);
             gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
+            tcg_gen_andi_i32(arg, arg, 0xffff0000);
             rn = "BadInstrX";
             break;
         default:
@@ -6719,6 +6720,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         case 3:
             CP0_CHECK(ctx->bi);
             gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
+            tcg_gen_andi_i32(arg, arg, 0xffff0000);
             rn = "BadInstrX";
             break;
         default:


Regards,
Stefan


________________________________
From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> on behalf of Philippe Mathieu-Daudé <f4bug@amsat.org>
Sent: Wednesday, July 4, 2018 10:31:27 PM
To: Aleksandar Markovic; qemu-devel@nongnu.org; Richard Henderson
Cc: aurelien@aurel32.net; Aleksandar Markovic; Stefan Markovic; Petar Jovanovic; Paul Burton
Subject: Re: [PATCH v3 5/8] target/mips: Add CP0 BadInstrX register

Hi Aleksandar,

On 07/04/2018 04:30 PM, Aleksandar Markovic wrote:
> From: Stefan Markovic <smarkovic@wavecomp.com>
>
> Add CP0 BadInstrX register. This register will be used in nanoMIPS.
>
> Signed-off-by: Stefan Markovic <smarkovic@wavecomp.com>
> Signed-off-by: Yongbok Kim <yongbok.kim@mips.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/cpu.h       |  1 +
>  target/mips/machine.c   |  5 +++--
>  target/mips/translate.c | 20 +++++++++++++++++++-
>  3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index edbb66d..8ccbc21 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -323,6 +323,7 @@ struct CPUMIPSState {
>      target_ulong CP0_BadVAddr;
>      uint32_t CP0_BadInstr;
>      uint32_t CP0_BadInstrP;
> +    uint32_t CP0_BadInstrX;
>      int32_t CP0_Count;
>      target_ulong CP0_EntryHi;
>  #define CP0EnHi_EHINV 10
> diff --git a/target/mips/machine.c b/target/mips/machine.c
> index 20100d5..5ba78ac 100644
> --- a/target/mips/machine.c
> +++ b/target/mips/machine.c
> @@ -212,8 +212,8 @@ const VMStateDescription vmstate_tlb = {
>
>  const VMStateDescription vmstate_mips_cpu = {
>      .name = "cpu",
> -    .version_id = 10,
> -    .minimum_version_id = 10,
> +    .version_id = 11,
> +    .minimum_version_id = 11,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
>          /* Active TC */
> @@ -266,6 +266,7 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_UINTTL(env.CP0_BadVAddr, MIPSCPU),
>          VMSTATE_UINT32(env.CP0_BadInstr, MIPSCPU),
>          VMSTATE_UINT32(env.CP0_BadInstrP, MIPSCPU),
> +        VMSTATE_UINT32(env.CP0_BadInstrX, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Count, MIPSCPU),
>          VMSTATE_UINTTL(env.CP0_EntryHi, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Compare, MIPSCPU),
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 88699ae..0562851 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -5315,7 +5315,12 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
>              rn = "BadInstrP";
>              break;
> -        default:
> +        case 3:
> +            CP0_CHECK(ctx->bi);
> +            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
> +            rn = "BadInstrX";
> +            break;
> +       default:
>              goto cp0_unimplemented;
>          }
>          break;
> @@ -6006,6 +6011,10 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              /* ignored */
>              rn = "BadInstrP";
>              break;
> +        case 3:
> +            /* ignored */
> +            rn = "BadInstrX";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -6711,6 +6720,11 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
>              rn = "BadInstrP";
>              break;
> +        case 3:
> +            CP0_CHECK(ctx->bi);
> +            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));

I'm unsure re-using gen_mfc0_load32() is enough, shouldn't we zero the
16 lower bits?

> +            rn = "BadInstrX";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -7385,6 +7399,10 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              /* ignored */
>              rn = "BadInstrP";
>              break;
> +        case 3:
> +            /* ignored */
> +            rn = "BadInstrX";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
>
Re: [Qemu-devel]?==?utf-8?q? [PATCH v3 5/8] target/mips: Add CP0 BadInstrX register
Posted by Aleksandar Markovic 7 years, 7 months ago 
Hi, Philippe!

> > @@ -6711,6 +6720,11 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
> > gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrP));
> > rn = "BadInstrP";
> > break;
> > + case 3:
> > + CP0_CHECK(ctx->bi);
> > + gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_BadInstrX));
> 
> I'm unsure re-using gen_mfc0_load32() is enough, shouldn't we zero the
> 16 lower bits?

I contacted Stefan, and he says that you are right - zeroing 16 lower bits seems to be necessary. We'll hopefully fix this in v4.

Yours,
Aleksandar

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.