[PATCH 0/8] ppc/ppc405: Fixes

[PATCH 0/8] ppc/ppc405: Fixes

[Cédric Le Goater]

Hello,

The series starts with a couple of cleanups helpping debug. It then
adds back support for 405 timers which was broken 10 years ago.

Thanks,

C. 

Cédric Le Goater (8):
  target/ppc: Print out literal exception names in logs
  ppc/ppc4xx: Convert printfs()
  ppc/ppc405: Activate MMU logs
  ppc/ppc405: Restore TCR and STR write handlers
  ppc/ppc405: Rework ppc_40x_timers_init() to use a PowerPCCPU
  ppc/ppc405: Fix timer initialization
  ppc/ppc405: Introduce a store helper for SPR_40x_PID
  ppc/ppc405: Dump specific registers

 target/ppc/cpu.h             |  2 +
 target/ppc/helper.h          |  2 +
 target/ppc/spr_tcg.h         |  3 ++
 hw/ppc/mpc8544_guts.c        |  9 +++--
 hw/ppc/ppc.c                 | 67 +++++++++++++++++++++-----------
 hw/ppc/ppc405_uc.c           |  2 -
 hw/ppc/ppc4xx_devs.c         | 39 ++++++-------------
 hw/ppc/ppc4xx_pci.c          | 11 ++++--
 target/ppc/cpu_init.c        | 17 ++++++--
 target/ppc/excp_helper.c     | 75 +++++++++++++++++++++++++++++++++++-
 target/ppc/mmu_common.c      |  4 +-
 target/ppc/mmu_helper.c      |  2 +-
 target/ppc/timebase_helper.c | 10 +++++
 target/ppc/translate.c       | 20 ++++++++++
 hw/ppc/trace-events          |  7 ++++
 15 files changed, 203 insertions(+), 67 deletions(-)

-- 
2.31.1

[PATCH 1/8] target/ppc: Print out literal exception names in logs

[Cédric Le Goater]

It facilitates reading the logs when mask CPU_LOG_INT is activated. We
should do the same for error codes.

Cc: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/excp_helper.c | 75 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index f90e616aacda..71ad983e67d6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -32,6 +32,78 @@
 
 /* #define DEBUG_SOFTWARE_TLB */
 
+static inline const char *powerpc_excp_name(int excp)
+{
+    switch (excp) {
+    case POWERPC_EXCP_CRITICAL: return "CRITICAL";
+    case POWERPC_EXCP_MCHECK:   return "MCHECK";
+    case POWERPC_EXCP_DSI:      return "DSI";
+    case POWERPC_EXCP_ISI:      return "ISI";
+    case POWERPC_EXCP_EXTERNAL: return "EXTERNAL";
+    case POWERPC_EXCP_ALIGN:    return "ALIGN";
+    case POWERPC_EXCP_PROGRAM:  return "PROGRAM";
+    case POWERPC_EXCP_FPU:      return "FPU";
+    case POWERPC_EXCP_SYSCALL:  return "SYSCALL";
+    case POWERPC_EXCP_APU:      return "APU";
+    case POWERPC_EXCP_DECR:     return "DECR";
+    case POWERPC_EXCP_FIT:      return "FIT";
+    case POWERPC_EXCP_WDT:      return "WDT";
+    case POWERPC_EXCP_DTLB:     return "DTLB";
+    case POWERPC_EXCP_ITLB:     return "ITLB";
+    case POWERPC_EXCP_DEBUG:    return "DEBUG";
+    case POWERPC_EXCP_SPEU:     return "SPEU";
+    case POWERPC_EXCP_EFPDI:    return "EFPDI";
+    case POWERPC_EXCP_EFPRI:    return "EFPRI";
+    case POWERPC_EXCP_EPERFM:   return "EPERFM";
+    case POWERPC_EXCP_DOORI:    return "DOORI";
+    case POWERPC_EXCP_DOORCI:   return "DOORCI";
+    case POWERPC_EXCP_GDOORI:   return "GDOORI";
+    case POWERPC_EXCP_GDOORCI:  return "GDOORCI";
+    case POWERPC_EXCP_HYPPRIV:  return "HYPPRIV";
+    case POWERPC_EXCP_RESET:    return "RESET";
+    case POWERPC_EXCP_DSEG:     return "DSEG";
+    case POWERPC_EXCP_ISEG:     return "ISEG";
+    case POWERPC_EXCP_HDECR:    return "HDECR";
+    case POWERPC_EXCP_TRACE:    return "TRACE";
+    case POWERPC_EXCP_HDSI:     return "HDSI";
+    case POWERPC_EXCP_HISI:     return "HISI";
+    case POWERPC_EXCP_HDSEG:    return "HDSEG";
+    case POWERPC_EXCP_HISEG:    return "HISEG";
+    case POWERPC_EXCP_VPU:      return "VPU";
+    case POWERPC_EXCP_PIT:      return "PIT";
+    case POWERPC_EXCP_IO:       return "IO";
+    case POWERPC_EXCP_RUNM:     return "RUNM";
+    case POWERPC_EXCP_EMUL:     return "EMUL";
+    case POWERPC_EXCP_IFTLB:    return "IFTLB";
+    case POWERPC_EXCP_DLTLB:    return "DLTLB";
+    case POWERPC_EXCP_DSTLB:    return "DSTLB";
+    case POWERPC_EXCP_FPA:      return "FPA";
+    case POWERPC_EXCP_DABR:     return "DABR";
+    case POWERPC_EXCP_IABR:     return "IABR";
+    case POWERPC_EXCP_SMI:      return "SMI";
+    case POWERPC_EXCP_PERFM:    return "PERFM";
+    case POWERPC_EXCP_THERM:    return "THERM";
+    case POWERPC_EXCP_VPUA:     return "VPUA";
+    case POWERPC_EXCP_SOFTP:    return "SOFTP";
+    case POWERPC_EXCP_MAINT:    return "MAINT";
+    case POWERPC_EXCP_MEXTBR:   return "MEXTBR";
+    case POWERPC_EXCP_NMEXTBR:  return "NMEXTBR";
+    case POWERPC_EXCP_ITLBE:    return "ITLBE";
+    case POWERPC_EXCP_DTLBE:    return "DTLBE";
+    case POWERPC_EXCP_VSXU:     return "VSXU";
+    case POWERPC_EXCP_FU:       return "FU";
+    case POWERPC_EXCP_HV_EMU:   return "HV_EMU";
+    case POWERPC_EXCP_HV_MAINT: return "HV_MAINT";
+    case POWERPC_EXCP_HV_FU:    return "HV_FU";
+    case POWERPC_EXCP_SDOOR:    return "SDOOR";
+    case POWERPC_EXCP_SDOOR_HV: return "SDOOR_HV";
+    case POWERPC_EXCP_HVIRT:    return "HVIRT";
+    case POWERPC_EXCP_SYSCALL_VECTORED: return "SYSCALL_VECTORED";
+    default:
+        g_assert_not_reached();
+    }
+}
+
 /*****************************************************************************/
 /* Exception processing */
 #if !defined(CONFIG_USER_ONLY)
@@ -301,7 +373,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     int srr0, srr1, asrr0, asrr1, lev = -1;
 
     qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
-                  " => %08x (%02x)\n", env->nip, excp, env->error_code);
+                  " => %s (%d) error=%02x\n", env->nip, powerpc_excp_name(excp),
+                  excp, env->error_code);
 
     /* new srr1 value excluding must-be-zero bits */
     if (excp_model == POWERPC_EXCP_BOOKE) {
-- 
2.31.1

[PATCH 2/8] ppc/ppc4xx: Convert printfs()

[Cédric Le Goater]

Use a QEMU log primitive for errors and trace events for debug.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/mpc8544_guts.c |  9 ++++++---
 hw/ppc/ppc4xx_devs.c  | 39 +++++++++++----------------------------
 hw/ppc/ppc4xx_pci.c   | 11 +++++++----
 hw/ppc/trace-events   |  5 +++++
 4 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
index e8d2d51c20c0..a26e83d0484b 100644
--- a/hw/ppc/mpc8544_guts.c
+++ b/hw/ppc/mpc8544_guts.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu/log.h"
 #include "sysemu/runstate.h"
 #include "cpu.h"
 #include "hw/sysbus.h"
@@ -82,7 +83,9 @@ static uint64_t mpc8544_guts_read(void *opaque, hwaddr addr,
         value = env->spr[SPR_E500_SVR];
         break;
     default:
-        fprintf(stderr, "guts: Unknown register read: %x\n", (int)addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Unknown register 0x%" HWADDR_PRIx "\n",
+                      __func__, addr);
         break;
     }
 
@@ -101,8 +104,8 @@ static void mpc8544_guts_write(void *opaque, hwaddr addr,
         }
         break;
     default:
-        fprintf(stderr, "guts: Unknown register write: %x = %x\n",
-                (int)addr, (unsigned)value);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown register 0x%" HWADDR_PRIx
+                       " = 0x%" PRIx64 "\n", __func__, addr, value);
         break;
     }
 }
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index 980c48944fc7..e7d82ae5016c 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -35,14 +35,7 @@
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
-
-/*#define DEBUG_UIC*/
-
-#ifdef DEBUG_UIC
-#  define LOG_UIC(...) qemu_log_mask(CPU_LOG_INT, ## __VA_ARGS__)
-#else
-#  define LOG_UIC(...) do { } while (0)
-#endif
+#include "trace.h"
 
 static void ppc4xx_reset(void *opaque)
 {
@@ -137,8 +130,9 @@ static uint32_t sdram_bcr (hwaddr ram_base,
         bcr = 0x000C0000;
         break;
     default:
-        printf("%s: invalid RAM size " TARGET_FMT_plx "\n", __func__,
-               ram_size);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid RAM size 0x%" HWADDR_PRIx "\n", __func__,
+                      ram_size);
         return 0x00000000;
     }
     bcr |= ram_base & 0xFF800000;
@@ -171,10 +165,8 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram, int i,
 {
     if (sdram->bcr[i] & 0x00000001) {
         /* Unmap RAM */
-#ifdef DEBUG_SDRAM
-        printf("%s: unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
-               __func__, sdram_base(sdram->bcr[i]), sdram_size(sdram->bcr[i]));
-#endif
+        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bcr[i]),
+                                 sdram_size(sdram->bcr[i]));
         memory_region_del_subregion(get_system_memory(),
                                     &sdram->containers[i]);
         memory_region_del_subregion(&sdram->containers[i],
@@ -183,10 +175,7 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram, int i,
     }
     sdram->bcr[i] = bcr & 0xFFDEE001;
     if (enabled && (bcr & 0x00000001)) {
-#ifdef DEBUG_SDRAM
-        printf("%s: Map RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
-               __func__, sdram_base(bcr), sdram_size(bcr));
-#endif
+        trace_ppc4xx_sdram_unmap(sdram_base(bcr), sdram_size(bcr));
         memory_region_init(&sdram->containers[i], NULL, "sdram-containers",
                            sdram_size(bcr));
         memory_region_add_subregion(&sdram->containers[i], 0,
@@ -216,10 +205,8 @@ static void sdram_unmap_bcr (ppc4xx_sdram_t *sdram)
     int i;
 
     for (i = 0; i < sdram->nbanks; i++) {
-#ifdef DEBUG_SDRAM
-        printf("%s: Unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
-               __func__, sdram_base(sdram->bcr[i]), sdram_size(sdram->bcr[i]));
-#endif
+        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bcr[i]),
+                                 sdram_size(sdram->bcr[i]));
         memory_region_del_subregion(get_system_memory(),
                                     &sdram->ram_memories[i]);
     }
@@ -316,16 +303,12 @@ static void dcr_write_sdram (void *opaque, int dcrn, uint32_t val)
         case 0x20: /* SDRAM_CFG */
             val &= 0xFFE00000;
             if (!(sdram->cfg & 0x80000000) && (val & 0x80000000)) {
-#ifdef DEBUG_SDRAM
-                printf("%s: enable SDRAM controller\n", __func__);
-#endif
+                trace_ppc4xx_sdram_enable("enable");
                 /* validate all RAM mappings */
                 sdram_map_bcr(sdram);
                 sdram->status &= ~0x80000000;
             } else if ((sdram->cfg & 0x80000000) && !(val & 0x80000000)) {
-#ifdef DEBUG_SDRAM
-                printf("%s: disable SDRAM controller\n", __func__);
-#endif
+                trace_ppc4xx_sdram_enable("disable");
                 /* invalidate all RAM mappings */
                 sdram_unmap_bcr(sdram);
                 sdram->status |= 0x80000000;
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 304a29349c2e..5df97e6d156f 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -20,6 +20,7 @@
  * 4xx SoCs, such as the 440EP. */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/irq.h"
 #include "hw/ppc/ppc.h"
 #include "hw/ppc/ppc4xx.h"
@@ -152,8 +153,9 @@ static void ppc4xx_pci_reg_write4(void *opaque, hwaddr offset,
         break;
 
     default:
-        printf("%s: unhandled PCI internal register 0x%lx\n", __func__,
-               (unsigned long)offset);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                     "%s: unhandled PCI internal register 0x%" HWADDR_PRIx "\n",
+                     __func__, offset);
         break;
     }
 }
@@ -218,8 +220,9 @@ static uint64_t ppc4xx_pci_reg_read4(void *opaque, hwaddr offset,
         break;
 
     default:
-        printf("%s: invalid PCI internal register 0x%lx\n", __func__,
-               (unsigned long)offset);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid PCI internal register 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
         value = 0;
     }
 
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index ada644652d94..0c55aa501471 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -164,3 +164,8 @@ ppc4xx_gpt_init(uint64_t addr) "offet 0x%" PRIx64
 
 ppc405ep_clocks_compute(const char *param, uint32_t param2, uint32_t val) "%s 0x%1" PRIx32 " %d"
 ppc405ep_clocks_setup(const char *trace) "%s"
+
+# ppc4xx_devs.c
+ppc4xx_sdram_enable(const char *trace) "%s SDRAM controller"
+ppc4xx_sdram_unmap(uint64_t addr, uint64_t size) "Unmap RAM area 0x%" PRIx64 " size 0x%" PRIx64
+ppc4xx_sdram_map(uint64_t addr, uint64_t size) "Map RAM area 0x%" PRIx64 " size 0x%" PRIx64
-- 
2.31.1

[PATCH 3/8] ppc/ppc405: Activate MMU logs

[Cédric Le Goater]

There is no need to deactivate MMU logging at compile time.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/mmu_common.c | 4 ++--
 target/ppc/mmu_helper.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 4e278365ca55..c0d3d32fafaa 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -34,9 +34,9 @@
 #include "mmu-book3s-v3.h"
 #include "mmu-radix64.h"
 
-/* #define DEBUG_MMU */
+#define DEBUG_MMU
 /* #define DEBUG_BATS */
-/* #define DEBUG_SOFTWARE_TLB */
+#define DEBUG_SOFTWARE_TLB
 /* #define DUMP_PAGE_TABLES */
 /* #define FLUSH_ALL_TLBS */
 
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 2ec3d203a081..6788577f449f 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -37,7 +37,7 @@
 #include "exec/cpu_ldst.h"
 
 /* #define DEBUG_BATS */
-/* #define DEBUG_SOFTWARE_TLB */
+#define DEBUG_SOFTWARE_TLB
 /* #define DUMP_PAGE_TABLES */
 /* #define FLUSH_ALL_TLBS */
 
-- 
2.31.1

[PATCH 4/8] ppc/ppc405: Restore TCR and STR write handlers

[Cédric Le Goater]

The 405 timers were broken when booke support was added. Assumption
was made that the register numbers were the same but it's not :

    SPR_BOOKE_TSR         (0x150)
    SPR_BOOKE_TCR         (0x154)
    SPR_40x_TSR           (0x3D8)
    SPR_40x_TCR           (0x3DA)

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: ddd1055b07fd ("PPC: booke timers")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/cpu.h             |  2 ++
 target/ppc/helper.h          |  2 ++
 target/ppc/spr_tcg.h         |  2 ++
 hw/ppc/ppc.c                 | 25 +++++++++++++++++++++++++
 target/ppc/cpu_init.c        |  4 ++--
 target/ppc/timebase_helper.c | 10 ++++++++++
 target/ppc/translate.c       | 12 ++++++++++++
 hw/ppc/trace-events          |  2 ++
 8 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index fc66c3561dab..4808e10ebe8b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1399,6 +1399,8 @@ target_ulong load_40x_pit(CPUPPCState *env);
 void store_40x_pit(CPUPPCState *env, target_ulong val);
 void store_40x_dbcr0(CPUPPCState *env, uint32_t val);
 void store_40x_sler(CPUPPCState *env, uint32_t val);
+void store_40x_tcr(CPUPPCState *env, target_ulong val);
+void store_40x_tsr(CPUPPCState *env, target_ulong val);
 void store_booke_tcr(CPUPPCState *env, target_ulong val);
 void store_booke_tsr(CPUPPCState *env, target_ulong val);
 void ppc_tlb_invalidate_all(CPUPPCState *env);
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index fb6cac38b4c5..f9c72dcd504d 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -706,6 +706,8 @@ DEF_HELPER_2(store_hid0_601, void, env, tl)
 DEF_HELPER_3(store_403_pbr, void, env, i32, tl)
 DEF_HELPER_FLAGS_1(load_40x_pit, TCG_CALL_NO_RWG, tl, env)
 DEF_HELPER_FLAGS_2(store_40x_pit, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_40x_tcr, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(store_40x_tsr, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
 DEF_HELPER_FLAGS_2(store_booke_tcr, TCG_CALL_NO_RWG, void, env, tl)
diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
index f98d97c0ba17..64cf5302cb86 100644
--- a/target/ppc/spr_tcg.h
+++ b/target/ppc/spr_tcg.h
@@ -87,6 +87,8 @@ void spr_read_40x_pit(DisasContext *ctx, int gprn, int sprn);
 void spr_write_40x_pit(DisasContext *ctx, int sprn, int gprn);
 void spr_write_40x_dbcr0(DisasContext *ctx, int sprn, int gprn);
 void spr_write_40x_sler(DisasContext *ctx, int sprn, int gprn);
+void spr_write_40x_tcr(DisasContext *ctx, int sprn, int gprn);
+void spr_write_40x_tsr(DisasContext *ctx, int sprn, int gprn);
 void spr_write_booke_tcr(DisasContext *ctx, int sprn, int gprn);
 void spr_write_booke_tsr(DisasContext *ctx, int sprn, int gprn);
 void spr_read_403_pbr(DisasContext *ctx, int gprn, int sprn);
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 818d75798584..cca99cb86f81 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1300,6 +1300,31 @@ target_ulong load_40x_pit (CPUPPCState *env)
     return cpu_ppc_load_decr(env);
 }
 
+void store_40x_tsr(CPUPPCState *env, target_ulong val)
+{
+    PowerPCCPU *cpu = env_archcpu(env);
+
+    trace_ppc40x_store_tcr(val);
+
+    env->spr[SPR_40x_TSR] &= ~(val & 0xFC000000);
+    if (val & 0x80000000) {
+        ppc_set_irq(cpu, PPC_INTERRUPT_PIT, 0);
+    }
+}
+
+void store_40x_tcr(CPUPPCState *env, target_ulong val)
+{
+    PowerPCCPU *cpu = env_archcpu(env);
+    ppc_tb_t *tb_env;
+
+    trace_ppc40x_store_tsr(val);
+
+    tb_env = env->tb_env;
+    env->spr[SPR_40x_TCR] = val & 0xFFC00000;
+    start_stop_pit(env, tb_env, 1);
+    cpu_4xx_wdt_cb(cpu);
+}
+
 static void ppc_40x_set_tb_clk (void *opaque, uint32_t freq)
 {
     CPUPPCState *env = opaque;
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 06ef15cd9e4e..b5e2fde9ec4d 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1440,11 +1440,11 @@ static void register_40x_sprs(CPUPPCState *env)
                  0x00000000);
     spr_register(env, SPR_40x_TCR, "TCR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_booke_tcr,
+                 &spr_read_generic, &spr_write_40x_tcr,
                  0x00000000);
     spr_register(env, SPR_40x_TSR, "TSR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_booke_tsr,
+                 &spr_read_generic, &spr_write_40x_tsr,
                  0x00000000);
 }
 
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 8ff4080eb91e..af378318c19c 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -144,6 +144,16 @@ void helper_store_40x_pit(CPUPPCState *env, target_ulong val)
     store_40x_pit(env, val);
 }
 
+void helper_store_40x_tcr(CPUPPCState *env, target_ulong val)
+{
+    store_40x_tcr(env, val);
+}
+
+void helper_store_40x_tsr(CPUPPCState *env, target_ulong val)
+{
+    store_40x_tsr(env, val);
+}
+
 void helper_store_booke_tcr(CPUPPCState *env, target_ulong val)
 {
     store_booke_tcr(env, val);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 114456148c4b..eb45f679d34f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -878,6 +878,18 @@ void spr_write_40x_sler(DisasContext *ctx, int sprn, int gprn)
     gen_helper_store_40x_sler(cpu_env, cpu_gpr[gprn]);
 }
 
+void spr_write_40x_tcr(DisasContext *ctx, int sprn, int gprn)
+{
+    gen_icount_io_start(ctx);
+    gen_helper_store_40x_tcr(cpu_env, cpu_gpr[gprn]);
+}
+
+void spr_write_40x_tsr(DisasContext *ctx, int sprn, int gprn)
+{
+    gen_icount_io_start(ctx);
+    gen_helper_store_40x_tsr(cpu_env, cpu_gpr[gprn]);
+}
+
 void spr_write_booke_tcr(DisasContext *ctx, int sprn, int gprn)
 {
     gen_icount_io_start(ctx);
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 0c55aa501471..5c0a215cad90 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -110,6 +110,8 @@ ppc4xx_pit_start(uint64_t reload) "PIT 0x%016" PRIx64
 ppc4xx_pit(uint32_t ar, uint32_t ir, uint64_t tcr, uint64_t tsr, uint64_t reload) "ar %d ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64 " PIT 0x%016" PRIx64
 ppc4xx_wdt(uint64_t tcr, uint64_t tsr) "TCR 0x%" PRIx64 " TSR 0x%" PRIx64
 ppc40x_store_pit(uint64_t value) "val 0x%" PRIx64
+ppc40x_store_tcr(uint64_t value) "val 0x%" PRIx64
+ppc40x_store_tsr(uint64_t value) "val 0x%" PRIx64
 ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
 ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
 
-- 
2.31.1

[PATCH 5/8] ppc/ppc405: Rework ppc_40x_timers_init() to use a PowerPCCPU

[Cédric Le Goater]

This is a small cleanup to ease reading.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/ppc.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index cca99cb86f81..bb5bee9a3382 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1124,14 +1124,12 @@ struct ppc40x_timer_t {
 /* Fixed interval timer */
 static void cpu_4xx_fit_cb (void *opaque)
 {
-    PowerPCCPU *cpu;
-    CPUPPCState *env;
+    PowerPCCPU *cpu = opaque;
+    CPUPPCState *env = &cpu->env;
     ppc_tb_t *tb_env;
     ppc40x_timer_t *ppc40x_timer;
     uint64_t now, next;
 
-    env = opaque;
-    cpu = env_archcpu(env);
     tb_env = env->tb_env;
     ppc40x_timer = tb_env->opaque;
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
@@ -1193,13 +1191,11 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
 
 static void cpu_4xx_pit_cb (void *opaque)
 {
-    PowerPCCPU *cpu;
-    CPUPPCState *env;
+    PowerPCCPU *cpu = opaque;
+    CPUPPCState *env = &cpu->env;
     ppc_tb_t *tb_env;
     ppc40x_timer_t *ppc40x_timer;
 
-    env = opaque;
-    cpu = env_archcpu(env);
     tb_env = env->tb_env;
     ppc40x_timer = tb_env->opaque;
     env->spr[SPR_40x_TSR] |= 1 << 27;
@@ -1216,14 +1212,12 @@ static void cpu_4xx_pit_cb (void *opaque)
 /* Watchdog timer */
 static void cpu_4xx_wdt_cb (void *opaque)
 {
-    PowerPCCPU *cpu;
-    CPUPPCState *env;
+    PowerPCCPU *cpu = opaque;
+    CPUPPCState *env = &cpu->env;
     ppc_tb_t *tb_env;
     ppc40x_timer_t *ppc40x_timer;
     uint64_t now, next;
 
-    env = opaque;
-    cpu = env_archcpu(env);
     tb_env = env->tb_env;
     ppc40x_timer = tb_env->opaque;
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
@@ -1341,24 +1335,26 @@ clk_setup_cb ppc_40x_timers_init (CPUPPCState *env, uint32_t freq,
 {
     ppc_tb_t *tb_env;
     ppc40x_timer_t *ppc40x_timer;
+    PowerPCCPU *cpu = env_archcpu(env);
+
+    trace_ppc40x_timers_init(freq);
 
     tb_env = g_malloc0(sizeof(ppc_tb_t));
+    ppc40x_timer = g_malloc0(sizeof(ppc40x_timer_t));
+
     env->tb_env = tb_env;
     tb_env->flags = PPC_DECR_UNDERFLOW_TRIGGERED;
-    ppc40x_timer = g_malloc0(sizeof(ppc40x_timer_t));
     tb_env->tb_freq = freq;
     tb_env->decr_freq = freq;
     tb_env->opaque = ppc40x_timer;
-    trace_ppc40x_timers_init(freq);
-    if (ppc40x_timer != NULL) {
-        /* We use decr timer for PIT */
-        tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_pit_cb, env);
-        ppc40x_timer->fit_timer =
-            timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_fit_cb, env);
-        ppc40x_timer->wdt_timer =
-            timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_wdt_cb, env);
-        ppc40x_timer->decr_excp = decr_excp;
-    }
+
+    /* We use decr timer for PIT */
+    tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_pit_cb, cpu);
+    ppc40x_timer->fit_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_fit_cb, cpu);
+    ppc40x_timer->wdt_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_wdt_cb, cpu);
+    ppc40x_timer->decr_excp = decr_excp;
 
     return &ppc_40x_set_tb_clk;
 }
-- 
2.31.1

[PATCH 6/8] ppc/ppc405: Fix timer initialization

[Cédric Le Goater]

Timers are already initialized in ppc4xx_init(). No need to do it a
second time with a wrong set.

Fixes: d715ea961254 ("PPC: 405: Fix ppc405ep initialization")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/ppc405_uc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index ec97b22bd019..8aacd275a652 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1461,8 +1461,6 @@ PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem,
     ppc4xx_pob_init(env);
     /* OBP arbitrer */
     ppc4xx_opba_init(0xef600600);
-    /* Initialize timers */
-    ppc_booke_timers_init(cpu, sysclk, 0);
     /* Universal interrupt controller */
     uicdev = qdev_new(TYPE_PPC_UIC);
     uicsbd = SYS_BUS_DEVICE(uicdev);
-- 
2.31.1

[PATCH 7/8] ppc/ppc405: Introduce a store helper for SPR_40x_PID

[Cédric Le Goater]

The PID SPR of the 405 CPU contains the translation ID of the TLB
which is a 8-bit field. Enforce the mask with a store helper.

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/spr_tcg.h   | 1 +
 target/ppc/cpu_init.c  | 2 +-
 target/ppc/translate.c | 8 ++++++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
index 64cf5302cb86..89ff111724dc 100644
--- a/target/ppc/spr_tcg.h
+++ b/target/ppc/spr_tcg.h
@@ -89,6 +89,7 @@ void spr_write_40x_dbcr0(DisasContext *ctx, int sprn, int gprn);
 void spr_write_40x_sler(DisasContext *ctx, int sprn, int gprn);
 void spr_write_40x_tcr(DisasContext *ctx, int sprn, int gprn);
 void spr_write_40x_tsr(DisasContext *ctx, int sprn, int gprn);
+void spr_write_40x_pid(DisasContext *ctx, int sprn, int gprn);
 void spr_write_booke_tcr(DisasContext *ctx, int sprn, int gprn);
 void spr_write_booke_tsr(DisasContext *ctx, int sprn, int gprn);
 void spr_read_403_pbr(DisasContext *ctx, int gprn, int sprn);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index b5e2fde9ec4d..9ef9a1a5ddd5 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1454,7 +1454,7 @@ static void register_405_sprs(CPUPPCState *env)
     /* MMU */
     spr_register(env, SPR_40x_PID, "PID",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_40x_pid,
                  0x00000000);
     spr_register(env, SPR_4xx_CCR0, "CCR0",
                  SPR_NOACCESS, SPR_NOACCESS,
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index eb45f679d34f..cb8ab4d67635 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -890,6 +890,14 @@ void spr_write_40x_tsr(DisasContext *ctx, int sprn, int gprn)
     gen_helper_store_40x_tsr(cpu_env, cpu_gpr[gprn]);
 }
 
+void spr_write_40x_pid(DisasContext *ctx, int sprn, int gprn)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, cpu_gpr[gprn], 0xFF);
+    gen_store_spr(SPR_40x_PID, t0);
+    tcg_temp_free(t0);
+}
+
 void spr_write_booke_tcr(DisasContext *ctx, int sprn, int gprn)
 {
     gen_icount_io_start(ctx);
-- 
2.31.1

[PATCH 8/8] ppc/ppc405: Dump specific registers

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/cpu_init.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 9ef9a1a5ddd5..8f6a58e82483 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -8689,6 +8689,17 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
          * they can be read with "p $ivor0", "p $ivor1", etc.
          */
     }
+    if (env->excp_model == POWERPC_EXCP_40x) {
+        qemu_fprintf(f, "  TCR " TARGET_FMT_lx "   TSR " TARGET_FMT_lx
+                     "    ESR " TARGET_FMT_lx "   DEAR " TARGET_FMT_lx "\n",
+                     env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
+                     env->spr[SPR_40x_ESR], env->spr[SPR_40x_DEAR]);
+
+        qemu_fprintf(f, " EVPR " TARGET_FMT_lx "  SRR2 " TARGET_FMT_lx
+                     "   SRR3 " TARGET_FMT_lx  "   PID " TARGET_FMT_lx "\n",
+                     env->spr[SPR_40x_EVPR], env->spr[SPR_40x_SRR2],
+                     env->spr[SPR_40x_SRR3], env->spr[SPR_40x_PID]);
+    }
 
 #if defined(TARGET_PPC64)
     if (env->flags & POWERPC_FLAG_CFAR) {
-- 
2.31.1

Re: [PATCH 1/8] target/ppc: Print out literal exception names in logs

[Fabiano Rosas]

Cédric Le Goater <clg@kaod.org> writes:

> It facilitates reading the logs when mask CPU_LOG_INT is activated. We
> should do the same for error codes.
>
> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

Re: [PATCH 1/8] target/ppc: Print out literal exception names in logs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5109 bytes --]

On Wed, Dec 22, 2021 at 07:40:18AM +0100, Cédric le Goater wrote:
> It facilitates reading the logs when mask CPU_LOG_INT is activated. We
> should do the same for error codes.
> 
> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  target/ppc/excp_helper.c | 75 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index f90e616aacda..71ad983e67d6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -32,6 +32,78 @@
>  
>  /* #define DEBUG_SOFTWARE_TLB */
>  
> +static inline const char *powerpc_excp_name(int excp)
> +{
> +    switch (excp) {
> +    case POWERPC_EXCP_CRITICAL: return "CRITICAL";
> +    case POWERPC_EXCP_MCHECK:   return "MCHECK";
> +    case POWERPC_EXCP_DSI:      return "DSI";
> +    case POWERPC_EXCP_ISI:      return "ISI";
> +    case POWERPC_EXCP_EXTERNAL: return "EXTERNAL";
> +    case POWERPC_EXCP_ALIGN:    return "ALIGN";
> +    case POWERPC_EXCP_PROGRAM:  return "PROGRAM";
> +    case POWERPC_EXCP_FPU:      return "FPU";
> +    case POWERPC_EXCP_SYSCALL:  return "SYSCALL";
> +    case POWERPC_EXCP_APU:      return "APU";
> +    case POWERPC_EXCP_DECR:     return "DECR";
> +    case POWERPC_EXCP_FIT:      return "FIT";
> +    case POWERPC_EXCP_WDT:      return "WDT";
> +    case POWERPC_EXCP_DTLB:     return "DTLB";
> +    case POWERPC_EXCP_ITLB:     return "ITLB";
> +    case POWERPC_EXCP_DEBUG:    return "DEBUG";
> +    case POWERPC_EXCP_SPEU:     return "SPEU";
> +    case POWERPC_EXCP_EFPDI:    return "EFPDI";
> +    case POWERPC_EXCP_EFPRI:    return "EFPRI";
> +    case POWERPC_EXCP_EPERFM:   return "EPERFM";
> +    case POWERPC_EXCP_DOORI:    return "DOORI";
> +    case POWERPC_EXCP_DOORCI:   return "DOORCI";
> +    case POWERPC_EXCP_GDOORI:   return "GDOORI";
> +    case POWERPC_EXCP_GDOORCI:  return "GDOORCI";
> +    case POWERPC_EXCP_HYPPRIV:  return "HYPPRIV";
> +    case POWERPC_EXCP_RESET:    return "RESET";
> +    case POWERPC_EXCP_DSEG:     return "DSEG";
> +    case POWERPC_EXCP_ISEG:     return "ISEG";
> +    case POWERPC_EXCP_HDECR:    return "HDECR";
> +    case POWERPC_EXCP_TRACE:    return "TRACE";
> +    case POWERPC_EXCP_HDSI:     return "HDSI";
> +    case POWERPC_EXCP_HISI:     return "HISI";
> +    case POWERPC_EXCP_HDSEG:    return "HDSEG";
> +    case POWERPC_EXCP_HISEG:    return "HISEG";
> +    case POWERPC_EXCP_VPU:      return "VPU";
> +    case POWERPC_EXCP_PIT:      return "PIT";
> +    case POWERPC_EXCP_IO:       return "IO";
> +    case POWERPC_EXCP_RUNM:     return "RUNM";
> +    case POWERPC_EXCP_EMUL:     return "EMUL";
> +    case POWERPC_EXCP_IFTLB:    return "IFTLB";
> +    case POWERPC_EXCP_DLTLB:    return "DLTLB";
> +    case POWERPC_EXCP_DSTLB:    return "DSTLB";
> +    case POWERPC_EXCP_FPA:      return "FPA";
> +    case POWERPC_EXCP_DABR:     return "DABR";
> +    case POWERPC_EXCP_IABR:     return "IABR";
> +    case POWERPC_EXCP_SMI:      return "SMI";
> +    case POWERPC_EXCP_PERFM:    return "PERFM";
> +    case POWERPC_EXCP_THERM:    return "THERM";
> +    case POWERPC_EXCP_VPUA:     return "VPUA";
> +    case POWERPC_EXCP_SOFTP:    return "SOFTP";
> +    case POWERPC_EXCP_MAINT:    return "MAINT";
> +    case POWERPC_EXCP_MEXTBR:   return "MEXTBR";
> +    case POWERPC_EXCP_NMEXTBR:  return "NMEXTBR";
> +    case POWERPC_EXCP_ITLBE:    return "ITLBE";
> +    case POWERPC_EXCP_DTLBE:    return "DTLBE";
> +    case POWERPC_EXCP_VSXU:     return "VSXU";
> +    case POWERPC_EXCP_FU:       return "FU";
> +    case POWERPC_EXCP_HV_EMU:   return "HV_EMU";
> +    case POWERPC_EXCP_HV_MAINT: return "HV_MAINT";
> +    case POWERPC_EXCP_HV_FU:    return "HV_FU";
> +    case POWERPC_EXCP_SDOOR:    return "SDOOR";
> +    case POWERPC_EXCP_SDOOR_HV: return "SDOOR_HV";
> +    case POWERPC_EXCP_HVIRT:    return "HVIRT";
> +    case POWERPC_EXCP_SYSCALL_VECTORED: return "SYSCALL_VECTORED";
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>  /*****************************************************************************/
>  /* Exception processing */
>  #if !defined(CONFIG_USER_ONLY)
> @@ -301,7 +373,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>      int srr0, srr1, asrr0, asrr1, lev = -1;
>  
>      qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
> -                  " => %08x (%02x)\n", env->nip, excp, env->error_code);
> +                  " => %s (%d) error=%02x\n", env->nip, powerpc_excp_name(excp),
> +                  excp, env->error_code);
>  
>      /* new srr1 value excluding must-be-zero bits */
>      if (excp_model == POWERPC_EXCP_BOOKE) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/8] ppc/ppc4xx: Convert printfs()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7737 bytes --]

On Wed, Dec 22, 2021 at 07:40:19AM +0100, Cédric le Goater wrote:
> Use a QEMU log primitive for errors and trace events for debug.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.drobear.id.au>

> ---
>  hw/ppc/mpc8544_guts.c |  9 ++++++---
>  hw/ppc/ppc4xx_devs.c  | 39 +++++++++++----------------------------
>  hw/ppc/ppc4xx_pci.c   | 11 +++++++----
>  hw/ppc/trace-events   |  5 +++++
>  4 files changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
> index e8d2d51c20c0..a26e83d0484b 100644
> --- a/hw/ppc/mpc8544_guts.c
> +++ b/hw/ppc/mpc8544_guts.c
> @@ -19,6 +19,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
> +#include "qemu/log.h"
>  #include "sysemu/runstate.h"
>  #include "cpu.h"
>  #include "hw/sysbus.h"
> @@ -82,7 +83,9 @@ static uint64_t mpc8544_guts_read(void *opaque, hwaddr addr,
>          value = env->spr[SPR_E500_SVR];
>          break;
>      default:
> -        fprintf(stderr, "guts: Unknown register read: %x\n", (int)addr);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Unknown register 0x%" HWADDR_PRIx "\n",
> +                      __func__, addr);
>          break;
>      }
>  
> @@ -101,8 +104,8 @@ static void mpc8544_guts_write(void *opaque, hwaddr addr,
>          }
>          break;
>      default:
> -        fprintf(stderr, "guts: Unknown register write: %x = %x\n",
> -                (int)addr, (unsigned)value);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown register 0x%" HWADDR_PRIx
> +                       " = 0x%" PRIx64 "\n", __func__, addr, value);
>          break;
>      }
>  }
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index 980c48944fc7..e7d82ae5016c 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -35,14 +35,7 @@
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> -
> -/*#define DEBUG_UIC*/
> -
> -#ifdef DEBUG_UIC
> -#  define LOG_UIC(...) qemu_log_mask(CPU_LOG_INT, ## __VA_ARGS__)
> -#else
> -#  define LOG_UIC(...) do { } while (0)
> -#endif
> +#include "trace.h"
>  
>  static void ppc4xx_reset(void *opaque)
>  {
> @@ -137,8 +130,9 @@ static uint32_t sdram_bcr (hwaddr ram_base,
>          bcr = 0x000C0000;
>          break;
>      default:
> -        printf("%s: invalid RAM size " TARGET_FMT_plx "\n", __func__,
> -               ram_size);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid RAM size 0x%" HWADDR_PRIx "\n", __func__,
> +                      ram_size);
>          return 0x00000000;
>      }
>      bcr |= ram_base & 0xFF800000;
> @@ -171,10 +165,8 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram, int i,
>  {
>      if (sdram->bcr[i] & 0x00000001) {
>          /* Unmap RAM */
> -#ifdef DEBUG_SDRAM
> -        printf("%s: unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
> -               __func__, sdram_base(sdram->bcr[i]), sdram_size(sdram->bcr[i]));
> -#endif
> +        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bcr[i]),
> +                                 sdram_size(sdram->bcr[i]));
>          memory_region_del_subregion(get_system_memory(),
>                                      &sdram->containers[i]);
>          memory_region_del_subregion(&sdram->containers[i],
> @@ -183,10 +175,7 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram, int i,
>      }
>      sdram->bcr[i] = bcr & 0xFFDEE001;
>      if (enabled && (bcr & 0x00000001)) {
> -#ifdef DEBUG_SDRAM
> -        printf("%s: Map RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
> -               __func__, sdram_base(bcr), sdram_size(bcr));
> -#endif
> +        trace_ppc4xx_sdram_unmap(sdram_base(bcr), sdram_size(bcr));
>          memory_region_init(&sdram->containers[i], NULL, "sdram-containers",
>                             sdram_size(bcr));
>          memory_region_add_subregion(&sdram->containers[i], 0,
> @@ -216,10 +205,8 @@ static void sdram_unmap_bcr (ppc4xx_sdram_t *sdram)
>      int i;
>  
>      for (i = 0; i < sdram->nbanks; i++) {
> -#ifdef DEBUG_SDRAM
> -        printf("%s: Unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
> -               __func__, sdram_base(sdram->bcr[i]), sdram_size(sdram->bcr[i]));
> -#endif
> +        trace_ppc4xx_sdram_unmap(sdram_base(sdram->bcr[i]),
> +                                 sdram_size(sdram->bcr[i]));
>          memory_region_del_subregion(get_system_memory(),
>                                      &sdram->ram_memories[i]);
>      }
> @@ -316,16 +303,12 @@ static void dcr_write_sdram (void *opaque, int dcrn, uint32_t val)
>          case 0x20: /* SDRAM_CFG */
>              val &= 0xFFE00000;
>              if (!(sdram->cfg & 0x80000000) && (val & 0x80000000)) {
> -#ifdef DEBUG_SDRAM
> -                printf("%s: enable SDRAM controller\n", __func__);
> -#endif
> +                trace_ppc4xx_sdram_enable("enable");
>                  /* validate all RAM mappings */
>                  sdram_map_bcr(sdram);
>                  sdram->status &= ~0x80000000;
>              } else if ((sdram->cfg & 0x80000000) && !(val & 0x80000000)) {
> -#ifdef DEBUG_SDRAM
> -                printf("%s: disable SDRAM controller\n", __func__);
> -#endif
> +                trace_ppc4xx_sdram_enable("disable");
>                  /* invalidate all RAM mappings */
>                  sdram_unmap_bcr(sdram);
>                  sdram->status |= 0x80000000;
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index 304a29349c2e..5df97e6d156f 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -20,6 +20,7 @@
>   * 4xx SoCs, such as the 440EP. */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "hw/irq.h"
>  #include "hw/ppc/ppc.h"
>  #include "hw/ppc/ppc4xx.h"
> @@ -152,8 +153,9 @@ static void ppc4xx_pci_reg_write4(void *opaque, hwaddr offset,
>          break;
>  
>      default:
> -        printf("%s: unhandled PCI internal register 0x%lx\n", __func__,
> -               (unsigned long)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                     "%s: unhandled PCI internal register 0x%" HWADDR_PRIx "\n",
> +                     __func__, offset);
>          break;
>      }
>  }
> @@ -218,8 +220,9 @@ static uint64_t ppc4xx_pci_reg_read4(void *opaque, hwaddr offset,
>          break;
>  
>      default:
> -        printf("%s: invalid PCI internal register 0x%lx\n", __func__,
> -               (unsigned long)offset);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid PCI internal register 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
>          value = 0;
>      }
>  
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index ada644652d94..0c55aa501471 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -164,3 +164,8 @@ ppc4xx_gpt_init(uint64_t addr) "offet 0x%" PRIx64
>  
>  ppc405ep_clocks_compute(const char *param, uint32_t param2, uint32_t val) "%s 0x%1" PRIx32 " %d"
>  ppc405ep_clocks_setup(const char *trace) "%s"
> +
> +# ppc4xx_devs.c
> +ppc4xx_sdram_enable(const char *trace) "%s SDRAM controller"
> +ppc4xx_sdram_unmap(uint64_t addr, uint64_t size) "Unmap RAM area 0x%" PRIx64 " size 0x%" PRIx64
> +ppc4xx_sdram_map(uint64_t addr, uint64_t size) "Map RAM area 0x%" PRIx64 " size 0x%" PRIx64

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/8] target/ppc: Print out literal exception names in logs

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> +static inline const char *powerpc_excp_name(int excp)

Drop the inline.  It's not performance critical; let the compiler decide.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/8] ppc/ppc405: Activate MMU logs

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> There is no need to deactivate MMU logging at compile time.
> 
> Signed-off-by: Cédric Le Goater<clg@kaod.org>
> ---
>   target/ppc/mmu_common.c | 4 ++--
>   target/ppc/mmu_helper.c | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)

There's also no need to use #defines.
You should just remove these and the ifdefs.


r~

Re: [PATCH 4/8] ppc/ppc405: Restore TCR and STR write handlers

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> The 405 timers were broken when booke support was added. Assumption
> was made that the register numbers were the same but it's not :
> 
>      SPR_BOOKE_TSR         (0x150)
>      SPR_BOOKE_TCR         (0x154)
>      SPR_40x_TSR           (0x3D8)
>      SPR_40x_TCR           (0x3DA)
> 
> Cc: Christophe Leroy<christophe.leroy@c-s.fr>
> Fixes: ddd1055b07fd ("PPC: booke timers")
> Signed-off-by: Cédric Le Goater<clg@kaod.org>
> ---
>   target/ppc/cpu.h             |  2 ++
>   target/ppc/helper.h          |  2 ++
>   target/ppc/spr_tcg.h         |  2 ++
>   hw/ppc/ppc.c                 | 25 +++++++++++++++++++++++++
>   target/ppc/cpu_init.c        |  4 ++--
>   target/ppc/timebase_helper.c | 10 ++++++++++
>   target/ppc/translate.c       | 12 ++++++++++++
>   hw/ppc/trace-events          |  2 ++
>   8 files changed, 57 insertions(+), 2 deletions(-)

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

r~

Re: [PATCH 5/8] ppc/ppc405: Rework ppc_40x_timers_init() to use a PowerPCCPU

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> This is a small cleanup to ease reading.
> 
> Signed-off-by: Cédric Le Goater<clg@kaod.org>
> ---
>   hw/ppc/ppc.c | 42 +++++++++++++++++++-----------------------
>   1 file changed, 19 insertions(+), 23 deletions(-)

You should mention dropping the comparison vs null as impossible as a result of g_malloc0.

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


r~

Re: [PATCH 6/8] ppc/ppc405: Fix timer initialization

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> Timers are already initialized in ppc4xx_init(). No need to do it a
> second time with a wrong set.
> 
> Fixes: d715ea961254 ("PPC: 405: Fix ppc405ep initialization")
> Signed-off-by: Cédric Le Goater<clg@kaod.org>
> ---
>   hw/ppc/ppc405_uc.c | 2 --
>   1 file changed, 2 deletions(-)

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

r~

Re: [PATCH 7/8] ppc/ppc405: Introduce a store helper for SPR_40x_PID

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> The PID SPR of the 405 CPU contains the translation ID of the TLB
> which is a 8-bit field. Enforce the mask with a store helper.
> 
> Cc: Christophe Leroy<christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater<clg@kaod.org>
> ---
>   target/ppc/spr_tcg.h   | 1 +
>   target/ppc/cpu_init.c  | 2 +-
>   target/ppc/translate.c | 8 ++++++++
>   3 files changed, 10 insertions(+), 1 deletion(-)

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

r~

Re: [PATCH 8/8] ppc/ppc405: Dump specific registers

[Richard Henderson]

On 12/21/21 10:40 PM, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   target/ppc/cpu_init.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 9ef9a1a5ddd5..8f6a58e82483 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -8689,6 +8689,17 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>            * they can be read with "p $ivor0", "p $ivor1", etc.
>            */
>       }
> +    if (env->excp_model == POWERPC_EXCP_40x) {

Perhaps turn this and the previous booke test into a switch.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> +        qemu_fprintf(f, "  TCR " TARGET_FMT_lx "   TSR " TARGET_FMT_lx
> +                     "    ESR " TARGET_FMT_lx "   DEAR " TARGET_FMT_lx "\n",
> +                     env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
> +                     env->spr[SPR_40x_ESR], env->spr[SPR_40x_DEAR]);
> +
> +        qemu_fprintf(f, " EVPR " TARGET_FMT_lx "  SRR2 " TARGET_FMT_lx
> +                     "   SRR3 " TARGET_FMT_lx  "   PID " TARGET_FMT_lx "\n",
> +                     env->spr[SPR_40x_EVPR], env->spr[SPR_40x_SRR2],
> +                     env->spr[SPR_40x_SRR3], env->spr[SPR_40x_PID]);
> +    }
>   
>   #if defined(TARGET_PPC64)
>       if (env->flags & POWERPC_FLAG_CFAR) {
> 

Re: [PATCH 3/8] ppc/ppc405: Activate MMU logs

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Thu, 23 Dec 2021, Richard Henderson wrote:
> On 12/21/21 10:40 PM, Cédric Le Goater wrote:
>> There is no need to deactivate MMU logging at compile time.
>> 
>> Signed-off-by: Cédric Le Goater<clg@kaod.org>
>> ---
>>   target/ppc/mmu_common.c | 4 ++--
>>   target/ppc/mmu_helper.c | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> There's also no need to use #defines.
> You should just remove these and the ifdefs.

I thought the same unless these are disabled by default for performance 
reasons. MMU is already quite slow, would this make it even slower?

Regards,
BALATON Zoltan

Re: [PATCH 3/8] ppc/ppc405: Activate MMU logs

[Richard Henderson]

On 12/24/21 4:57 AM, BALATON Zoltan wrote:
> On Thu, 23 Dec 2021, Richard Henderson wrote:
>> On 12/21/21 10:40 PM, Cédric Le Goater wrote:
>>> There is no need to deactivate MMU logging at compile time.
>>>
>>> Signed-off-by: Cédric Le Goater<clg@kaod.org>
>>> ---
>>>   target/ppc/mmu_common.c | 4 ++--
>>>   target/ppc/mmu_helper.c | 2 +-
>>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> There's also no need to use #defines.
>> You should just remove these and the ifdefs.
> 
> I thought the same unless these are disabled by default for performance reasons. MMU is 
> already quite slow, would this make it even slower?

I don't believe the difference will be measurable.


r~