Add Microblaze configuration options

PATCH Superseded Alistair Francis 2 participants 17 messages May 18, 2015 – May 28, 2015

lore

Share & tools

lore

Alistair Francis

author

2015-05-18 01:13 UTC #0 lore raw

cover Add Microblaze configuration options

Firstly this patch series tidies up some code and removes a "xlnx." prefix.

Then it moves the Microblaze PVR registers to the end of the CPUMBState to preserve them during reset. This allows most of the operations on them to be moved from the reset to the realise. Except for the machine specific ones which will be moved when the other properties are converted across to standard QEMU props, after this patch series is accepted (either merged or the method I'm using is approved). See the individual commit for more details.

Next it adds the "use-stack-protection" property to the Microblaze CPU, which allows stack protection to be disabled.

It also converts the previously hardcoded method of enabling the FPU to use standard QEMU properties. This simplifies the logic in the target-microblaze translate.c.

Changes since RFC:
- Preserve the PVR registers during resets
- Move most of the logic into realise functions
- Small name and function changes

Alistair Francis (5):
target-microblaze: Fix up indentation
target-microblaze: Preserve the pvr registers during reset
target-microblaze: Allow the stack protection to be disabled
target-microblaze: Tidy up the base-vectors property
target-microblaze: Convert use-fpu to a CPU property

 hw/microblaze/petalogix_ml605_mmu.c |    4 +-
 target-microblaze/cpu-qom.h         |    9 +++++-
 target-microblaze/cpu.c             |   59 ++++++++++++++++++++++-------------
 target-microblaze/cpu.h             |   11 ++++--
 target-microblaze/helper.c          |    8 ++--
 target-microblaze/op_helper.c       |   10 +++---
 target-microblaze/translate.c       |    8 ++--
 7 files changed, 67 insertions(+), 42 deletions(-)

Alistair Francis

author

2015-05-18 01:13 UTC #1 lore raw

PATCH 1/5 target-microblaze: Fix up indentation

Fix up the incorrect indentation level in the helper_stackprot() function.

Signed-off-by Alistair Francis <[email protected]>

---

 target-microblaze/op_helper.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

target-microblaze/op_helper.c +5 -5

index a4c8f04..d2b3624 100644
@@ -468,11 +468,11 @@ void helper_memalign(CPUMBState *env, uint32_t addr, uint32_t dr, uint32_t wr,
 void helper_stackprot(CPUMBState *env, uint32_t addr)
 {
     if (addr < env->slr || addr > env->shr) {
-            qemu_log("Stack protector violation at %x %x %x\n",
-                     addr, env->slr, env->shr);
-            env->sregs[SR_EAR] = addr;
-            env->sregs[SR_ESR] = ESR_EC_STACKPROT;
-            helper_raise_exception(env, EXCP_HW_EXCP);
+        qemu_log("Stack protector violation at %x %x %x\n",
+                 addr, env->slr, env->shr);
+        env->sregs[SR_EAR] = addr;
+        env->sregs[SR_ESR] = ESR_EC_STACKPROT;
+        helper_raise_exception(env, EXCP_HW_EXCP);
     }
 }

Signature · 1 line

1.7.1

Peter Crosthwaite

Reviewed-by on v1 1/5

2015-05-25 03:44 UTC #2 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 1/5] target-microblaze: Fix up indentation

Quoted 3 lines · Alistair Francis

Fix up the incorrect indentation level in the helper_stackprot() function.
Signed-off-by Alistair Francis <[email protected]>

Reviewed-by Peter Crosthwaite <[email protected]>

Quoted 30 lines

---

 target-microblaze/op_helper.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

target-microblaze/op_helper.c +5 -6

index a4c8f04..d2b3624 100644
@@ -468,11 +468,11 @@ void helper_memalign(CPUMBState *env, uint32_t addr, uint32_t dr, uint32_t wr,
 void helper_stackprot(CPUMBState *env, uint32_t addr)
 {
     if (addr < env->slr || addr > env->shr) {
-            qemu_log("Stack protector violation at %x %x %x\n",
-                     addr, env->slr, env->shr);
-            env->sregs[SR_EAR] = addr;
-            env->sregs[SR_ESR] = ESR_EC_STACKPROT;
-            helper_raise_exception(env, EXCP_HW_EXCP);
+        qemu_log("Stack protector violation at %x %x %x\n",
+                 addr, env->slr, env->shr);
+        env->sregs[SR_EAR] = addr;
+        env->sregs[SR_ESR] = ESR_EC_STACKPROT;
+        helper_raise_exception(env, EXCP_HW_EXCP);
     }
 }
 
--

1.7.1

Alistair Francis

author

2015-05-28 00:37 UTC #3 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 1/5] target-microblaze: Fix up indentation

Quoted 7 lines · Peter Crosthwaite

Quoted 3 lines · Alistair Francis
Fix up the incorrect indentation level in the helper_stackprot() function.
Signed-off-by Alistair Francis <[email protected]>
Reviewed-by Peter Crosthwaite <[email protected]>

Thanks

Alistair

Quoted 32 lines

Quoted 31 lines

---

 target-microblaze/op_helper.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

target-microblaze/op_helper.c +5 -6

index a4c8f04..d2b3624 100644
@@ -468,11 +468,11 @@ void helper_memalign(CPUMBState *env, uint32_t addr, uint32_t dr, uint32_t wr,
 void helper_stackprot(CPUMBState *env, uint32_t addr)
 {
     if (addr < env->slr || addr > env->shr) {
-            qemu_log("Stack protector violation at %x %x %x\n",
-                     addr, env->slr, env->shr);
-            env->sregs[SR_EAR] = addr;
-            env->sregs[SR_ESR] = ESR_EC_STACKPROT;
-            helper_raise_exception(env, EXCP_HW_EXCP);
+        qemu_log("Stack protector violation at %x %x %x\n",
+                 addr, env->slr, env->shr);
+        env->sregs[SR_EAR] = addr;
+        env->sregs[SR_ESR] = ESR_EC_STACKPROT;
+        helper_raise_exception(env, EXCP_HW_EXCP);
     }
 }
 
--

1.7.1

Alistair Francis

author

2015-05-18 01:13 UTC #4 lore raw

PATCH 2/5 target-microblaze: Preserve the pvr registers during reset

In reply to Alistair Francis

Move the Microblaze PVR registers to the end of the CPUMBState and preserve them during reset. This is similar to what the QEMU ARM model does with some of it's registers.

This allows the Microblaze PVR registers to only be set once at realise instead of constantly at reset.

Signed-off-by Alistair Francis <[email protected]>

---

NOTE: The individial machine resets still write to the PVR registers on each reset. This is no longer required as it only needs to be done once. Instead of moving them now, they are being left there and will be removed when they are all converted to the standard CPU properties.

 target-microblaze/cpu.c |   43 +++++++++++++++++++++++++------------------
 target-microblaze/cpu.h |   10 ++++++----
 2 files changed, 31 insertions(+), 22 deletions(-)

target-microblaze/cpu.c +25 ~~-18~~

index 67e3182..555bc4c 100644
@@ -63,13 +63,37 @@ static void mb_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, sizeof(CPUMBState));
+    memset(env, 0, offsetof(CPUMBState, pvr));
     env->res_addr = RES_ADDR_NONE;
     tlb_flush(s, 1);
 
     /* Disable stack protector.  */
     env->shr = ~0;
 
+#if defined(CONFIG_USER_ONLY)
+    /* start in user mode with interrupts enabled.  */
+    env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM;
+#else
+    env->sregs[SR_MSR] = 0;
+    mmu_init(&env->mmu);
+    env->mmu.c_mmu = 3;
+    env->mmu.c_mmu_tlb_access = 3;
+    env->mmu.c_mmu_zones = 16;
+#endif
+}
+
+static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
+{
+    CPUState *cs = CPU(dev);
+    MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
+    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+    CPUMBState *env = &cpu->env;
+
+    cpu_reset(cs);
+    qemu_init_vcpu(cs);
+
+    memset(&env->pvr, 0, sizeof(env->pvr));
+
     env->pvr.regs[0] = PVR0_PVR_FULL_MASK \
                        | PVR0_USE_BARREL_MASK \
                        | PVR0_USE_DIV_MASK \

@@ -99,25 +123,8 @@ static void mb_cpu_reset(CPUState *s)
     env->sregs[SR_PC] = cpu->base_vectors;
 
 #if defined(CONFIG_USER_ONLY)
-    /* start in user mode with interrupts enabled.  */
-    env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM;
     env->pvr.regs[10] = 0x0c000000; /* Spartan 3a dsp.  */
-#else
-    env->sregs[SR_MSR] = 0;
-    mmu_init(&env->mmu);
-    env->mmu.c_mmu = 3;
-    env->mmu.c_mmu_tlb_access = 3;
-    env->mmu.c_mmu_zones = 16;
 #endif
-}
-
-static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
-{
-    CPUState *cs = CPU(dev);
-    MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
-
-    cpu_reset(cs);
-    qemu_init_vcpu(cs);
 
     mcc->parent_realize(dev, errp);
 }

target-microblaze/cpu.h +6 -4

index 4ea04ac..e4c1cde 100644
@@ -260,16 +260,18 @@ struct CPUMBState {
 #define IFLAGS_TB_MASK  (D_FLAG | IMM_FLAG | DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)
     uint32_t iflags;
 
-    struct {
-        uint32_t regs[16];
-    } pvr;
-
 #if !defined(CONFIG_USER_ONLY)
     /* Unified MMU.  */
     struct microblaze_mmu mmu;
 #endif
 
     CPU_COMMON
+
+    /* These fields are preserved on reset.  */
+
+    struct {
+        uint32_t regs[16];
+    } pvr;
 };
 
 #include "cpu-qom.h"

Signature · 1 line

1.7.1

Peter Crosthwaite

Reviewed-by on v1 2/5

2015-05-25 03:54 UTC #5 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 2/5] target-microblaze: Preserve the pvr registers during reset

Quoted 46 lines · Alistair Francis

Move the Microblaze PVR registers to the end of the CPUMBState and preserve them during reset. This is similar to what the QEMU ARM model does with some of it's registers.
This allows the Microblaze PVR registers to only be set once at realise instead of constantly at reset.
Signed-off-by Alistair Francis <[email protected]>
---
NOTE: The individial machine resets still write to the PVR registers on each reset. This is no longer required as it only needs to be done once. Instead of moving them now, they are being left there and will be removed when they are all converted to the standard CPU properties.
 target-microblaze/cpu.c |   43 +++++++++++++++++++++++++------------------
 target-microblaze/cpu.h |   10 ++++++----
 2 files changed, 31 insertions(+), 22 deletions(-)
target-microblaze/cpu.c +11 -1
index 67e3182..555bc4c 100644
@@ -63,13 +63,37 @@ static void mb_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, sizeof(CPUMBState));
+    memset(env, 0, offsetof(CPUMBState, pvr));
     env->res_addr = RES_ADDR_NONE;
     tlb_flush(s, 1);
 
     /* Disable stack protector.  */
     env->shr = ~0;
 
+#if defined(CONFIG_USER_ONLY)
+    /* start in user mode with interrupts enabled.  */
+    env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM;
+#else
+    env->sregs[SR_MSR] = 0;
+    mmu_init(&env->mmu);
+    env->mmu.c_mmu = 3;
+    env->mmu.c_mmu_tlb_access = 3;
+    env->mmu.c_mmu_zones = 16;
+#endif

target-microblaze/cpu.c static void mb_cpu_reset(CPUState *s) · line 92

Quoted 10 lines

target-microblaze/cpu.c +10 -0

+}
+
+static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
+{
+    CPUState *cs = CPU(dev);
+    MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
+    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+    CPUMBState *env = &cpu->env;
+
+    cpu_reset(cs);

So git did a poor job of showing the code motion of this patch as this is really context. But as follow up we should consider removing this as realize functions should not call resets.

target-microblaze/cpu.c static void mb_cpu_reset(CPUState *s) · line 96

Quoted 4 lines

target-microblaze/cpu.c +4 -0

+    qemu_init_vcpu(cs);
+
+    memset(&env->pvr, 0, sizeof(env->pvr));
+

Similar, 0 memsets in realize not needed but it is ultimately just context with the strange way git has done this patch.

Reviewed-by Peter Crosthwaite <[email protected]>

Regards,
Peter

Quoted 60 lines

target-microblaze/cpu.c +0 ~~-17~~

     env->pvr.regs[0] = PVR0_PVR_FULL_MASK \
                        | PVR0_USE_BARREL_MASK \
                        | PVR0_USE_DIV_MASK \

@@ -99,25 +123,8 @@ static void mb_cpu_reset(CPUState *s)
     env->sregs[SR_PC] = cpu->base_vectors;
 
 #if defined(CONFIG_USER_ONLY)
-    /* start in user mode with interrupts enabled.  */
-    env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM;
     env->pvr.regs[10] = 0x0c000000; /* Spartan 3a dsp.  */
-#else
-    env->sregs[SR_MSR] = 0;
-    mmu_init(&env->mmu);
-    env->mmu.c_mmu = 3;
-    env->mmu.c_mmu_tlb_access = 3;
-    env->mmu.c_mmu_zones = 16;
 #endif
-}
-
-static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
-{
-    CPUState *cs = CPU(dev);
-    MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
-
-    cpu_reset(cs);
-    qemu_init_vcpu(cs);
 
     mcc->parent_realize(dev, errp);
 }

target-microblaze/cpu.h +6 -5

index 4ea04ac..e4c1cde 100644
@@ -260,16 +260,18 @@ struct CPUMBState {
 #define IFLAGS_TB_MASK  (D_FLAG | IMM_FLAG | DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)
     uint32_t iflags;
 
-    struct {
-        uint32_t regs[16];
-    } pvr;
-
 #if !defined(CONFIG_USER_ONLY)
     /* Unified MMU.  */
     struct microblaze_mmu mmu;
 #endif
 
     CPU_COMMON
+
+    /* These fields are preserved on reset.  */
+
+    struct {
+        uint32_t regs[16];
+    } pvr;
 };
 
 #include "cpu-qom.h"
--

1.7.1

Alistair Francis

author

2015-05-28 00:47 UTC #6 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 2/5] target-microblaze: Preserve the pvr registers during reset

Quoted 54 lines · Peter Crosthwaite

Quoted 52 lines · Alistair Francis
Move the Microblaze PVR registers to the end of the CPUMBState and preserve them during reset. This is similar to what the QEMU ARM model does with some of it's registers.
This allows the Microblaze PVR registers to only be set once at realise instead of constantly at reset.
Signed-off-by Alistair Francis <[email protected]>
---
NOTE: The individial machine resets still write to the PVR registers on each reset. This is no longer required as it only needs to be done once. Instead of moving them now, they are being left there and will be removed when they are all converted to the standard CPU properties.
 target-microblaze/cpu.c |   43 +++++++++++++++++++++++++------------------
 target-microblaze/cpu.h |   10 ++++++----
 2 files changed, 31 insertions(+), 22 deletions(-)
target-microblaze/cpu.c +17 -1
index 67e3182..555bc4c 100644
@@ -63,13 +63,37 @@ static void mb_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, sizeof(CPUMBState));
+    memset(env, 0, offsetof(CPUMBState, pvr));
     env->res_addr = RES_ADDR_NONE;
     tlb_flush(s, 1);
 
     /* Disable stack protector.  */
     env->shr = ~0;
 
+#if defined(CONFIG_USER_ONLY)
+    /* start in user mode with interrupts enabled.  */
+    env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM;
+#else
+    env->sregs[SR_MSR] = 0;
+    mmu_init(&env->mmu);
+    env->mmu.c_mmu = 3;
+    env->mmu.c_mmu_tlb_access = 3;
+    env->mmu.c_mmu_zones = 16;
+#endif
+}
+
+static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
+{
+    CPUState *cs = CPU(dev);
+    MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);

Quoted 8 lines

target-microblaze/cpu.c static void mb_cpu_reset(CPUState *s) · line 92
Quoted 4 lines
target-microblaze/cpu.c +4 -0
+    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+    CPUMBState *env = &cpu->env;
+
+    cpu_reset(cs);
So git did a poor job of showing the code motion of this patch as this is really context. But as follow up we should consider removing this as realize functions should not call resets.

Yeah, it really struggled with this patch for some reason.

I can still remove them from this patch. I'm touching it anyway

Quoted 8 lines

target-microblaze/cpu.c static void mb_cpu_reset(CPUState *s) · line 96
Quoted 5 lines
target-microblaze/cpu.c +4 -0
+    qemu_init_vcpu(cs);
+
+    memset(&env->pvr, 0, sizeof(env->pvr));
+
Similar, 0 memsets in realize not needed but it is ultimately just context with the strange way git has done this patch.

Same with this.

Quoted 2 lines

Reviewed-by Peter Crosthwaite <[email protected]>

Thanks

Alistair

Quoted 65 lines

Regards,
Peter

Quoted 60 lines

target-microblaze/cpu.c +0 ~~-17~~

     env->pvr.regs[0] = PVR0_PVR_FULL_MASK \
                        | PVR0_USE_BARREL_MASK \
                        | PVR0_USE_DIV_MASK \

@@ -99,25 +123,8 @@ static void mb_cpu_reset(CPUState *s)
     env->sregs[SR_PC] = cpu->base_vectors;
 
 #if defined(CONFIG_USER_ONLY)
-    /* start in user mode with interrupts enabled.  */
-    env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM;
     env->pvr.regs[10] = 0x0c000000; /* Spartan 3a dsp.  */
-#else
-    env->sregs[SR_MSR] = 0;
-    mmu_init(&env->mmu);
-    env->mmu.c_mmu = 3;
-    env->mmu.c_mmu_tlb_access = 3;
-    env->mmu.c_mmu_zones = 16;
 #endif
-}
-
-static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
-{
-    CPUState *cs = CPU(dev);
-    MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
-
-    cpu_reset(cs);
-    qemu_init_vcpu(cs);
 
     mcc->parent_realize(dev, errp);
 }

target-microblaze/cpu.h +6 -5

index 4ea04ac..e4c1cde 100644
@@ -260,16 +260,18 @@ struct CPUMBState {
 #define IFLAGS_TB_MASK  (D_FLAG | IMM_FLAG | DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)
     uint32_t iflags;
 
-    struct {
-        uint32_t regs[16];
-    } pvr;
-
 #if !defined(CONFIG_USER_ONLY)
     /* Unified MMU.  */
     struct microblaze_mmu mmu;
 #endif
 
     CPU_COMMON
+
+    /* These fields are preserved on reset.  */
+
+    struct {
+        uint32_t regs[16];
+    } pvr;
 };
 
 #include "cpu-qom.h"
--

1.7.1

Alistair Francis

author

2015-05-18 01:13 UTC #7 lore raw

PATCH 3/5 target-microblaze: Allow the stack protection to be disabled

In reply to Alistair Francis

Microblaze stack protection is configurable and isn't always enabled. This patch allows the stack protection to be disabled from the CPU properties.

Signed-off-by Alistair Francis <[email protected]>

---
Changes since RFC:
- Move the cfg.stackproc check into translate.c
- Set the PVR register

 target-microblaze/cpu-qom.h   |    5 +++++
 target-microblaze/cpu.c       |    5 +++++
 target-microblaze/cpu.h       |    1 +
 target-microblaze/translate.c |    2 +-
 4 files changed, 12 insertions(+), 1 deletions(-)

target-microblaze/cpu-qom.h +5 -0

index e3e0701..7bc5b81 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackproc;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;

target-microblaze/cpu.c +5 -0

index 555bc4c..4deb256 100644
@@ -117,6 +117,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
+
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -159,6 +162,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/cpu.h +1 -0

index e4c1cde..481f463 100644
@@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState;
 #define PVR0_FAULT			0x00100000
 #define PVR0_VERSION_MASK               0x0000FF00
 #define PVR0_USER1_MASK                 0x000000FF
+#define PVR0_SPROT_MASK                 0x00000001
 
 /* User 2 PVR mask */
 #define PVR1_USER2_MASK                 0xFFFFFFFF

target-microblaze/translate.c +1 -1

index 4068946..19faf40 100644
@@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
     int stackprot = 0;
 
     /* All load/stores use ra.  */
-    if (dc->ra == 1) {
+    if (dc->ra == 1 && dc->cpu->cfg.stackproc) {
         stackprot = 1;
     }

Signature · 1 line

1.7.1

Peter Crosthwaite

2015-05-25 04:07 UTC #8 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 3/5] target-microblaze: Allow the stack protection to be disabled

Quoted 21 lines · Alistair Francis

Microblaze stack protection is configurable and isn't always enabled. This patch allows the stack protection to be disabled from the CPU properties.
Signed-off-by Alistair Francis <[email protected]>
---
Changes since RFC:
- Move the cfg.stackproc check into translate.c
- Set the PVR register
 target-microblaze/cpu-qom.h   |    5 +++++
 target-microblaze/cpu.c       |    5 +++++
 target-microblaze/cpu.h       |    1 +
 target-microblaze/translate.c |    2 +-
 4 files changed, 12 insertions(+), 1 deletions(-)
target-microblaze/cpu-qom.h +0 -0
index e3e0701..7bc5b81 100644

target-microblaze/cpu-qom.h typedef struct MicroBlazeCPU { · line 64

Quoted 6 lines

target-microblaze/cpu-qom.h +3 -0

@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackproc;

stackprot? Although should we just verbatim match to the TRMs name for these variables (dropping the redundant leading C_)? That would make this "use_stack_protection" and match to QOM property name exactly.

Quoted 10 lines

target-microblaze/cpu-qom.h +2 -0
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
target-microblaze/cpu.c +0 -0
index 555bc4c..4deb256 100644

target-microblaze/cpu.c static void mb_cpu_realizefn(DeviceState *dev, Error **errp) · line 122

Quoted 6 lines

target-microblaze/cpu.c +3 -0

@@ -117,6 +117,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
+
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+

Parentheses not needed.

Quoted 29 lines

target-microblaze/cpu.c +2 -0

     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -159,6 +162,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/cpu.h +1 -0

index e4c1cde..481f463 100644
@@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState;
 #define PVR0_FAULT                     0x00100000
 #define PVR0_VERSION_MASK               0x0000FF00
 #define PVR0_USER1_MASK                 0x000000FF
+#define PVR0_SPROT_MASK                 0x00000001
 
 /* User 2 PVR mask */
 #define PVR1_USER2_MASK                 0xFFFFFFFF

target-microblaze/translate.c +0 -0

index 4068946..19faf40 100644

target-microblaze/translate.c static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) · line 865

Quoted 5 lines

target-microblaze/translate.c +1 -1

@@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
     int stackprot = 0;
 
     /* All load/stores use ra.  */
-    if (dc->ra == 1) {
+    if (dc->ra == 1 && dc->cpu->cfg.stackproc) {

There is a similar logic below for dc->rb:

        if (dc->rb == 1) {
            stackprot = 1;
        }

Should it have the same guard?

Regards,
Peter

Quoted 7 lines

target-microblaze/translate.c +0 -1
         stackprot = 1;
     }
 
--
1.7.1

Alistair Francis

author

2015-05-26 01:56 UTC #9 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 3/5] target-microblaze: Allow the stack protection to be disabled

Quoted 26 lines · Peter Crosthwaite

Quoted 24 lines · Alistair Francis
Microblaze stack protection is configurable and isn't always enabled. This patch allows the stack protection to be disabled from the CPU properties.
Signed-off-by Alistair Francis <[email protected]>
---
Changes since RFC:
- Move the cfg.stackproc check into translate.c
- Set the PVR register
 target-microblaze/cpu-qom.h   |    5 +++++
 target-microblaze/cpu.c       |    5 +++++
 target-microblaze/cpu.h       |    1 +
 target-microblaze/translate.c |    2 +-
 4 files changed, 12 insertions(+), 1 deletions(-)
target-microblaze/cpu-qom.h +0 -0
index e3e0701..7bc5b81 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 

Quoted 7 lines

target-microblaze/cpu-qom.h typedef struct MicroBlazeCPU { · line 64
Quoted 3 lines
target-microblaze/cpu-qom.h +3 -0
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackproc;
stackprot? Although should we just verbatim match to the TRMs name for these variables (dropping the redundant leading C_)? That would make this "use_stack_protection" and match to QOM property name exactly.

I think "use_stack_protection" is too long, it makes it harder to stay under the 80 characters for each line.

I'll change it to 'stackprot' though.

Quoted 6 lines

Quoted 6 lines
target-microblaze/cpu-qom.h +2 -0
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 

Quoted 13 lines

target-microblaze/cpu.c static void mb_cpu_realizefn(DeviceState *dev, Error **errp) · line 122

Quoted 11 lines

target-microblaze/cpu.c +3 -0

index 555bc4c..4deb256 100644
@@ -117,6 +117,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
+
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+

Parentheses not needed.

True, but then they get re-added in the next patch as more configuration variables get added.

I can remove them from this one.

Quoted 38 lines

Quoted 30 lines

target-microblaze/cpu.c +2 -0

     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -159,6 +162,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/cpu.h +1 -0

index e4c1cde..481f463 100644
@@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState;
 #define PVR0_FAULT                     0x00100000
 #define PVR0_VERSION_MASK               0x0000FF00
 #define PVR0_USER1_MASK                 0x000000FF
+#define PVR0_SPROT_MASK                 0x00000001
 
 /* User 2 PVR mask */
 #define PVR1_USER2_MASK                 0xFFFFFFFF

target-microblaze/translate.c +0 -0

index 4068946..19faf40 100644

target-microblaze/translate.c static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) · line 865

Quoted 5 lines

target-microblaze/translate.c +1 -1

@@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
     int stackprot = 0;
 
     /* All load/stores use ra.  */
-    if (dc->ra == 1) {
+    if (dc->ra == 1 && dc->cpu->cfg.stackproc) {

There is a similar logic below for dc->rb:

Quoted 5 lines

        if (dc->rb == 1) {
            stackprot = 1;
        }

Should it have the same guard?

Yes, it should. I just missed that one.

Thanks,

Alistair

Quoted 12 lines

Regards,
Peter
Quoted 7 lines
target-microblaze/translate.c +0 -1
         stackprot = 1;
     }
 
--
1.7.1

Peter Crosthwaite

2015-05-26 01:59 UTC #10 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 3/5] target-microblaze: Allow the stack protection to be disabled

Quoted 61 lines · Alistair Francis

Quoted 26 lines · Peter Crosthwaite
Quoted 24 lines · Alistair Francis
Microblaze stack protection is configurable and isn't always enabled. This patch allows the stack protection to be disabled from the CPU properties.
Signed-off-by Alistair Francis <[email protected]>
---
Changes since RFC:
- Move the cfg.stackproc check into translate.c
- Set the PVR register
 target-microblaze/cpu-qom.h   |    5 +++++
 target-microblaze/cpu.c       |    5 +++++
 target-microblaze/cpu.h       |    1 +
 target-microblaze/translate.c |    2 +-
 4 files changed, 12 insertions(+), 1 deletions(-)
target-microblaze/cpu-qom.h +0 -0
index e3e0701..7bc5b81 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
Quoted 7 lines
target-microblaze/cpu-qom.h typedef struct MicroBlazeCPU { · line 64
Quoted 3 lines
target-microblaze/cpu-qom.h +3 -0
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackproc;
stackprot? Although should we just verbatim match to the TRMs name for these variables (dropping the redundant leading C_)? That would make this "use_stack_protection" and match to QOM property name exactly.
I think "use_stack_protection" is too long, it makes it harder to stay under the 80 characters for each line.
I'll change it to 'stackprot' though.
Quoted 6 lines
Quoted 6 lines
target-microblaze/cpu-qom.h +2 -0
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
Quoted 13 lines
target-microblaze/cpu.c static void mb_cpu_realizefn(DeviceState *dev, Error **errp) · line 122
Quoted 11 lines
target-microblaze/cpu.c +3 -0
index 555bc4c..4deb256 100644
@@ -117,6 +117,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
+
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+
Parentheses not needed.

Quoted 5 lines

True, but then they get re-added in the next patch as more configuration variables get added.
I can remove them from this one.

Nah just leave them. I get it now.

Regards,
Peter

Quoted 63 lines

Quoted 38 lines

Quoted 30 lines

target-microblaze/cpu.c +2 -0

     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -159,6 +162,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/cpu.h +1 -0

index e4c1cde..481f463 100644
@@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState;
 #define PVR0_FAULT                     0x00100000
 #define PVR0_VERSION_MASK               0x0000FF00
 #define PVR0_USER1_MASK                 0x000000FF
+#define PVR0_SPROT_MASK                 0x00000001
 
 /* User 2 PVR mask */
 #define PVR1_USER2_MASK                 0xFFFFFFFF

target-microblaze/translate.c +0 -0

index 4068946..19faf40 100644

target-microblaze/translate.c static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) · line 865

Quoted 5 lines

target-microblaze/translate.c +1 -1

@@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
     int stackprot = 0;
 
     /* All load/stores use ra.  */
-    if (dc->ra == 1) {
+    if (dc->ra == 1 && dc->cpu->cfg.stackproc) {

There is a similar logic below for dc->rb:

Quoted 5 lines

        if (dc->rb == 1) {
            stackprot = 1;
        }

Should it have the same guard?

Yes, it should. I just missed that one.

Thanks,

Alistair

Quoted 12 lines

Regards,
Peter
Quoted 7 lines
target-microblaze/translate.c +0 -1
         stackprot = 1;
     }
 
--
1.7.1

Alistair Francis

author

2015-05-18 01:13 UTC #11 lore raw

PATCH 4/5 target-microblaze: Tidy up the base-vectors property

In reply to Alistair Francis

Rename the "xlnx.base-vectors" string to "base-vectors" and move the base_vectors variable into the cfg struct.

Signed-off-by Alistair Francis <[email protected]>

---

 target-microblaze/cpu-qom.h |    3 ++-
 target-microblaze/cpu.c     |    4 ++--
 target-microblaze/helper.c  |    8 ++++----
 3 files changed, 8 insertions(+), 7 deletions(-)

target-microblaze/cpu-qom.h +2 -1

index 7bc5b81..750ff3b 100644
@@ -56,12 +56,13 @@ typedef struct MicroBlazeCPUClass {
 typedef struct MicroBlazeCPU {
     /*< private >*/
     CPUState parent_obj;
-    uint32_t base_vectors;
+
     /*< public >*/
 
     /* Microblaze Configuration Settings */
     struct {
         bool stackproc;
+        uint32_t base_vectors;
     } cfg;
 
     CPUMBState env;

target-microblaze/cpu.c +2 -2

index 4deb256..c7ad5d5 100644
@@ -123,7 +123,7 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
-    env->sregs[SR_PC] = cpu->base_vectors;
+    env->sregs[SR_PC] = cpu->cfg.base_vectors;
 
 #if defined(CONFIG_USER_ONLY)
     env->pvr.regs[10] = 0x0c000000; /* Spartan 3a dsp.  */

@@ -161,7 +161,7 @@ static const VMStateDescription vmstate_mb_cpu = {
 };
 
 static Property mb_properties[] = {
-    DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
                      true),
     DEFINE_PROP_END_OF_LIST(),

target-microblaze/helper.c +4 -4

index 32896f4..69c3252 100644
@@ -154,7 +154,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
                           env->sregs[SR_ESR], env->iflags);
             log_cpu_state_mask(CPU_LOG_INT, cs, 0);
             env->iflags &= ~(IMM_FLAG | D_FLAG);
-            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
+            env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x20;
             break;
 
         case EXCP_MMU:

@@ -194,7 +194,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
                           env->sregs[SR_PC], env->sregs[SR_EAR], env->iflags);
             log_cpu_state_mask(CPU_LOG_INT, cs, 0);
             env->iflags &= ~(IMM_FLAG | D_FLAG);
-            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
+            env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x20;
             break;
 
         case EXCP_IRQ:

@@ -235,7 +235,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
             env->sregs[SR_MSR] |= t;
 
             env->regs[14] = env->sregs[SR_PC];
-            env->sregs[SR_PC] = cpu->base_vectors + 0x10;
+            env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x10;
             //log_cpu_state_mask(CPU_LOG_INT, cs, 0);
             break;

@@ -254,7 +254,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
             if (cs->exception_index == EXCP_HW_BREAK) {
                 env->regs[16] = env->sregs[SR_PC];
                 env->sregs[SR_MSR] |= MSR_BIP;
-                env->sregs[SR_PC] = cpu->base_vectors + 0x18;
+                env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x18;
             } else
                 env->sregs[SR_PC] = env->btarget;
             break;

Signature · 1 line

1.7.1

Peter Crosthwaite

Reviewed-by on v1 4/5

2015-05-25 04:09 UTC #12 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 4/5] target-microblaze: Tidy up the base-vectors property

Quoted 4 lines · Alistair Francis

Rename the "xlnx.base-vectors" string to "base-vectors" and move the base_vectors variable into the cfg struct.
Signed-off-by Alistair Francis <[email protected]>

Reviewed-by Peter Crosthwaite <[email protected]>

Quoted 92 lines

---

 target-microblaze/cpu-qom.h |    3 ++-
 target-microblaze/cpu.c     |    4 ++--
 target-microblaze/helper.c  |    8 ++++----
 3 files changed, 8 insertions(+), 7 deletions(-)

target-microblaze/cpu-qom.h +2 -1

index 7bc5b81..750ff3b 100644
@@ -56,12 +56,13 @@ typedef struct MicroBlazeCPUClass {
 typedef struct MicroBlazeCPU {
     /*< private >*/
     CPUState parent_obj;
-    uint32_t base_vectors;
+
     /*< public >*/
 
     /* Microblaze Configuration Settings */
     struct {
         bool stackproc;
+        uint32_t base_vectors;
     } cfg;
 
     CPUMBState env;

target-microblaze/cpu.c +2 -2

index 4deb256..c7ad5d5 100644
@@ -123,7 +123,7 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
-    env->sregs[SR_PC] = cpu->base_vectors;
+    env->sregs[SR_PC] = cpu->cfg.base_vectors;
 
 #if defined(CONFIG_USER_ONLY)
     env->pvr.regs[10] = 0x0c000000; /* Spartan 3a dsp.  */

@@ -161,7 +161,7 @@ static const VMStateDescription vmstate_mb_cpu = {
 };
 
 static Property mb_properties[] = {
-    DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
                      true),
     DEFINE_PROP_END_OF_LIST(),

target-microblaze/helper.c +4 -5

index 32896f4..69c3252 100644
@@ -154,7 +154,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
                           env->sregs[SR_ESR], env->iflags);
             log_cpu_state_mask(CPU_LOG_INT, cs, 0);
             env->iflags &= ~(IMM_FLAG | D_FLAG);
-            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
+            env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x20;
             break;
 
         case EXCP_MMU:

@@ -194,7 +194,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
                           env->sregs[SR_PC], env->sregs[SR_EAR], env->iflags);
             log_cpu_state_mask(CPU_LOG_INT, cs, 0);
             env->iflags &= ~(IMM_FLAG | D_FLAG);
-            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
+            env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x20;
             break;
 
         case EXCP_IRQ:

@@ -235,7 +235,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
             env->sregs[SR_MSR] |= t;
 
             env->regs[14] = env->sregs[SR_PC];
-            env->sregs[SR_PC] = cpu->base_vectors + 0x10;
+            env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x10;
             //log_cpu_state_mask(CPU_LOG_INT, cs, 0);
             break;

@@ -254,7 +254,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
             if (cs->exception_index == EXCP_HW_BREAK) {
                 env->regs[16] = env->sregs[SR_PC];
                 env->sregs[SR_MSR] |= MSR_BIP;
-                env->sregs[SR_PC] = cpu->base_vectors + 0x18;
+                env->sregs[SR_PC] = cpu->cfg.base_vectors + 0x18;
             } else
                 env->sregs[SR_PC] = env->btarget;
             break;
--

1.7.1

Alistair Francis

author

2015-05-18 23:15 UTC #13 lore raw

PATCH RESEND 5/5 target-microblaze: Convert use-fpu to a CPU property

In reply to Alistair Francis

Originally the use-fpu PVR bits were manually set for each machine. This is a hassle and difficult to read, instead set them based on the CPU properties.

Signed-off-by Alistair Francis <[email protected]>

---
Changes since RFC:
- Tidy up the if logic

 hw/microblaze/petalogix_ml605_mmu.c |    4 ++--
 target-microblaze/cpu-qom.h         |    1 +
 target-microblaze/cpu.c             |    9 ++++++---
 target-microblaze/translate.c       |    6 +++---
 4 files changed, 12 insertions(+), 8 deletions(-)

hw/microblaze/petalogix_ml605_mmu.c +2 -2

index 48c264b..a93ca51 100644
@@ -71,9 +71,8 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu)
     env->pvr.regs[10] = 0x0e000000; /* virtex 6 */
     /* setup pvr to match kernel setting */
     env->pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK;
-    env->pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI;
+    env->pvr.regs[0] |= PVR0_ENDI;
     env->pvr.regs[0] = (env->pvr.regs[0] & ~PVR0_VERSION_MASK) | (0x14 << 8);
-    env->pvr.regs[2] ^= PVR2_USE_FPU2_MASK;
     env->pvr.regs[4] = 0xc56b8000;
     env->pvr.regs[5] = 0xc56be000;
 }

@@ -95,6 +94,7 @@ petalogix_ml605_init(MachineState *machine)
 
     /* init CPUs */
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+    object_property_set_int(OBJECT(cpu), 1, "use-fpu", &error_abort);
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */

target-microblaze/cpu-qom.h +1 -0

index 750ff3b..f3312f8 100644
@@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU {
     struct {
         bool stackproc;
         uint32_t base_vectors;
+        uint8_t usefpu;
     } cfg;
 
     CPUMBState env;

target-microblaze/cpu.c +6 -3

index c7ad5d5..ce40a3f 100644
@@ -113,12 +113,14 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_DIV_MASK \
                         | PVR2_USE_HW_MUL_MASK \
                         | PVR2_USE_MUL64_MASK \
-                        | PVR2_USE_FPU_MASK \
-                        | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
 
-    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0) |
+                        (cpu->cfg.usefpu ? PVR0_USE_FPU_MASK : 0);
+
+    env->pvr.regs[2] |= (cpu->cfg.usefpu ? PVR2_USE_FPU_MASK : 0) |
+                        (cpu->cfg.usefpu > 1 ? PVR2_USE_FPU2_MASK : 0);
 
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -164,6 +166,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
                      true),
+    DEFINE_PROP_UINT8("use-fpu", MicroBlazeCPU, cfg.usefpu, 2),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/translate.c +3 -3

index 19faf40..7383d4d 100644
@@ -1415,11 +1415,11 @@ static int dec_check_fpuv2(DisasContext *dc)
 
     r = dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU2_MASK;
 
-    if (!r && (dc->tb_flags & MSR_EE_FLAG)) {
+    if ((dc->cpu->cfg.usefpu != 2) && (dc->tb_flags & MSR_EE_FLAG)) {
         tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_FPU);
         t_gen_raise_exception(dc, EXCP_HW_EXCP);
     }
-    return r;
+    return (dc->cpu->cfg.usefpu == 2) ? 0 : PVR2_USE_FPU2_MASK;
 }
 
 static void dec_fpu(DisasContext *dc)

@@ -1428,7 +1428,7 @@ static void dec_fpu(DisasContext *dc)
 
     if ((dc->tb_flags & MSR_EE_FLAG)
           && (dc->cpu->env.pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)
-          && !((dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU_MASK))) {
+          && (dc->cpu->cfg.usefpu != 1)) {
         tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_ILLEGAL_OP);
         t_gen_raise_exception(dc, EXCP_HW_EXCP);
         return;

Signature · 1 line

1.7.1

Peter Crosthwaite

Reviewed-by on v1 5/5

2015-05-25 04:34 UTC #14 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 5/5] target-microblaze: Convert use-fpu to a CPU property

Quoted 85 lines · Alistair Francis

Originally the use-fpu PVR bits were manually set for each machine. This is a hassle and difficult to read, instead set them based on the CPU properties.

Signed-off-by Alistair Francis <[email protected]>

---
Changes since RFC:
- Tidy up the if logic

 hw/microblaze/petalogix_ml605_mmu.c |    4 ++--
 target-microblaze/cpu-qom.h         |    1 +
 target-microblaze/cpu.c             |    9 ++++++---
 target-microblaze/translate.c       |    6 +++---
 4 files changed, 12 insertions(+), 8 deletions(-)

hw/microblaze/petalogix_ml605_mmu.c +2 -2

index 48c264b..a93ca51 100644
@@ -71,9 +71,8 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu)
     env->pvr.regs[10] = 0x0e000000; /* virtex 6 */
     /* setup pvr to match kernel setting */
     env->pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK;
-    env->pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI;
+    env->pvr.regs[0] |= PVR0_ENDI;
     env->pvr.regs[0] = (env->pvr.regs[0] & ~PVR0_VERSION_MASK) | (0x14 << 8);
-    env->pvr.regs[2] ^= PVR2_USE_FPU2_MASK;
     env->pvr.regs[4] = 0xc56b8000;
     env->pvr.regs[5] = 0xc56be000;
 }

@@ -95,6 +94,7 @@ petalogix_ml605_init(MachineState *machine)
 
     /* init CPUs */
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+    object_property_set_int(OBJECT(cpu), 1, "use-fpu", &error_abort);
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */

target-microblaze/cpu-qom.h +1 -0

index 750ff3b..f3312f8 100644
@@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU {
     struct {
         bool stackproc;
         uint32_t base_vectors;
+        uint8_t usefpu;
     } cfg;
 
     CPUMBState env;

target-microblaze/cpu.c +6 -3

index c7ad5d5..ce40a3f 100644
@@ -113,12 +113,14 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_DIV_MASK \
                         | PVR2_USE_HW_MUL_MASK \
                         | PVR2_USE_MUL64_MASK \
-                        | PVR2_USE_FPU_MASK \
-                        | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
 
-    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0) |
+                        (cpu->cfg.usefpu ? PVR0_USE_FPU_MASK : 0);
+
+    env->pvr.regs[2] |= (cpu->cfg.usefpu ? PVR2_USE_FPU_MASK : 0) |
+                        (cpu->cfg.usefpu > 1 ? PVR2_USE_FPU2_MASK : 0);
 
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -164,6 +166,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
                      true),
+    DEFINE_PROP_UINT8("use-fpu", MicroBlazeCPU, cfg.usefpu, 2),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/translate.c +0 -0

index 19faf40..7383d4d 100644

target-microblaze/translate.c static int dec_check_fpuv2(DisasContext *dc) · line 1417

Quoted 3 lines

target-microblaze/translate.c +0 -0

@@ -1415,11 +1415,11 @@ static int dec_check_fpuv2(DisasContext *dc)
 
     r = dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU2_MASK;

r is unused, so I think this and the decl. has to be deleted.

Otherwise:

Reviewed-by Peter Crosthwaite <[email protected]>

Regards,
Peter

Quoted 23 lines

target-microblaze/translate.c +3 -4

-    if (!r && (dc->tb_flags & MSR_EE_FLAG)) {
+    if ((dc->cpu->cfg.usefpu != 2) && (dc->tb_flags & MSR_EE_FLAG)) {
         tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_FPU);
         t_gen_raise_exception(dc, EXCP_HW_EXCP);
     }
-    return r;
+    return (dc->cpu->cfg.usefpu == 2) ? 0 : PVR2_USE_FPU2_MASK;
 }
 
 static void dec_fpu(DisasContext *dc)

@@ -1428,7 +1428,7 @@ static void dec_fpu(DisasContext *dc)
 
     if ((dc->tb_flags & MSR_EE_FLAG)
           && (dc->cpu->env.pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)
-          && !((dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU_MASK))) {
+          && (dc->cpu->cfg.usefpu != 1)) {
         tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_ILLEGAL_OP);
         t_gen_raise_exception(dc, EXCP_HW_EXCP);
         return;
--

1.7.1

Alistair Francis

author

2015-05-28 01:02 UTC #15 lore raw

Re: [Qemu-devel] [PATCH RESEND v1 5/5] target-microblaze: Convert use-fpu to a CPU property

Quoted 88 lines · Peter Crosthwaite

Quoted 86 lines · Alistair Francis

Originally the use-fpu PVR bits were manually set for each machine. This is a hassle and difficult to read, instead set them based on the CPU properties.

Signed-off-by Alistair Francis <[email protected]>

---
Changes since RFC:
- Tidy up the if logic

 hw/microblaze/petalogix_ml605_mmu.c |    4 ++--
 target-microblaze/cpu-qom.h         |    1 +
 target-microblaze/cpu.c             |    9 ++++++---
 target-microblaze/translate.c       |    6 +++---
 4 files changed, 12 insertions(+), 8 deletions(-)

hw/microblaze/petalogix_ml605_mmu.c +2 -2

index 48c264b..a93ca51 100644
@@ -71,9 +71,8 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu)
     env->pvr.regs[10] = 0x0e000000; /* virtex 6 */
     /* setup pvr to match kernel setting */
     env->pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK;
-    env->pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI;
+    env->pvr.regs[0] |= PVR0_ENDI;
     env->pvr.regs[0] = (env->pvr.regs[0] & ~PVR0_VERSION_MASK) | (0x14 << 8);
-    env->pvr.regs[2] ^= PVR2_USE_FPU2_MASK;
     env->pvr.regs[4] = 0xc56b8000;
     env->pvr.regs[5] = 0xc56be000;
 }

@@ -95,6 +94,7 @@ petalogix_ml605_init(MachineState *machine)
 
     /* init CPUs */
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+    object_property_set_int(OBJECT(cpu), 1, "use-fpu", &error_abort);
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */

target-microblaze/cpu-qom.h +1 -0

index 750ff3b..f3312f8 100644
@@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU {
     struct {
         bool stackproc;
         uint32_t base_vectors;
+        uint8_t usefpu;
     } cfg;
 
     CPUMBState env;

target-microblaze/cpu.c +6 -3

index c7ad5d5..ce40a3f 100644
@@ -113,12 +113,14 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
                         | PVR2_USE_DIV_MASK \
                         | PVR2_USE_HW_MUL_MASK \
                         | PVR2_USE_MUL64_MASK \
-                        | PVR2_USE_FPU_MASK \
-                        | PVR2_USE_FPU2_MASK \
                         | PVR2_FPU_EXC_MASK \
                         | 0;
 
-    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0);
+    env->pvr.regs[0] |= (cpu->cfg.stackproc ? PVR0_SPROT_MASK : 0) |
+                        (cpu->cfg.usefpu ? PVR0_USE_FPU_MASK : 0);
+
+    env->pvr.regs[2] |= (cpu->cfg.usefpu ? PVR2_USE_FPU_MASK : 0) |
+                        (cpu->cfg.usefpu > 1 ? PVR2_USE_FPU2_MASK : 0);
 
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);

@@ -164,6 +166,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackproc,
                      true),
+    DEFINE_PROP_UINT8("use-fpu", MicroBlazeCPU, cfg.usefpu, 2),
     DEFINE_PROP_END_OF_LIST(),
 };

target-microblaze/translate.c +0 -0

index 19faf40..7383d4d 100644
@@ -1415,11 +1415,11 @@ static int dec_check_fpuv2(DisasContext *dc)

Quoted 4 lines

target-microblaze/translate.c static int dec_check_fpuv2(DisasContext *dc) · line 1417
Quoted 2 lines
target-microblaze/translate.c +0 -0
     r = dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU2_MASK;
 
r is unused, so I think this and the decl. has to be deleted.

Good catch, I completely missed that.

Quoted 4 lines

Otherwise:
Reviewed-by Peter Crosthwaite <[email protected]>

Thanks

Alistair

Quoted 28 lines

Regards,
Peter

Quoted 23 lines

target-microblaze/translate.c +3 -4

-    if (!r && (dc->tb_flags & MSR_EE_FLAG)) {
+    if ((dc->cpu->cfg.usefpu != 2) && (dc->tb_flags & MSR_EE_FLAG)) {
         tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_FPU);
         t_gen_raise_exception(dc, EXCP_HW_EXCP);
     }
-    return r;
+    return (dc->cpu->cfg.usefpu == 2) ? 0 : PVR2_USE_FPU2_MASK;
 }
 
 static void dec_fpu(DisasContext *dc)

@@ -1428,7 +1428,7 @@ static void dec_fpu(DisasContext *dc)
 
     if ((dc->tb_flags & MSR_EE_FLAG)
           && (dc->cpu->env.pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)
-          && !((dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU_MASK))) {
+          && (dc->cpu->cfg.usefpu != 1)) {
         tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_ILLEGAL_OP);
         t_gen_raise_exception(dc, EXCP_HW_EXCP);
         return;
--

1.7.1

Alistair Francis

author

2015-05-18 23:16 UTC #16 lore raw

In reply to Alistair Francis

Quoted 3 lines · Alistair Francis

Firstly this patch series tidies up some code and removes a "xlnx." prefix.

Sorry about the spam. Not all of the emails went through the first time, so I swapped SMTP servers and am re-sending.

Thanks,

Alistair

Quoted 39 lines

Then it moves the Microblaze PVR registers to the end of the CPUMBState to preserve them during reset. This allows most of the operations on them to be moved from the reset to the realise. Except for the machine specific ones which will be moved when the other properties are converted across to standard QEMU props, after this patch series is accepted (either merged or the method I'm using is approved). See the individual commit for more details.
Next it adds the "use-stack-protection" property to the Microblaze CPU, which allows stack protection to be disabled.
It also converts the previously hardcoded method of enabling the FPU to use standard QEMU properties. This simplifies the logic in the target-microblaze translate.c.
Changes since RFC:
- Preserve the PVR registers during resets
- Move most of the logic into realise functions
- Small name and function changes
Alistair Francis (5):
target-microblaze: Fix up indentation
target-microblaze: Preserve the pvr registers during reset
target-microblaze: Allow the stack protection to be disabled
target-microblaze: Tidy up the base-vectors property
target-microblaze: Convert use-fpu to a CPU property
 hw/microblaze/petalogix_ml605_mmu.c |    4 +-
 target-microblaze/cpu-qom.h         |    9 +++++-
 target-microblaze/cpu.c             |   59 ++++++++++++++++++++++-------------
 target-microblaze/cpu.h             |   11 ++++--
 target-microblaze/helper.c          |    8 ++--
 target-microblaze/op_helper.c       |   10 +++---
 target-microblaze/translate.c       |    8 ++--
 7 files changed, 67 insertions(+), 42 deletions(-)