[Qemu-devel] [PATCH v7 09/80] target/mips: Check ELPA flag only in some cases of MFHC0 and MTHC0

Aleksandar Markovic posted 80 patches 7 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v7 09/80] target/mips: Check ELPA flag only in some cases of MFHC0 and MTHC0
Posted by Aleksandar Markovic 7 years, 3 months ago
From: Yongbok Kim <yongbok.kim@mips.com>

MFHC0 and MTHC0 used to handle EntryLo0 and EntryLo1 registers only,
and placing ELPA flag checks before switch statement were technically
correct. However, after adding handling more registers, these checks
should be moved to act only in cases of handling EntryLo0 and
EntryLo1.

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Yongbok Kim <yongbok.kim@mips.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 841c0c8..bc1f21f 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4896,12 +4896,11 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
 {
     const char *rn = "invalid";
 
-    CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
-
     switch (reg) {
     case 2:
         switch (sel) {
         case 0:
+            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
             gen_mfhc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo0));
             rn = "EntryLo0";
             break;
@@ -4912,6 +4911,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     case 3:
         switch (sel) {
         case 0:
+            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
             gen_mfhc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo1));
             rn = "EntryLo1";
             break;
@@ -4964,12 +4964,11 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     const char *rn = "invalid";
     uint64_t mask = ctx->PAMask >> 36;
 
-    CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
-
     switch (reg) {
     case 2:
         switch (sel) {
         case 0:
+            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
             tcg_gen_andi_tl(arg, arg, mask);
             gen_mthc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo0));
             rn = "EntryLo0";
@@ -4981,6 +4980,7 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     case 3:
         switch (sel) {
         case 0:
+            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
             tcg_gen_andi_tl(arg, arg, mask);
             gen_mthc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo1));
             rn = "EntryLo1";
-- 
2.7.4


Re: [Qemu-devel] [PATCH v7 09/80] target/mips: Check ELPA flag only in some cases of MFHC0 and MTHC0
Posted by Philippe Mathieu-Daudé 7 years, 2 months ago
On 08/06/2018 01:59 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim <yongbok.kim@mips.com>
> 
> MFHC0 and MTHC0 used to handle EntryLo0 and EntryLo1 registers only,
> and placing ELPA flag checks before switch statement were technically
> correct. However, after adding handling more registers, these checks
> should be moved to act only in cases of handling EntryLo0 and
> EntryLo1.
> 
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Yongbok Kim <yongbok.kim@mips.com>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/mips/translate.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 841c0c8..bc1f21f 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -4896,12 +4896,11 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>  {
>      const char *rn = "invalid";
>  
> -    CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> -
>      switch (reg) {
>      case 2:
>          switch (sel) {
>          case 0:
> +            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
>              gen_mfhc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo0));
>              rn = "EntryLo0";
>              break;
> @@ -4912,6 +4911,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>      case 3:
>          switch (sel) {
>          case 0:
> +            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
>              gen_mfhc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo1));
>              rn = "EntryLo1";
>              break;
> @@ -4964,12 +4964,11 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>      const char *rn = "invalid";
>      uint64_t mask = ctx->PAMask >> 36;
>  
> -    CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
> -
>      switch (reg) {
>      case 2:
>          switch (sel) {
>          case 0:
> +            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
>              tcg_gen_andi_tl(arg, arg, mask);
>              gen_mthc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo0));
>              rn = "EntryLo0";
> @@ -4981,6 +4980,7 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>      case 3:
>          switch (sel) {
>          case 0:
> +            CP0_CHECK(ctx->hflags & MIPS_HFLAG_ELPA);
>              tcg_gen_andi_tl(arg, arg, mask);
>              gen_mthc0_entrylo(arg, offsetof(CPUMIPSState, CP0_EntryLo1));
>              rn = "EntryLo1";
>