Re: [PATCH v2 3/7] target/ppc: Isolated SPR read/write callbacks

Fri, 30 Apr 2021 06:07:58 -0700 

On 30/04/2021 01:21, David Gibson wrote:

On Thu, Apr 29, 2021 at 01:21:26PM -0300, Bruno Larsen (billionai) wrote:

Moved all SPR read/write callback, and some related functions, to a
new file specific for it. These callbacks are TCG only, so separating
them is required to support the flag disable-tcg.

Making the spr_noaccess function not static, and moving the define to
internal.h is required, as spr_tcg.c.inc has to be included after
SPR_NOACCESS has been used.

Signed-off-by: Bruno Larsen (billionai) <bruno.lar...@eldorado.org.br>
---
  target/ppc/internal.h           |    3 +
  target/ppc/spr_tcg.c.inc        | 1033 +++++++++++++++++++++++++++++++
  target/ppc/translate.c          |   49 +-
  target/ppc/translate_init.c.inc |  981 -----------------------------
  4 files changed, 1039 insertions(+), 1027 deletions(-)
  create mode 100644 target/ppc/spr_tcg.c.inc

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 184ba6d6b3..1de15acfbd 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -228,4 +228,7 @@ void destroy_ppc_opcodes(PowerPCCPU *cpu);
  void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *ppc);
  gchar *ppc_gdb_arch_name(CPUState *cs);
+void spr_noaccess(DisasContext *ctx, int gprn, int sprn); 
+#define SPR_NOACCESS (&spr_noaccess)
+
  #endif /* PPC_INTERNAL_H */
diff --git a/target/ppc/spr_tcg.c.inc b/target/ppc/spr_tcg.c.inc
new file mode 100644
index 0000000000..48274dd52b
--- /dev/null
+++ b/target/ppc/spr_tcg.c.inc

I'm confused.  You create the new file as a .inc.c, but I don't see
anything #including the new file.

<snip>

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b319d409c6..a6e677fa6d 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -36,6 +36,7 @@
  #include "exec/translator.h"
  #include "exec/log.h"
  #include "qemu/atomic128.h"
+#include "internal.h"
#define CPU_SINGLE_STEP 0x1 
@@ -4175,43 +4176,6 @@ static void gen_tdi(DisasContext *ctx)
/*** Processor control ***/ -static void gen_read_xer(DisasContext *ctx, TCGv dst) 
-{
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-    TCGv t2 = tcg_temp_new();
-    tcg_gen_mov_tl(dst, cpu_xer);
-    tcg_gen_shli_tl(t0, cpu_so, XER_SO);
-    tcg_gen_shli_tl(t1, cpu_ov, XER_OV);
-    tcg_gen_shli_tl(t2, cpu_ca, XER_CA);
-    tcg_gen_or_tl(t0, t0, t1);
-    tcg_gen_or_tl(dst, dst, t2);
-    tcg_gen_or_tl(dst, dst, t0);
-    if (is_isa300(ctx)) {
-        tcg_gen_shli_tl(t0, cpu_ov32, XER_OV32);
-        tcg_gen_or_tl(dst, dst, t0);
-        tcg_gen_shli_tl(t0, cpu_ca32, XER_CA32);
-        tcg_gen_or_tl(dst, dst, t0);
-    }
-    tcg_temp_free(t0);
-    tcg_temp_free(t1);
-    tcg_temp_free(t2);
-}
-
-static void gen_write_xer(TCGv src)
-{
-    /* Write all flags, while reading back check for isa300 */
-    tcg_gen_andi_tl(cpu_xer, src,
-                    ~((1u << XER_SO) |
-                      (1u << XER_OV) | (1u << XER_OV32) |
-                      (1u << XER_CA) | (1u << XER_CA32)));
-    tcg_gen_extract_tl(cpu_ov32, src, XER_OV32, 1);
-    tcg_gen_extract_tl(cpu_ca32, src, XER_CA32, 1);
-    tcg_gen_extract_tl(cpu_so, src, XER_SO, 1);
-    tcg_gen_extract_tl(cpu_ov, src, XER_OV, 1);
-    tcg_gen_extract_tl(cpu_ca, src, XER_CA, 1);
-}
-
  /* mcrxr */
  static void gen_mcrxr(DisasContext *ctx)
  {
@@ -4299,15 +4263,6 @@ static void gen_mfmsr(DisasContext *ctx)
      tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_msr);
  }
-static void spr_noaccess(DisasContext *ctx, int gprn, int sprn) 
-{
-#if 0
-    sprn = ((sprn >> 5) & 0x1F) | ((sprn & 0x1F) << 5);
-    printf("ERROR: try to access SPR %d !\n", sprn);
-#endif
-}
-#define SPR_NOACCESS (&spr_noaccess)
-
  /* mfspr */
  static inline void gen_op_mfspr(DisasContext *ctx)
  {
@@ -7639,6 +7594,7 @@ GEN_HANDLER2_E(trechkpt, "trechkpt", 0x1F, 0x0E, 0x1F, 
0x03FFF800, \
  };
#include "helper_regs.h" 
+#include "spr_tcg.c.inc"
  #include "translate_init.c.inc"
You probably just missed it here. Buried a bit in all the code motion, unfortunately... 

--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> 
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <about:blank?compose>

Reply via email to