[Qemu-devel] [PATCH 0/8] PPC: e5500 emulation

[Qemu-devel] [PATCH 0/8] PPC: e5500 emulation

[Alexander Graf]

This patch set adds support to emulate an e5500 based virtual machine. We don't
have a machine model for that one yet, but with this patch set applied we can
fake the compatibility property of the MPC8544DS model into P5020DS, which
gets guest kernels working for me.

The patch set is based on my recent dynamic device tree work. For a ready to use
git tree, please check here:

  git://repo.or.cz/qemu/agraf.git ppc-e5500

To use the code, grab yourself an e5500 kernel and run:

  $ qemu-system-ppc64 -M mpc8544ds -cpu e5500 -nographic -kernel uImage \
    -machine dt_compatible=fsl,,P5020DS

This should get you a working kernel. Everything after that works just the same
as with e500v2 or e500mc.


Alex

Alexander Graf (8):
  dt: make setprop argument static
  PPC: e500: allow users to set the /compatible property via -machine
  uImage: increase the gzip load size
  PPC: Add some booke SPR defines
  PPC: Add support for MSR_CM
  PPC: BookE: Implement EPR SPR
  PPC: Turn hardcoded reset mask into env variable
  PPC: Add e5500 CPU target

 device_tree.c               |    2 +-
 device_tree.h               |    2 +-
 hw/loader.c                 |    4 +-
 hw/ppce500_mpc8544ds.c      |   13 ++++-
 qemu-config.c               |    4 ++
 target-ppc/Makefile.objs    |    1 +
 target-ppc/cpu.h            |   33 ++++++++++++
 target-ppc/excp_helper.c    |    9 ++--
 target-ppc/helper.h         |    1 +
 target-ppc/mem_helper.c     |    2 +-
 target-ppc/mpic_helper.c    |   35 +++++++++++++
 target-ppc/translate.c      |    2 +-
 target-ppc/translate_init.c |  118 +++++++++++++++++++++++++++++++++++++++---
 13 files changed, 204 insertions(+), 22 deletions(-)
 create mode 100644 target-ppc/mpic_helper.c

[Qemu-devel] [PATCH 1/8] dt: make setprop argument static

[Alexander Graf]

Whatever we pass in to qemu_devtree_setprop to put into the device tree
will not get modified by that function, so it can easily be declared const.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    2 +-
 device_tree.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index acae53e..b366fdd 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -127,7 +127,7 @@ static int findnode_nofail(void *fdt, const char *node_path)
 }
 
 int qemu_devtree_setprop(void *fdt, const char *node_path,
-                         const char *property, void *val_array, int size)
+                         const char *property, const void *val_array, int size)
 {
     int r;
 
diff --git a/device_tree.h b/device_tree.h
index 4898d95..2244270 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -18,7 +18,7 @@ void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 
 int qemu_devtree_setprop(void *fdt, const char *node_path,
-                         const char *property, void *val_array, int size);
+                         const char *property, const void *val_array, int size);
 int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
                               const char *property, uint32_t val);
 int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/8] PPC: e500: allow users to set the /compatible property via -machine

[Alexander Graf]

Device trees usually have a node /compatible, which indicate which machine
type we're looking at. For quick prototyping, it can be very useful to change
the contents of that node via the command line.

Thus, introduce a new option to -machine called dt_compatible, which when
set changes the /compatible contents to its value.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   12 +++++++++---
 qemu-config.c          |    4 ++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index f6da25b..d38ad99 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -119,7 +119,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     uint32_t clock_freq = 400000000;
     uint32_t tb_freq = 400000000;
     int i;
-    char compatible[] = "MPC8544DS\0MPC85xxDS";
+    const char *compatible = "MPC8544DS\0MPC85xxDS";
+    int compatible_len = sizeof("MPC8544DS\0MPC85xxDS");
     char compatible_sb[] = "fsl,mpc8544-immr\0simple-bus";
     char model[] = "MPC8544DS";
     char soc[128];
@@ -144,8 +145,14 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 
     machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
     if (machine_opts) {
+        const char *tmp;
         dumpdtb = qemu_opt_get(machine_opts, "dumpdtb");
         dtb_file = qemu_opt_get(machine_opts, "dtb");
+        tmp = qemu_opt_get(machine_opts, "dt_compatible");
+        if (tmp) {
+            compatible = tmp;
+            compatible_len = strlen(compatible) + 1;
+        }
     }
 
     if (dtb_file) {
@@ -169,8 +176,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 
     /* Manipulate device tree in memory. */
     qemu_devtree_setprop_string(fdt, "/", "model", model);
-    qemu_devtree_setprop(fdt, "/", "compatible", compatible,
-                         sizeof(compatible));
+    qemu_devtree_setprop(fdt, "/", "compatible", compatible, compatible_len);
     qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 2);
     qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 2);
 
diff --git a/qemu-config.c b/qemu-config.c
index 2cd2726..5c3296b 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -591,6 +591,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "phandle_start",
             .type = QEMU_OPT_STRING,
             .help = "The first phandle ID we may generate dynamically",
+        }, {
+            .name = "dt_compatible",
+            .type = QEMU_OPT_STRING,
+            .help = "Overrides the \"compatible\" property of the dt root node",
         },
         { /* End of list */ }
     },
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/8] uImage: increase the gzip load size

[Alexander Graf]

Recent u-boot has different defines for its gzip extract buffer, but the
common ground seems to be 64MB. So let's bump it up to that, enabling me
to load my test image again ;).

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/loader.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/loader.c b/hw/loader.c
index 7d64113..33acc2f 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -377,9 +377,9 @@ static void zfree(void *x, void *addr)
 
 #define DEFLATED	8
 
-/* This is the maximum in uboot, so if a uImage overflows this, it would
+/* This is the usual maximum in uboot, so if a uImage overflows this, it would
  * overflow on real hardware too. */
-#define UBOOT_MAX_GUNZIP_BYTES 0x800000
+#define UBOOT_MAX_GUNZIP_BYTES (64 << 20)
 
 static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
                       size_t srclen)
-- 
1.6.0.2

[Qemu-devel] [PATCH 4/8] PPC: Add some booke SPR defines

[Alexander Graf]

The number of SPRs avaiable in different PowerPC chip is still increasing. Add
definitions for the MAS7_MAS3 SPR and all currently known bits in EPCR.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/cpu.h |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 67e699c..12200ab 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1395,6 +1395,7 @@ static inline void cpu_clone_regs(CPUPPCState *env, target_ulong newsp)
 #define SPR_BOOKE_TLB1PS      (0x159)
 #define SPR_BOOKE_TLB2PS      (0x15A)
 #define SPR_BOOKE_TLB3PS      (0x15B)
+#define SPR_BOOKE_MAS7_MAS3   (0x174)
 #define SPR_BOOKE_IVOR0       (0x190)
 #define SPR_BOOKE_IVOR1       (0x191)
 #define SPR_BOOKE_IVOR2       (0x192)
@@ -1762,6 +1763,27 @@ static inline void cpu_clone_regs(CPUPPCState *env, target_ulong newsp)
 #define SPR_604_HID15         (0x3FF)
 #define SPR_E500_SVR          (0x3FF)
 
+/* Disable MAS Interrupt Updates for Hypervisor */
+#define EPCR_DMIUH            (1 << 22)
+/* Disable Guest TLB Management Instructions */
+#define EPCR_DGTMI            (1 << 23)
+/* Guest Interrupt Computation Mode */
+#define EPCR_GICM             (1 << 24)
+/* Interrupt Computation Mode */
+#define EPCR_ICM              (1 << 25)
+/* Disable Embedded Hypervisor Debug */
+#define EPCR_DUVD             (1 << 26)
+/* Instruction Storage Interrupt Directed to Guest State */
+#define EPCR_ISIGS            (1 << 27)
+/* Data Storage Interrupt Directed to Guest State */
+#define EPCR_DSIGS            (1 << 28)
+/* Instruction TLB Error Interrupt Directed to Guest State */
+#define EPCR_ITLBGS           (1 << 29)
+/* Data TLB Error Interrupt Directed to Guest State */
+#define EPCR_DTLBGS           (1 << 30)
+/* External Input Interrupt Directed to Guest State */
+#define EPCR_EXTGS            (1 << 31)
+
 /*****************************************************************************/
 /* PowerPC Instructions types definitions                                    */
 enum {
-- 
1.6.0.2

[Qemu-devel] [PATCH 5/8] PPC: Add support for MSR_CM

[Alexander Graf]

The BookE variant of MSR_SF is MSR_CM. Implement everything it takes in TCG to
support running 64bit code with MSR_CM set.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/cpu.h         |    9 +++++++++
 target-ppc/excp_helper.c |    9 +++++----
 target-ppc/mem_helper.c  |    2 +-
 target-ppc/translate.c   |    2 +-
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 12200ab..7a77fff 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2212,6 +2212,15 @@ static inline uint32_t booke206_tlbnps(CPUPPCState *env, const int tlbn)
 
 #endif
 
+static inline bool msr_is_64bit(CPUPPCState *env, target_ulong msr)
+{
+    if (env->mmu_model == POWERPC_MMU_BOOKE206) {
+        return msr & (1ULL << MSR_CM);
+    }
+
+    return msr & (1ULL << MSR_SF);
+}
+
 extern void (*cpu_ppc_hypercall)(CPUPPCState *);
 
 static inline bool cpu_has_work(CPUPPCState *env)
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index c7762b9..1a593f6 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -608,10 +608,11 @@ static inline void powerpc_excp(CPUPPCState *env, int excp_model, int excp)
     vector |= env->excp_prefix;
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_BOOKE) {
-        if (!msr_icm) {
-            vector = (uint32_t)vector;
-        } else {
+        if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
+            /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
             new_msr |= (target_ulong)1 << MSR_CM;
+        } else {
+            vector = (uint32_t)vector;
         }
     } else {
         if (!msr_isf && !(env->mmu_model & POWERPC_MMU_64)) {
@@ -803,7 +804,7 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
                           target_ulong msrm, int keep_msrh)
 {
 #if defined(TARGET_PPC64)
-    if (msr & (1ULL << MSR_SF)) {
+    if (msr_is_64bit(env, msr)) {
         nip = (uint64_t)nip;
         msr &= (uint64_t)msrm;
     } else {
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index ebcd7b2..5b5f1bd 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -35,7 +35,7 @@ static inline target_ulong addr_add(CPUPPCState *env, target_ulong addr,
                                     target_long arg)
 {
 #if defined(TARGET_PPC64)
-    if (!msr_sf) {
+    if (!msr_is_64bit(env, env->msr)) {
         return (uint32_t)(addr + arg);
     } else
 #endif
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 9103fd5..73ee74b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -9626,7 +9626,7 @@ static inline void gen_intermediate_code_internal(CPUPPCState *env,
     ctx.access_type = -1;
     ctx.le_mode = env->hflags & (1 << MSR_LE) ? 1 : 0;
 #if defined(TARGET_PPC64)
-    ctx.sf_mode = msr_sf;
+    ctx.sf_mode = msr_is_64bit(env, env->msr);
     ctx.has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
 #endif
     ctx.fpu_enabled = msr_fp;
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/8] PPC: BookE: Implement EPR SPR

[Alexander Graf]

On the e500 series, accessing SPR_EPR magically turns into an access at
that CPU's IACK register on the MPIC. Implement that logic to get kernels
that make use of that feature work.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c   |    1 +
 target-ppc/Makefile.objs |    1 +
 target-ppc/cpu.h         |    1 +
 target-ppc/helper.h      |    1 +
 target-ppc/mpic_helper.c |   35 +++++++++++++++++++++++++++++++++++
 5 files changed, 39 insertions(+), 0 deletions(-)
 create mode 100644 target-ppc/mpic_helper.c

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index d38ad99..8b9fd83 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -469,6 +469,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
         irqs[i][OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
         irqs[i][OPENPIC_OUTPUT_CINT] = input[PPCE500_INPUT_CINT];
         env->spr[SPR_BOOKE_PIR] = env->cpu_index = i;
+        env->mpic_cpu_base = MPC8544_MPIC_REGS_BASE + 0x20000;
 
         ppc_booke_timers_init(env, 400000000, PPC_TIMER_E500);
 
diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs
index 6c11ef8..237a0ed 100644
--- a/target-ppc/Makefile.objs
+++ b/target-ppc/Makefile.objs
@@ -9,3 +9,4 @@ obj-y += mmu_helper.o
 obj-y += timebase_helper.o
 obj-y += misc_helper.o
 obj-y += mem_helper.o
+obj-y += mpic_helper.o
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 7a77fff..652a35a 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1066,6 +1066,7 @@ struct CPUPPCState {
     target_ulong ivor_mask;
     target_ulong ivpr_mask;
     target_ulong hreset_vector;
+    target_phys_addr_t mpic_cpu_base;
 #endif
 
     /* Those resources are used only during code translation */
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index ddab97b..fd04c06 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -405,6 +405,7 @@ DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
 DEF_HELPER_2(store_booke_tcr, void, env, tl)
 DEF_HELPER_2(store_booke_tsr, void, env, tl)
+DEF_HELPER_1(load_epr, tl, env)
 DEF_HELPER_3(store_ibatl, void, env, i32, tl)
 DEF_HELPER_3(store_ibatu, void, env, i32, tl)
 DEF_HELPER_3(store_dbatl, void, env, i32, tl)
diff --git a/target-ppc/mpic_helper.c b/target-ppc/mpic_helper.c
new file mode 100644
index 0000000..2c6a4d3
--- /dev/null
+++ b/target-ppc/mpic_helper.c
@@ -0,0 +1,35 @@
+/*
+ *  PowerPC emulation helpers for QEMU.
+ *
+ *  Copyright (c) 2003-2007 Jocelyn Mayer
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "cpu.h"
+#include "helper.h"
+
+/*****************************************************************************/
+/* SPR accesses */
+
+#if !defined(CONFIG_USER_ONLY)
+/*
+ * This is an ugly helper for EPR, which is basically the same as accessing
+ * the IACK (PIAC) register on the MPIC. Because we model the MPIC as a device
+ * that can only talk to the CPU through MMIO, let's access it that way!
+ */
+target_ulong helper_load_epr(CPUPPCState *env)
+{
+    return ldl_phys(env->mpic_cpu_base + 0xA0);
+}
+#endif
-- 
1.6.0.2

[Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Alexander Graf]

Some machines have MSR bits they reset with as enabled. Don't hardcode the
logic, but let the individual core implementations save their own reset
mask into an env variable.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/cpu.h            |    1 +
 target-ppc/translate_init.c |   14 ++++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 652a35a..acf5816 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1043,6 +1043,7 @@ struct CPUPPCState {
 #if defined(TARGET_PPC64)
     struct ppc_segment_page_sizes sps;
 #endif
+    uint64_t reset_msr;
 
 #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
     target_phys_addr_t vpa;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 57027a2..efa05fc 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -6273,6 +6273,7 @@ static void init_proc_970 (CPUPPCState *env)
     env->slb_nr = 32;
 #endif
     init_excp_970(env);
+    env->reset_msr = (1ULL < MSR_SF);
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
     /* Allocate hardware IRQ controller */
@@ -6375,6 +6376,7 @@ static void init_proc_970FX (CPUPPCState *env)
     env->slb_nr = 64;
 #endif
     init_excp_970(env);
+    env->reset_msr = (1ULL < MSR_SF);
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
     /* Allocate hardware IRQ controller */
@@ -6465,6 +6467,7 @@ static void init_proc_970GX (CPUPPCState *env)
     env->slb_nr = 32;
 #endif
     init_excp_970(env);
+    env->reset_msr = (1ULL < MSR_SF);
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
     /* Allocate hardware IRQ controller */
@@ -6555,6 +6558,7 @@ static void init_proc_970MP (CPUPPCState *env)
     env->slb_nr = 32;
 #endif
     init_excp_970(env);
+    env->reset_msr = (1ULL < MSR_SF);
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
     /* Allocate hardware IRQ controller */
@@ -6640,6 +6644,7 @@ static void init_proc_POWER7 (CPUPPCState *env)
     env->slb_nr = 32;
 #endif
     init_excp_POWER7(env);
+    env->reset_msr = (1ULL < MSR_SF);
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
     /* Allocate hardware IRQ controller */
@@ -6686,6 +6691,7 @@ static void init_proc_620 (CPUPPCState *env)
     /* Memory management */
     gen_low_BATs(env);
     init_excp_620(env);
+    env->reset_msr = (1ULL < MSR_SF);
     env->dcache_line_size = 64;
     env->icache_line_size = 64;
     /* Allocate hardware IRQ controller */
@@ -9306,6 +9312,7 @@ static void init_ppc_proc (CPUPPCState *env, const ppc_def_t *def)
     env->nb_BATs = 0;
     env->nb_tlb = 0;
     env->nb_ways = 0;
+    env->reset_msr = 0;
     env->tlb_type = TLB_NONE;
 #endif
     /* Register SPR common to all PowerPC implementations */
@@ -10246,7 +10253,7 @@ static void ppc_cpu_reset(CPUState *s)
 
     pcc->parent_reset(s);
 
-    msr = (target_ulong)0;
+    msr = (target_ulong)env->reset_msr;
     if (0) {
         /* XXX: find a suitable condition to enable the hypervisor mode */
         msr |= (target_ulong)MSR_HVB;
@@ -10272,11 +10279,6 @@ static void ppc_cpu_reset(CPUState *s)
     }
 #endif
     env->msr = msr & env->msr_mask;
-#if defined(TARGET_PPC64)
-    if (env->mmu_model & POWERPC_MMU_64) {
-        env->msr |= (1ULL << MSR_SF);
-    }
-#endif
     hreg_compute_hflags(env);
     env->reserve_addr = (target_ulong)-1ULL;
     /* Be sure no exception or interrupt is pending */
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Blue Swirl]

On Wed, Jun 20, 2012 at 8:11 PM, Alexander Graf <agraf@suse.de> wrote:
> Some machines have MSR bits they reset with as enabled. Don't hardcode the
> logic, but let the individual core implementations save their own reset
> mask into an env variable.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-ppc/cpu.h            |    1 +
>  target-ppc/translate_init.c |   14 ++++++++------
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 652a35a..acf5816 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1043,6 +1043,7 @@ struct CPUPPCState {
>  #if defined(TARGET_PPC64)
>     struct ppc_segment_page_sizes sps;
>  #endif
> +    uint64_t reset_msr;
>
>  #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>     target_phys_addr_t vpa;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 57027a2..efa05fc 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -6273,6 +6273,7 @@ static void init_proc_970 (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);

The parenthesis are not necessary.

>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6375,6 +6376,7 @@ static void init_proc_970FX (CPUPPCState *env)
>     env->slb_nr = 64;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6465,6 +6467,7 @@ static void init_proc_970GX (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6555,6 +6558,7 @@ static void init_proc_970MP (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6640,6 +6644,7 @@ static void init_proc_POWER7 (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_POWER7(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6686,6 +6691,7 @@ static void init_proc_620 (CPUPPCState *env)
>     /* Memory management */
>     gen_low_BATs(env);
>     init_excp_620(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 64;
>     env->icache_line_size = 64;
>     /* Allocate hardware IRQ controller */
> @@ -9306,6 +9312,7 @@ static void init_ppc_proc (CPUPPCState *env, const ppc_def_t *def)
>     env->nb_BATs = 0;
>     env->nb_tlb = 0;
>     env->nb_ways = 0;
> +    env->reset_msr = 0;
>     env->tlb_type = TLB_NONE;
>  #endif
>     /* Register SPR common to all PowerPC implementations */
> @@ -10246,7 +10253,7 @@ static void ppc_cpu_reset(CPUState *s)
>
>     pcc->parent_reset(s);
>
> -    msr = (target_ulong)0;
> +    msr = (target_ulong)env->reset_msr;
>     if (0) {
>         /* XXX: find a suitable condition to enable the hypervisor mode */
>         msr |= (target_ulong)MSR_HVB;
> @@ -10272,11 +10279,6 @@ static void ppc_cpu_reset(CPUState *s)
>     }
>  #endif
>     env->msr = msr & env->msr_mask;
> -#if defined(TARGET_PPC64)
> -    if (env->mmu_model & POWERPC_MMU_64) {
> -        env->msr |= (1ULL << MSR_SF);
> -    }
> -#endif
>     hreg_compute_hflags(env);
>     env->reserve_addr = (target_ulong)-1ULL;
>     /* Be sure no exception or interrupt is pending */
> --
> 1.6.0.2
>
>

Re: [Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Alexander Graf]

On 21.06.2012, at 20:09, Blue Swirl <blauwirbel@gmail.com> wrote:

> On Wed, Jun 20, 2012 at 8:11 PM, Alexander Graf <agraf@suse.de> wrote:
>> Some machines have MSR bits they reset with as enabled. Don't hardcode the
>> logic, but let the individual core implementations save their own reset
>> mask into an env variable.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  target-ppc/cpu.h            |    1 +
>>  target-ppc/translate_init.c |   14 ++++++++------
>>  2 files changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 652a35a..acf5816 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1043,6 +1043,7 @@ struct CPUPPCState {
>>  #if defined(TARGET_PPC64)
>>     struct ppc_segment_page_sizes sps;
>>  #endif
>> +    uint64_t reset_msr;
>> 
>>  #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>>     target_phys_addr_t vpa;
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 57027a2..efa05fc 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -6273,6 +6273,7 @@ static void init_proc_970 (CPUPPCState *env)
>>     env->slb_nr = 32;
>>  #endif
>>     init_excp_970(env);
>> +    env->reset_msr = (1ULL < MSR_SF);
> 
> The parenthesis are not necessary.

Already dropped this patch :).

Alex

Re: [Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Peter Maydell]

On 20 June 2012 21:11, Alexander Graf <agraf@suse.de> wrote:
> +    env->reset_msr = (1ULL < MSR_SF);

I assume you mean "<<" rather than "<" here and below...

-- PMM

[Qemu-devel] [PATCH 8/8] PPC: Add e5500 CPU target

[Alexander Graf]

This patch adds e5500's CPU initialization to the TCG CPU initialization
code.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/translate_init.c |  104 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 101 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index efa05fc..63452cc 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4424,16 +4424,69 @@ static void init_proc_e300 (CPUPPCState *env)
 #define check_pow_e500mc       check_pow_none
 #define init_proc_e500mc       init_proc_e500mc
 
+/* e5500 core                                                                 */
+#define POWERPC_INSNS_e5500    (PPC_INSNS_BASE | PPC_ISEL |                    \
+                                PPC_WRTEE | PPC_RFDI | PPC_RFMCI |             \
+                                PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI |  \
+                                PPC_CACHE_DCBZ | PPC_CACHE_DCBA |              \
+                                PPC_FLOAT | PPC_FLOAT_FRES |                   \
+                                PPC_FLOAT_FRSQRTE | PPC_FLOAT_FSEL |           \
+                                PPC_FLOAT_STFIWX | PPC_WAIT |                  \
+                                PPC_MEM_TLBSYNC | PPC_TLBIVAX | PPC_MEM_SYNC | \
+                                PPC_64B | PPC_POPCNTB | PPC_POPCNTWD)
+#define POWERPC_INSNS2_e5500   (PPC2_BOOKE206 | PPC2_PRCNTL)
+#define POWERPC_MSRM_e5500     (0x000000009402FB36ULL)
+#define POWERPC_MMU_e5500      (POWERPC_MMU_BOOKE206)
+#define POWERPC_EXCP_e5500     (POWERPC_EXCP_BOOKE)
+#define POWERPC_INPUT_e5500    (PPC_FLAGS_INPUT_BookE)
+/* Fixme: figure out the correct flag for e5500 */
+#define POWERPC_BFDM_e5500     (bfd_mach_ppc_e500)
+#define POWERPC_FLAG_e5500     (POWERPC_FLAG_CE | POWERPC_FLAG_DE | \
+                                POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK)
+#define check_pow_e5500        check_pow_none
+#define init_proc_e5500        init_proc_e5500
+
+#if !defined(CONFIG_USER_ONLY)
+static void spr_write_mas73(void *opaque, int sprn, int gprn)
+{
+    TCGv val = tcg_temp_new();
+    tcg_gen_ext32u_tl(val, cpu_gpr[gprn]);
+    gen_store_spr(SPR_BOOKE_MAS3, val);
+    tcg_gen_shri_tl(val, gprn, 32);
+    gen_store_spr(SPR_BOOKE_MAS7, val);
+    tcg_temp_free(val);
+}
+
+static void spr_read_mas73(void *opaque, int gprn, int sprn)
+{
+    TCGv mas7 = tcg_temp_new();
+    TCGv mas3 = tcg_temp_new();
+    gen_load_spr(mas7, SPR_BOOKE_MAS7);
+    tcg_gen_shli_tl(mas7, mas7, 32);
+    gen_load_spr(mas3, SPR_BOOKE_MAS3);
+    tcg_gen_or_tl(cpu_gpr[gprn], mas3, mas7);
+    tcg_temp_free(mas3);
+    tcg_temp_free(mas7);
+}
+
+static void spr_load_epr(void *opaque, int gprn, int sprn)
+{
+    gen_helper_load_epr(cpu_gpr[gprn], cpu_env);
+}
+
+#endif
+
 enum fsl_e500_version {
     fsl_e500v1,
     fsl_e500v2,
     fsl_e500mc,
+    fsl_e5500,
 };
 
 static void init_proc_e500 (CPUPPCState *env, int version)
 {
     uint32_t tlbncfg[2];
-    uint64_t ivor_mask = 0x0000000F0000FFFFULL;
+    uint64_t ivor_mask;
     uint32_t l1cfg0 = 0x3800  /* 8 ways */
                     | 0x0020; /* 32 kb */
 #if !defined(CONFIG_USER_ONLY)
@@ -4447,8 +4500,16 @@ static void init_proc_e500 (CPUPPCState *env, int version)
      *     complain when accessing them.
      * gen_spr_BookE(env, 0x0000000F0000FD7FULL);
      */
-    if (version == fsl_e500mc) {
-        ivor_mask = 0x000003FE0000FFFFULL;
+    switch (version) {
+        case fsl_e500v1:
+        case fsl_e500v2:
+        default:
+            ivor_mask = 0x0000000F0000FFFFULL;
+            break;
+        case fsl_e500mc:
+        case fsl_e5500:
+            ivor_mask = 0x000003FE0000FFFFULL;
+            break;
     }
     gen_spr_BookE(env, ivor_mask);
     /* Processor identification */
@@ -4476,6 +4537,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
         tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16);
         break;
     case fsl_e500mc:
+    case fsl_e5500:
         tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512);
         tlbncfg[1] = gen_tlbncfg(64, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 64);
         break;
@@ -4491,6 +4553,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
         env->icache_line_size = 32;
         break;
     case fsl_e500mc:
+    case fsl_e5500:
         env->dcache_line_size = 64;
         env->icache_line_size = 64;
         l1cfg0 |= 0x1000000; /* 64 byte cache block size */
@@ -4566,6 +4629,22 @@ static void init_proc_e500 (CPUPPCState *env, int version)
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_generic, &spr_write_booke206_mmucsr0,
                  0x00000000);
+    spr_register(env, SPR_BOOKE_EPR, "EPR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_load_epr, SPR_NOACCESS,
+                 0x00000000);
+    /* XXX better abstract into Emb.xxx features */
+    if (version == fsl_e5500) {
+        spr_register(env, SPR_BOOKE_EPCR, "EPCR",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     &spr_read_generic, &spr_write_generic,
+                     0x00000000);
+        spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     &spr_read_mas73, &spr_write_mas73,
+                     0x00000000);
+        env->reset_msr = (1ULL < MSR_CM);
+    }
 
 #if !defined(CONFIG_USER_ONLY)
     env->nb_tlb = 0;
@@ -4576,6 +4655,14 @@ static void init_proc_e500 (CPUPPCState *env, int version)
 #endif
 
     init_excp_e200(env);
+
+#if !defined(CONFIG_USER_ONLY)
+    /* We support 64bit wide IVPR on 64bit platforms */
+    if (version == fsl_e5500) {
+        env->ivpr_mask = (target_ulong)~0xFFFFULL;
+    }
+#endif
+
     /* Allocate hardware IRQ controller */
     ppce500_irq_init(env);
 }
@@ -4595,6 +4682,13 @@ static void init_proc_e500mc(CPUPPCState *env)
     init_proc_e500(env, fsl_e500mc);
 }
 
+#ifdef TARGET_PPC64
+static void init_proc_e5500(CPUPPCState *env)
+{
+    init_proc_e500(env, fsl_e5500);
+}
+#endif
+
 /* Non-embedded PowerPC                                                      */
 
 /* POWER : same as 601, without mfmsr, mfsr                                  */
@@ -7139,6 +7233,7 @@ enum {
     CPU_POWERPC_e500v2_v22         = 0x80210022,
     CPU_POWERPC_e500v2_v30         = 0x80210030,
     CPU_POWERPC_e500mc             = 0x80230020,
+    CPU_POWERPC_e5500              = 0x80240020,
     /* MPC85xx microcontrollers */
 #define CPU_POWERPC_MPC8533          CPU_POWERPC_MPC8533_v11
 #define CPU_POWERPC_MPC8533_v10      CPU_POWERPC_e500v2_v21
@@ -8533,6 +8628,9 @@ static const ppc_def_t ppc_defs[] = {
     /* PowerPC e500v2 v3.0 core                                              */
     POWERPC_DEF("e500v2_v30",    CPU_POWERPC_e500v2_v30,             e500v2),
     POWERPC_DEF("e500mc",        CPU_POWERPC_e500mc,                 e500mc),
+#ifdef TARGET_PPC64
+    POWERPC_DEF("e5500",         CPU_POWERPC_e5500,                  e5500),
+#endif
     /* PowerPC e500 microcontrollers                                         */
     /* MPC8533                                                               */
     POWERPC_DEF_SVR("MPC8533",
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 8/8] PPC: Add e5500 CPU target

[Scott Wood]

On 06/20/2012 03:11 PM, Alexander Graf wrote:
> +    /* XXX better abstract into Emb.xxx features */
> +    if (version == fsl_e5500) {
> +        spr_register(env, SPR_BOOKE_EPCR, "EPCR",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_generic,
> +                     0x00000000);
> +        spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_mas73, &spr_write_mas73,
> +                     0x00000000);
> +        env->reset_msr = (1ULL < MSR_CM);

That's a funny way of writing "env->reset_msr = 0". :-)

Assuming you really meant "env->reset_msr = 1ULL << MSR_CM", why?  We
enter the kernel in 32-bit mode.  It resets in 32-bit mode as well, if
we ever implement that for e5500 QEMU.

You have the same issue in the previous patch with "1ULL < MSR_SF",
though I don't know if those chips actually do reset into 64-bit mode.

> +    }
>  
>  #if !defined(CONFIG_USER_ONLY)
>      env->nb_tlb = 0;
> @@ -4576,6 +4655,14 @@ static void init_proc_e500 (CPUPPCState *env, int version)
>  #endif
>  
>      init_excp_e200(env);
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    /* We support 64bit wide IVPR on 64bit platforms */
> +    if (version == fsl_e5500) {
> +        env->ivpr_mask = (target_ulong)~0xFFFFULL;
> +    }
> +#endif

So, I'm guessing you don't do this unconditionally because QEMU will
generate 64-bit code if compiled that way, regardless of the actual
target -- and you don't want stray garbage in the upper 32 bits being
written into IVPR.  But why isn't this an issue with all the other SPRs?
 Why don't we have a problem with junk being written into the upper half
of MAS3, for example (there's MAS3_RPN_MASK, but it's not used)?

Speaking of which, I don't see where you change MAS2_EPN_MASK to take
MSR_CM into account (or to accept 64-bit MAS2 at all).

-Scott

Re: [Qemu-devel] [PATCH 8/8] PPC: Add e5500 CPU target

[Alexander Graf]

On 21.06.2012, at 00:26, Scott Wood wrote:

> On 06/20/2012 03:11 PM, Alexander Graf wrote:
>> +    /* XXX better abstract into Emb.xxx features */
>> +    if (version == fsl_e5500) {
>> +        spr_register(env, SPR_BOOKE_EPCR, "EPCR",
>> +                     SPR_NOACCESS, SPR_NOACCESS,
>> +                     &spr_read_generic, &spr_write_generic,
>> +                     0x00000000);
>> +        spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3",
>> +                     SPR_NOACCESS, SPR_NOACCESS,
>> +                     &spr_read_mas73, &spr_write_mas73,
>> +                     0x00000000);
>> +        env->reset_msr = (1ULL < MSR_CM);
> 
> That's a funny way of writing "env->reset_msr = 0". :-)
> 
> Assuming you really meant "env->reset_msr = 1ULL << MSR_CM", why?  We
> enter the kernel in 32-bit mode.  It resets in 32-bit mode as well, if
> we ever implement that for e5500 QEMU.

Hrm. At least my self-compiled kernel did issue an "ld" instruction before going into MSR_CM mode, hence I figured we need it.

> You have the same issue in the previous patch with "1ULL < MSR_SF",
> though I don't know if those chips actually do reset into 64-bit mode.

They do :).

> 
>> +    }
>> 
>> #if !defined(CONFIG_USER_ONLY)
>>     env->nb_tlb = 0;
>> @@ -4576,6 +4655,14 @@ static void init_proc_e500 (CPUPPCState *env, int version)
>> #endif
>> 
>>     init_excp_e200(env);
>> +
>> +#if !defined(CONFIG_USER_ONLY)
>> +    /* We support 64bit wide IVPR on 64bit platforms */
>> +    if (version == fsl_e5500) {
>> +        env->ivpr_mask = (target_ulong)~0xFFFFULL;
>> +    }
>> +#endif
> 
> So, I'm guessing you don't do this unconditionally because QEMU will
> generate 64-bit code if compiled that way, regardless of the actual
> target -- and you don't want stray garbage in the upper 32 bits being
> written into IVPR.  But why isn't this an issue with all the other SPRs?
> Why don't we have a problem with junk being written into the upper half
> of MAS3, for example (there's MAS3_RPN_MASK, but it's not used)?

I was thinking of making it unconditional, but this way seemed cleaner to me, as it actually follows exactly what the spec says. Not sure what would happen if you have -1 in your 32-bit register value and you try to write that to IVPR otherwise. It'd probably break :).

> Speaking of which, I don't see where you change MAS2_EPN_MASK to take
> MSR_CM into account (or to accept 64-bit MAS2 at all).

Ugh. Good question why this works at all. Do we by accident truncate everything to 32bit?


Alex

Re: [Qemu-devel] [PATCH 8/8] PPC: Add e5500 CPU target

[Scott Wood]

On 06/20/2012 05:59 PM, Alexander Graf wrote:
> 
> On 21.06.2012, at 00:26, Scott Wood wrote:
> 
>> On 06/20/2012 03:11 PM, Alexander Graf wrote:
>>> +    /* XXX better abstract into Emb.xxx features */
>>> +    if (version == fsl_e5500) {
>>> +        spr_register(env, SPR_BOOKE_EPCR, "EPCR",
>>> +                     SPR_NOACCESS, SPR_NOACCESS,
>>> +                     &spr_read_generic, &spr_write_generic,
>>> +                     0x00000000);
>>> +        spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3",
>>> +                     SPR_NOACCESS, SPR_NOACCESS,
>>> +                     &spr_read_mas73, &spr_write_mas73,
>>> +                     0x00000000);
>>> +        env->reset_msr = (1ULL < MSR_CM);
>>
>> That's a funny way of writing "env->reset_msr = 0". :-)
>>
>> Assuming you really meant "env->reset_msr = 1ULL << MSR_CM", why?  We
>> enter the kernel in 32-bit mode.  It resets in 32-bit mode as well, if
>> we ever implement that for e5500 QEMU.
> 
> Hrm. At least my self-compiled kernel did issue an "ld" instruction before going into MSR_CM mode, hence I figured we need it.

You don't need MSR_CM to run 64-bit instructions.  It just affects
masking in certain places.

>>> +    }
>>>
>>> #if !defined(CONFIG_USER_ONLY)
>>>     env->nb_tlb = 0;
>>> @@ -4576,6 +4655,14 @@ static void init_proc_e500 (CPUPPCState *env, int version)
>>> #endif
>>>
>>>     init_excp_e200(env);
>>> +
>>> +#if !defined(CONFIG_USER_ONLY)
>>> +    /* We support 64bit wide IVPR on 64bit platforms */
>>> +    if (version == fsl_e5500) {
>>> +        env->ivpr_mask = (target_ulong)~0xFFFFULL;
>>> +    }
>>> +#endif
>>
>> So, I'm guessing you don't do this unconditionally because QEMU will
>> generate 64-bit code if compiled that way, regardless of the actual
>> target -- and you don't want stray garbage in the upper 32 bits being
>> written into IVPR.  But why isn't this an issue with all the other SPRs?
>> Why don't we have a problem with junk being written into the upper half
>> of MAS3, for example (there's MAS3_RPN_MASK, but it's not used)?
> 
> I was thinking of making it unconditional, but this way seemed
> cleaner to me, as it actually follows exactly what the spec says. Not
> sure what would happen if you have -1 in your 32-bit register value
> and you try to write that to IVPR otherwise. It'd probably break :).

It would only break because there doesn't seem to be any generic way of
treating 32-bit SPRs as 32-bit.  We should probably have a separate
spr_write_generic32().  For a register like IVPR we'd select 32 or
full-size at init time, based on the type of CPU we're modelling.  For
something like MAS3 we'd always use the 32-bit version.

-Scott

Re: [Qemu-devel] [PATCH 8/8] PPC: Add e5500 CPU target

[Alexander Graf]

On 21.06.2012, at 01:07, Scott Wood wrote:

> On 06/20/2012 05:59 PM, Alexander Graf wrote:
>> 
>> On 21.06.2012, at 00:26, Scott Wood wrote:
>> 
>>> On 06/20/2012 03:11 PM, Alexander Graf wrote:
>>>> +    /* XXX better abstract into Emb.xxx features */
>>>> +    if (version == fsl_e5500) {
>>>> +        spr_register(env, SPR_BOOKE_EPCR, "EPCR",
>>>> +                     SPR_NOACCESS, SPR_NOACCESS,
>>>> +                     &spr_read_generic, &spr_write_generic,
>>>> +                     0x00000000);
>>>> +        spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3",
>>>> +                     SPR_NOACCESS, SPR_NOACCESS,
>>>> +                     &spr_read_mas73, &spr_write_mas73,
>>>> +                     0x00000000);
>>>> +        env->reset_msr = (1ULL < MSR_CM);
>>> 
>>> That's a funny way of writing "env->reset_msr = 0". :-)
>>> 
>>> Assuming you really meant "env->reset_msr = 1ULL << MSR_CM", why?  We
>>> enter the kernel in 32-bit mode.  It resets in 32-bit mode as well, if
>>> we ever implement that for e5500 QEMU.
>> 
>> Hrm. At least my self-compiled kernel did issue an "ld" instruction before going into MSR_CM mode, hence I figured we need it.
> 
> You don't need MSR_CM to run 64-bit instructions.  It just affects
> masking in certain places.

Wait - you don't? Is there a comprehensive description on what MSR_CM really does and does not?

> 
>>>> +    }
>>>> 
>>>> #if !defined(CONFIG_USER_ONLY)
>>>>    env->nb_tlb = 0;
>>>> @@ -4576,6 +4655,14 @@ static void init_proc_e500 (CPUPPCState *env, int version)
>>>> #endif
>>>> 
>>>>    init_excp_e200(env);
>>>> +
>>>> +#if !defined(CONFIG_USER_ONLY)
>>>> +    /* We support 64bit wide IVPR on 64bit platforms */
>>>> +    if (version == fsl_e5500) {
>>>> +        env->ivpr_mask = (target_ulong)~0xFFFFULL;
>>>> +    }
>>>> +#endif
>>> 
>>> So, I'm guessing you don't do this unconditionally because QEMU will
>>> generate 64-bit code if compiled that way, regardless of the actual
>>> target -- and you don't want stray garbage in the upper 32 bits being
>>> written into IVPR.  But why isn't this an issue with all the other SPRs?
>>> Why don't we have a problem with junk being written into the upper half
>>> of MAS3, for example (there's MAS3_RPN_MASK, but it's not used)?
>> 
>> I was thinking of making it unconditional, but this way seemed
>> cleaner to me, as it actually follows exactly what the spec says. Not
>> sure what would happen if you have -1 in your 32-bit register value
>> and you try to write that to IVPR otherwise. It'd probably break :).
> 
> It would only break because there doesn't seem to be any generic way of
> treating 32-bit SPRs as 32-bit.  We should probably have a separate
> spr_write_generic32().  For a register like IVPR we'd select 32 or
> full-size at init time, based on the type of CPU we're modelling.  For
> something like MAS3 we'd always use the 32-bit version.

Yup, that should work :)


Alex

Re: [Qemu-devel] [PATCH 8/8] PPC: Add e5500 CPU target

[Scott Wood]

On 06/20/2012 06:10 PM, Alexander Graf wrote:
> 
> On 21.06.2012, at 01:07, Scott Wood wrote:
> 
>> On 06/20/2012 05:59 PM, Alexander Graf wrote:
>>>
>>> On 21.06.2012, at 00:26, Scott Wood wrote:
>>>
>>>> On 06/20/2012 03:11 PM, Alexander Graf wrote:
>>>>> +    /* XXX better abstract into Emb.xxx features */
>>>>> +    if (version == fsl_e5500) {
>>>>> +        spr_register(env, SPR_BOOKE_EPCR, "EPCR",
>>>>> +                     SPR_NOACCESS, SPR_NOACCESS,
>>>>> +                     &spr_read_generic, &spr_write_generic,
>>>>> +                     0x00000000);
>>>>> +        spr_register(env, SPR_BOOKE_MAS7_MAS3, "MAS7_MAS3",
>>>>> +                     SPR_NOACCESS, SPR_NOACCESS,
>>>>> +                     &spr_read_mas73, &spr_write_mas73,
>>>>> +                     0x00000000);
>>>>> +        env->reset_msr = (1ULL < MSR_CM);
>>>>
>>>> That's a funny way of writing "env->reset_msr = 0". :-)
>>>>
>>>> Assuming you really meant "env->reset_msr = 1ULL << MSR_CM", why?  We
>>>> enter the kernel in 32-bit mode.  It resets in 32-bit mode as well, if
>>>> we ever implement that for e5500 QEMU.
>>>
>>> Hrm. At least my self-compiled kernel did issue an "ld" instruction before going into MSR_CM mode, hence I figured we need it.
>>
>> You don't need MSR_CM to run 64-bit instructions.  It just affects
>> masking in certain places.
> 
> Wait - you don't? Is there a comprehensive description on what MSR_CM really does and does not?

Not that I know of -- you need to search the ISA for places that mention
MSR[CM] or 64-bit mode.

-Scott