[10/11] target/arm: Use unallocated_encoding for aarch32

Message ID 20190807045335.1361-11-richard.henderson@linaro.org
State Superseded
Headers show
Series
  • target/arm: decodetree prep patches
Related show

Commit Message

Richard Henderson Aug. 7, 2019, 4:53 a.m.
Promote this function from aarch64 to fully general use.
Use it to unify the code sequences for generating illegal
opcode exceptions.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/translate-a64.h     |  2 --
 target/arm/translate.h         |  2 ++
 target/arm/translate-a64.c     |  7 -------
 target/arm/translate-vfp.inc.c |  3 +--
 target/arm/translate.c         | 22 ++++++++++++----------
 5 files changed, 15 insertions(+), 21 deletions(-)

-- 
2.17.1

Comments

Philippe Mathieu-Daudé Aug. 7, 2019, 5:35 p.m. | #1
On 8/7/19 6:53 AM, Richard Henderson wrote:
> Promote this function from aarch64 to fully general use.

> Use it to unify the code sequences for generating illegal

> opcode exceptions.

> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/translate-a64.h     |  2 --

>  target/arm/translate.h         |  2 ++

>  target/arm/translate-a64.c     |  7 -------

>  target/arm/translate-vfp.inc.c |  3 +--

>  target/arm/translate.c         | 22 ++++++++++++----------

>  5 files changed, 15 insertions(+), 21 deletions(-)

> 

> diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h

> index 9cd2b3d238..12ad8ac6ed 100644

> --- a/target/arm/translate-a64.h

> +++ b/target/arm/translate-a64.h

> @@ -18,8 +18,6 @@

>  #ifndef TARGET_ARM_TRANSLATE_A64_H

>  #define TARGET_ARM_TRANSLATE_A64_H

>  

> -void unallocated_encoding(DisasContext *s);

> -

>  #define unsupported_encoding(s, insn)                                    \

>      do {                                                                 \

>          qemu_log_mask(LOG_UNIMP,                                         \

> diff --git a/target/arm/translate.h b/target/arm/translate.h

> index de600073d8..6a65df0b27 100644

> --- a/target/arm/translate.h

> +++ b/target/arm/translate.h

> @@ -98,6 +98,8 @@ typedef struct DisasCompare {

>      bool value_global;

>  } DisasCompare;

>  

> +void unallocated_encoding(DisasContext *s);

> +

>  /* Share the TCG temporaries common between 32 and 64 bit modes.  */

>  extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;

>  extern TCGv_i64 cpu_exclusive_addr;

> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

> index d68bfc66d3..9e1ffe9cfb 100644

> --- a/target/arm/translate-a64.c

> +++ b/target/arm/translate-a64.c

> @@ -352,13 +352,6 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)

>      }

>  }

>  

> -void unallocated_encoding(DisasContext *s)

> -{

> -    /* Unallocated and reserved encodings are uncategorized */

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> -}

> -

>  static void init_tmp_a64_array(DisasContext *s)

>  {

>  #ifdef CONFIG_DEBUG_TCG

> diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c

> index 5065d4524c..3e8ea80493 100644

> --- a/target/arm/translate-vfp.inc.c

> +++ b/target/arm/translate-vfp.inc.c

> @@ -108,8 +108,7 @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled)

>  

>      if (!s->vfp_enabled && !ignore_vfp_enabled) {

>          assert(!arm_dc_feature(s, ARM_FEATURE_M));

> -        gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                           default_exception_el(s));

> +        unallocated_encoding(s);

>          return false;

>      }

>  

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

> index d6b0ab7247..2d447d4b90 100644

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

> +++ b/target/arm/translate.c

> @@ -1285,6 +1285,13 @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)

>      s->base.is_jmp = DISAS_NORETURN;

>  }

>  

> +void unallocated_encoding(DisasContext *s)

> +{

> +    /* Unallocated and reserved encodings are uncategorized */

> +    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> +                       default_exception_el(s));

> +}

> +

>  /* Force a TB lookup after an instruction that changes the CPU state.  */

>  static inline void gen_lookup_tb(DisasContext *s)

>  {

> @@ -1315,8 +1322,7 @@ static inline void gen_hlt(DisasContext *s, int imm)

>          return;

>      }

>  

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> +    unallocated_encoding(s);

>  }

>  

>  static inline void gen_add_data_offset(DisasContext *s, unsigned int insn,

> @@ -7638,8 +7644,7 @@ static void gen_srs(DisasContext *s,

>      }

>  

>      if (undef) {

> -        gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                           default_exception_el(s));

> +        unallocated_encoding(s);

>          return;

>      }

>  

> @@ -9266,8 +9271,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)

>              break;

>          default:

>          illegal_op:

> -            gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                               default_exception_el(s));

> +            unallocated_encoding(s);

>              break;

>          }

>      }

> @@ -10955,8 +10959,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)

>      }

>      return;

>  illegal_op:

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> +    unallocated_encoding(s);

>  }

>  

>  static void disas_thumb_insn(DisasContext *s, uint32_t insn)

> @@ -11779,8 +11782,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)

>      return;

>  illegal_op:

>  undef:

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> +    unallocated_encoding(s);

>  }

>  

>  static bool insn_crosses_page(CPUARMState *env, DisasContext *s)

> 


I wanted to suggest naming it gen_unallocated_encoding_exception() but
it is already used ~250 times, so no :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Laurent Desnogues Aug. 26, 2019, 8:45 a.m. | #2
Hi Richard,

On Wed, Aug 7, 2019 at 6:54 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> Promote this function from aarch64 to fully general use.

> Use it to unify the code sequences for generating illegal

> opcode exceptions.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/translate-a64.h     |  2 --

>  target/arm/translate.h         |  2 ++

>  target/arm/translate-a64.c     |  7 -------

>  target/arm/translate-vfp.inc.c |  3 +--

>  target/arm/translate.c         | 22 ++++++++++++----------

>  5 files changed, 15 insertions(+), 21 deletions(-)

>

> diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h

> index 9cd2b3d238..12ad8ac6ed 100644

> --- a/target/arm/translate-a64.h

> +++ b/target/arm/translate-a64.h

> @@ -18,8 +18,6 @@

>  #ifndef TARGET_ARM_TRANSLATE_A64_H

>  #define TARGET_ARM_TRANSLATE_A64_H

>

> -void unallocated_encoding(DisasContext *s);

> -

>  #define unsupported_encoding(s, insn)                                    \

>      do {                                                                 \

>          qemu_log_mask(LOG_UNIMP,                                         \

> diff --git a/target/arm/translate.h b/target/arm/translate.h

> index de600073d8..6a65df0b27 100644

> --- a/target/arm/translate.h

> +++ b/target/arm/translate.h

> @@ -98,6 +98,8 @@ typedef struct DisasCompare {

>      bool value_global;

>  } DisasCompare;

>

> +void unallocated_encoding(DisasContext *s);

> +

>  /* Share the TCG temporaries common between 32 and 64 bit modes.  */

>  extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;

>  extern TCGv_i64 cpu_exclusive_addr;

> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

> index d68bfc66d3..9e1ffe9cfb 100644

> --- a/target/arm/translate-a64.c

> +++ b/target/arm/translate-a64.c

> @@ -352,13 +352,6 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)

>      }

>  }

>

> -void unallocated_encoding(DisasContext *s)

> -{

> -    /* Unallocated and reserved encodings are uncategorized */

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> -}

> -

>  static void init_tmp_a64_array(DisasContext *s)

>  {

>  #ifdef CONFIG_DEBUG_TCG

> diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c

> index 5065d4524c..3e8ea80493 100644

> --- a/target/arm/translate-vfp.inc.c

> +++ b/target/arm/translate-vfp.inc.c

> @@ -108,8 +108,7 @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled)

>

>      if (!s->vfp_enabled && !ignore_vfp_enabled) {

>          assert(!arm_dc_feature(s, ARM_FEATURE_M));

> -        gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                           default_exception_el(s));

> +        unallocated_encoding(s);

>          return false;

>      }

>

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

> index d6b0ab7247..2d447d4b90 100644

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

> +++ b/target/arm/translate.c

> @@ -1285,6 +1285,13 @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)

>      s->base.is_jmp = DISAS_NORETURN;

>  }

>

> +void unallocated_encoding(DisasContext *s)

> +{

> +    /* Unallocated and reserved encodings are uncategorized */

> +    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> +                       default_exception_el(s));

> +}


Isn't the move of unallocated_encoding from translate-64.c to
translate.c causing issues since gen_exception_insn isn't the same?
In particular in one case the pc is 64-bit while now it's always
32-bit.  Did I miss something?

Thanks,

Laurent

> +

>  /* Force a TB lookup after an instruction that changes the CPU state.  */

>  static inline void gen_lookup_tb(DisasContext *s)

>  {

> @@ -1315,8 +1322,7 @@ static inline void gen_hlt(DisasContext *s, int imm)

>          return;

>      }

>

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> +    unallocated_encoding(s);

>  }

>

>  static inline void gen_add_data_offset(DisasContext *s, unsigned int insn,

> @@ -7638,8 +7644,7 @@ static void gen_srs(DisasContext *s,

>      }

>

>      if (undef) {

> -        gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                           default_exception_el(s));

> +        unallocated_encoding(s);

>          return;

>      }

>

> @@ -9266,8 +9271,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)

>              break;

>          default:

>          illegal_op:

> -            gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                               default_exception_el(s));

> +            unallocated_encoding(s);

>              break;

>          }

>      }

> @@ -10955,8 +10959,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)

>      }

>      return;

>  illegal_op:

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> +    unallocated_encoding(s);

>  }

>

>  static void disas_thumb_insn(DisasContext *s, uint32_t insn)

> @@ -11779,8 +11782,7 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)

>      return;

>  illegal_op:

>  undef:

> -    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

> -                       default_exception_el(s));

> +    unallocated_encoding(s);

>  }

>

>  static bool insn_crosses_page(CPUARMState *env, DisasContext *s)

> --

> 2.17.1

>

>

Patch

diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
index 9cd2b3d238..12ad8ac6ed 100644
--- a/target/arm/translate-a64.h
+++ b/target/arm/translate-a64.h
@@ -18,8 +18,6 @@ 
 #ifndef TARGET_ARM_TRANSLATE_A64_H
 #define TARGET_ARM_TRANSLATE_A64_H
 
-void unallocated_encoding(DisasContext *s);
-
 #define unsupported_encoding(s, insn)                                    \
     do {                                                                 \
         qemu_log_mask(LOG_UNIMP,                                         \
diff --git a/target/arm/translate.h b/target/arm/translate.h
index de600073d8..6a65df0b27 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -98,6 +98,8 @@  typedef struct DisasCompare {
     bool value_global;
 } DisasCompare;
 
+void unallocated_encoding(DisasContext *s);
+
 /* Share the TCG temporaries common between 32 and 64 bit modes.  */
 extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 extern TCGv_i64 cpu_exclusive_addr;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index d68bfc66d3..9e1ffe9cfb 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -352,13 +352,6 @@  static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
     }
 }
 
-void unallocated_encoding(DisasContext *s)
-{
-    /* Unallocated and reserved encodings are uncategorized */
-    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                       default_exception_el(s));
-}
-
 static void init_tmp_a64_array(DisasContext *s)
 {
 #ifdef CONFIG_DEBUG_TCG
diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index 5065d4524c..3e8ea80493 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -108,8 +108,7 @@  static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled)
 
     if (!s->vfp_enabled && !ignore_vfp_enabled) {
         assert(!arm_dc_feature(s, ARM_FEATURE_M));
-        gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                           default_exception_el(s));
+        unallocated_encoding(s);
         return false;
     }
 
diff --git a/target/arm/translate.c b/target/arm/translate.c
index d6b0ab7247..2d447d4b90 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1285,6 +1285,13 @@  static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
     s->base.is_jmp = DISAS_NORETURN;
 }
 
+void unallocated_encoding(DisasContext *s)
+{
+    /* Unallocated and reserved encodings are uncategorized */
+    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
+                       default_exception_el(s));
+}
+
 /* Force a TB lookup after an instruction that changes the CPU state.  */
 static inline void gen_lookup_tb(DisasContext *s)
 {
@@ -1315,8 +1322,7 @@  static inline void gen_hlt(DisasContext *s, int imm)
         return;
     }
 
-    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                       default_exception_el(s));
+    unallocated_encoding(s);
 }
 
 static inline void gen_add_data_offset(DisasContext *s, unsigned int insn,
@@ -7638,8 +7644,7 @@  static void gen_srs(DisasContext *s,
     }
 
     if (undef) {
-        gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                           default_exception_el(s));
+        unallocated_encoding(s);
         return;
     }
 
@@ -9266,8 +9271,7 @@  static void disas_arm_insn(DisasContext *s, unsigned int insn)
             break;
         default:
         illegal_op:
-            gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                               default_exception_el(s));
+            unallocated_encoding(s);
             break;
         }
     }
@@ -10955,8 +10959,7 @@  static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
     }
     return;
 illegal_op:
-    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                       default_exception_el(s));
+    unallocated_encoding(s);
 }
 
 static void disas_thumb_insn(DisasContext *s, uint32_t insn)
@@ -11779,8 +11782,7 @@  static void disas_thumb_insn(DisasContext *s, uint32_t insn)
     return;
 illegal_op:
 undef:
-    gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-                       default_exception_el(s));
+    unallocated_encoding(s);
 }
 
 static bool insn_crosses_page(CPUARMState *env, DisasContext *s)