Skip to contentSkip to conversation

Add Microblaze configuration options

PATCH Superseded Alistair Francis 2 participants 15 messages May 28, 2015 – May 29, 2015

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 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.

V2:
- Small fixes
- Disable stack protection by default
- Remove the memset and cpu_reset functions from the MB realise
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/enabled
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             |   56 +++++++++++++++++++++-------------
 target-microblaze/cpu.h             |   11 ++++--
 target-microblaze/helper.c          |    8 ++--
 target-microblaze/op_helper.c       |   10 +++---
 target-microblaze/translate.c       |   14 +++-----
 7 files changed, 65 insertions(+), 47 deletions(-)
#1 lore raw

PATCH v2 1/5 target-microblaze: Fix up indentation

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

---

 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
Reviewed-by on v2 1/5
#2 lore raw

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

Quoted 4 lines · Alistair Francis

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

Quoted 29 lines

---

 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
#3 lore raw

PATCH v2 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.

---
V2:
- Remove the memset and cpu_reset functions as they aren't required in the realize and I'm touching them anyway.

NOTE: The individual 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 |   40 ++++++++++++++++++++++------------------
 target-microblaze/cpu.h |   10 ++++++----
 2 files changed, 28 insertions(+), 22 deletions(-)
target-microblaze/cpu.c +22 -18
index 67e3182..95be540 100644
@@ -63,13 +63,34 @@ 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;
+
+    qemu_init_vcpu(cs);
+
     env->pvr.regs[0] = PVR0_PVR_FULL_MASK \
                        | PVR0_USE_BARREL_MASK \
                        | PVR0_USE_DIV_MASK \
@@ -99,25 +120,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
Reviewed-by on v2 2/5
#4 lore raw

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

Quoted 9 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.

Quoted 111 lines

---
V2:
- Remove the memset and cpu_reset functions as they aren't required in the realize and I'm touching them anyway.

NOTE: The individual 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 |   40 ++++++++++++++++++++++------------------
 target-microblaze/cpu.h |   10 ++++++----
 2 files changed, 28 insertions(+), 22 deletions(-)
target-microblaze/cpu.c +22 -18
index 67e3182..95be540 100644
@@ -63,13 +63,34 @@ 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;
+
+    qemu_init_vcpu(cs);
+
     env->pvr.regs[0] = PVR0_PVR_FULL_MASK \
                        | PVR0_USE_BARREL_MASK \
                        | PVR0_USE_DIV_MASK \
@@ -99,25 +120,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
#5 lore raw

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

In reply to Alistair Francis

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

The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is.

---
V2:
- Change the variable name to stackprot
- Include protection for the second time stack protection is enabled
- Disable stack protection by default
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 |    4 ++--
 4 files changed, 13 insertions(+), 2 deletions(-)
target-microblaze/cpu-qom.h +5 -0
index e3e0701..e08adb9 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackprot;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
target-microblaze/cpu.c +5 -0
index 95be540..ead2fcd 100644
@@ -114,6 +114,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.stackprot ? PVR0_SPROT_MASK : 0);
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
@@ -156,6 +159,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.stackprot,
+                     false),
     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 +2 -2
index 4068946..bd10b40 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.stackprot) {
         stackprot = 1;
     }
 
@@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
             return &cpu_R[dc->ra];
         }
 
-        if (dc->rb == 1) {
+        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
             stackprot = 1;
         }
 
Signature · 1 line
1.7.1
#6 lore raw

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

Quoted 9 lines · Alistair Francis

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

The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is.

Hi Alistair,

Quoted 37 lines

---
V2:
- Change the variable name to stackprot
- Include protection for the second time stack protection is enabled
- Disable stack protection by default
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 |    4 ++--
 4 files changed, 13 insertions(+), 2 deletions(-)
target-microblaze/cpu-qom.h +5 -0
index e3e0701..e08adb9 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackprot;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
target-microblaze/cpu.c +0 -0
index 95be540..ead2fcd 100644

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

Quoted 5 lines
target-microblaze/cpu.c +2 -0
@@ -114,6 +114,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.stackprot ? PVR0_SPROT_MASK : 0);

Could you please skip the parentheses.

target-microblaze/cpu.c static const VMStateDescription vmstate_mb_cpu = { · line 163

Quoted 10 lines
target-microblaze/cpu.c +3 -0
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
@@ -156,6 +159,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.stackprot,
+                     false),

I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU.

Quoted 40 lines
target-microblaze/cpu.c +0 -0
     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 +2 -2
index 4068946..bd10b40 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.stackprot) {
         stackprot = 1;
     }
 
@@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
             return &cpu_R[dc->ra];
         }
 
-        if (dc->rb == 1) {
+        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
             stackprot = 1;
         }
 
Signature · 1 line
1.7.1
#7 lore raw

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

Quoted 46 lines · Edgar E. Iglesias
Quoted 9 lines · Alistair Francis

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

The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is.

Hi Alistair,

Quoted 32 lines

---
V2:
- Change the variable name to stackprot
- Include protection for the second time stack protection is enabled
- Disable stack protection by default
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 |    4 ++--
 4 files changed, 13 insertions(+), 2 deletions(-)
target-microblaze/cpu-qom.h +5 -0
index e3e0701..e08adb9 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackprot;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
Quoted 12 lines

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

Quoted 10 lines
target-microblaze/cpu.c +2 -0
index 95be540..ead2fcd 100644
@@ -114,6 +114,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.stackprot ? PVR0_SPROT_MASK : 0);

Could you please skip the parentheses.

Hey Edgar,

Yeah I will. They get re-added straight away, which is why I left them in.

Quoted 7 lines
Quoted 7 lines
target-microblaze/cpu.c +1 -0
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
@@ -156,6 +159,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
Quoted 7 lines

target-microblaze/cpu.c static const VMStateDescription vmstate_mb_cpu = { · line 163

Quoted 4 lines
target-microblaze/cpu.c +2 -0
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
+                     false),

I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU.

Fair enough. I will leave it here and add a patch at the end that disables it.

Thanks,

Alistair

Quoted 45 lines
Quoted 44 lines
target-microblaze/cpu.c +0 -0
     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 +2 -3
index 4068946..bd10b40 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.stackprot) {
         stackprot = 1;
     }
 
@@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
             return &cpu_R[dc->ra];
         }
 
-        if (dc->rb == 1) {
+        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
             stackprot = 1;
         }
 
--

1.7.1

#8 lore raw

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

Quoted 80 lines · Alistair Francis
Quoted 46 lines · Edgar E. Iglesias
Quoted 9 lines · Alistair Francis

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

The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is.

Hi Alistair,

Quoted 32 lines

---
V2:
- Change the variable name to stackprot
- Include protection for the second time stack protection is enabled
- Disable stack protection by default
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 |    4 ++--
 4 files changed, 13 insertions(+), 2 deletions(-)
target-microblaze/cpu-qom.h +5 -0
index e3e0701..e08adb9 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackprot;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
Quoted 12 lines

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

Quoted 10 lines
target-microblaze/cpu.c +2 -0
index 95be540..ead2fcd 100644
@@ -114,6 +114,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.stackprot ? PVR0_SPROT_MASK : 0);

Could you please skip the parentheses.

Hey Edgar,

Yeah I will. They get re-added straight away, which is why I left them in.

Quoted 7 lines
Quoted 7 lines
target-microblaze/cpu.c +1 -0
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
@@ -156,6 +159,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
Quoted 7 lines

target-microblaze/cpu.c static const VMStateDescription vmstate_mb_cpu = { · line 163

Quoted 4 lines
target-microblaze/cpu.c +2 -0
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
+                     false),

I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU.

Quoted 1 line

Fair enough. I will leave it here and add a patch at the end that disables it.

On that note, should the current machines enable it?

It has always been enabled for them, but they do support MMU's so it should be disabled.

Thanks,

Alistair

Quoted 50 lines

Thanks,

Alistair

Quoted 45 lines
Quoted 44 lines
target-microblaze/cpu.c +0 -0
     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 +2 -3
index 4068946..bd10b40 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.stackprot) {
         stackprot = 1;
     }
 
@@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
             return &cpu_R[dc->ra];
         }
 
-        if (dc->rb == 1) {
+        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
             stackprot = 1;
         }
 
--

1.7.1

#9 lore raw

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

Quoted 82 lines · Alistair Francis
Quoted 80 lines · Alistair Francis
Quoted 46 lines · Edgar E. Iglesias
Quoted 9 lines · Alistair Francis

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

The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is.

Hi Alistair,

Quoted 32 lines

---
V2:
- Change the variable name to stackprot
- Include protection for the second time stack protection is enabled
- Disable stack protection by default
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 |    4 ++--
 4 files changed, 13 insertions(+), 2 deletions(-)
target-microblaze/cpu-qom.h +5 -0
index e3e0701..e08adb9 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackprot;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
Quoted 12 lines

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

Quoted 10 lines
target-microblaze/cpu.c +2 -0
index 95be540..ead2fcd 100644
@@ -114,6 +114,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.stackprot ? PVR0_SPROT_MASK : 0);

Could you please skip the parentheses.

Hey Edgar,

Yeah I will. They get re-added straight away, which is why I left them in.

Quoted 14 lines

target-microblaze/cpu.c static const VMStateDescription vmstate_mb_cpu = { · line 163

Quoted 11 lines
target-microblaze/cpu.c +3 -0
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
@@ -156,6 +159,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.stackprot,
+                     false),

I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU.

Quoted 6 lines
Quoted 1 line

Fair enough. I will leave it here and add a patch at the end that disables it.

On that note, should the current machines enable it?

It has always been enabled for them, but they do support MMU's so it should be disabled.

Right, stackprot should be disabled as the MMU is on.
I wasn't aware that the stackprot was not used in combination with the MMU at the time...

Cheers,
Edgar

Quoted 55 lines

Thanks,

Alistair

Quoted 50 lines

Thanks,

Alistair

Quoted 45 lines
Quoted 44 lines
target-microblaze/cpu.c +0 -0
     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 +2 -3
index 4068946..bd10b40 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.stackprot) {
         stackprot = 1;
     }
 
@@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
             return &cpu_R[dc->ra];
         }
 
-        if (dc->rb == 1) {
+        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
             stackprot = 1;
         }
 
--

1.7.1

#10 lore raw

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

Quoted 87 lines · Edgar E. Iglesias
Quoted 86 lines · Alistair Francis
Quoted 80 lines · Alistair Francis
Quoted 46 lines · Edgar E. Iglesias
Quoted 9 lines · Alistair Francis

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

The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is.

Hi Alistair,

Quoted 32 lines

---
V2:
- Change the variable name to stackprot
- Include protection for the second time stack protection is enabled
- Disable stack protection by default
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 |    4 ++--
 4 files changed, 13 insertions(+), 2 deletions(-)
target-microblaze/cpu-qom.h +5 -0
index e3e0701..e08adb9 100644
@@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackprot;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
Quoted 12 lines

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

Quoted 10 lines
target-microblaze/cpu.c +2 -0
index 95be540..ead2fcd 100644
@@ -114,6 +114,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.stackprot ? PVR0_SPROT_MASK : 0);

Could you please skip the parentheses.

Hey Edgar,

Yeah I will. They get re-added straight away, which is why I left them in.

Quoted 7 lines
Quoted 7 lines
target-microblaze/cpu.c +1 -0
+
     env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
     env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
 
@@ -156,6 +159,8 @@ static const VMStateDescription vmstate_mb_cpu = {
 
Quoted 7 lines

target-microblaze/cpu.c static const VMStateDescription vmstate_mb_cpu = { · line 163

Quoted 4 lines
target-microblaze/cpu.c +2 -0
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
+                     false),

I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU.

Quoted 1 line

Fair enough. I will leave it here and add a patch at the end that disables it.

On that note, should the current machines enable it?

Quoted 7 lines
Quoted 2 lines

It has always been enabled for them, but they do support MMU's so it should be disabled.

Right, stackprot should be disabled as the MMU is on.
I wasn't aware that the stackprot was not used in combination with the MMU at the time...

Ok, I will disable it by default and for all of the current machines.

Thanks,

Alistair

Quoted 61 lines

Cheers,
Edgar

Quoted 55 lines

Thanks,

Alistair

Quoted 50 lines

Thanks,

Alistair

Quoted 45 lines
Quoted 44 lines
target-microblaze/cpu.c +0 -0
     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 +2 -3
index 4068946..bd10b40 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.stackprot) {
         stackprot = 1;
     }
 
@@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
             return &cpu_R[dc->ra];
         }
 
-        if (dc->rb == 1) {
+        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
             stackprot = 1;
         }
 
--

1.7.1

#11 lore raw

PATCH v2 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.

---

 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 e08adb9..dd04199 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 stackprot;
+        uint32_t base_vectors;
     } cfg;
 
     CPUMBState env;
target-microblaze/cpu.c +2 -2
index ead2fcd..aefdd7a 100644
@@ -120,7 +120,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.  */
@@ -158,7 +158,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.stackprot,
                      false),
     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
#12 lore raw

PATCH v2 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.

---
V2:
- Remove unnecessary declaration of r
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       |   10 +++-------
 4 files changed, 12 insertions(+), 12 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 dd04199..a6474f9 100644
@@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU {
     struct {
         bool stackprot;
         uint32_t base_vectors;
+        uint8_t usefpu;
     } cfg;
 
     CPUMBState env;
target-microblaze/cpu.c +6 -3
index aefdd7a..20c4b9b 100644
@@ -110,12 +110,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.stackprot ? PVR0_SPROT_MASK : 0);
+    env->pvr.regs[0] |= (cpu->cfg.stackprot ? 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);
@@ -161,6 +163,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
                      false),
+    DEFINE_PROP_UINT8("use-fpu", MicroBlazeCPU, cfg.usefpu, 2),
     DEFINE_PROP_END_OF_LIST(),
 };
 
target-microblaze/translate.c +3 -7
index bd10b40..8187700 100644
@@ -1411,15 +1411,11 @@ static void dec_rts(DisasContext *dc)
 
 static int dec_check_fpuv2(DisasContext *dc)
 {
-    int r;
-
-    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 +1424,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
#13 lore raw

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

Quoted 34 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.

---
V2:
- Remove unnecessary declaration of r
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       |   10 +++-------
 4 files changed, 12 insertions(+), 12 deletions(-)
hw/microblaze/petalogix_ml605_mmu.c +1 -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;
 }

hw/microblaze/petalogix_ml605_mmu.c petalogix_ml605_init(MachineState *machine) · line 97

Quoted 4 lines
hw/microblaze/petalogix_ml605_mmu.c +1 -0
@@ -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);

Can you please add a comment on the values of use-fpu?

Quoted 79 lines
hw/microblaze/petalogix_ml605_mmu.c +0 -0
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
target-microblaze/cpu-qom.h +1 -0
index dd04199..a6474f9 100644
@@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU {
     struct {
         bool stackprot;
         uint32_t base_vectors;
+        uint8_t usefpu;
     } cfg;
 
     CPUMBState env;
target-microblaze/cpu.c +6 -3
index aefdd7a..20c4b9b 100644
@@ -110,12 +110,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.stackprot ? PVR0_SPROT_MASK : 0);
+    env->pvr.regs[0] |= (cpu->cfg.stackprot ? 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);
@@ -161,6 +163,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
                      false),
+    DEFINE_PROP_UINT8("use-fpu", MicroBlazeCPU, cfg.usefpu, 2),
     DEFINE_PROP_END_OF_LIST(),
 };
 
target-microblaze/translate.c +3 -7
index bd10b40..8187700 100644
@@ -1411,15 +1411,11 @@ static void dec_rts(DisasContext *dc)
 
 static int dec_check_fpuv2(DisasContext *dc)
 {
-    int r;
-
-    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 +1424,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
#14 lore raw

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

Quoted 36 lines · Edgar E. Iglesias
Quoted 35 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.

---
V2:
- Remove unnecessary declaration of r
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       |   10 +++-------
 4 files changed, 12 insertions(+), 12 deletions(-)
hw/microblaze/petalogix_ml605_mmu.c +1 -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)
 
Quoted 6 lines

hw/microblaze/petalogix_ml605_mmu.c petalogix_ml605_init(MachineState *machine) · line 97

Quoted 3 lines
hw/microblaze/petalogix_ml605_mmu.c +1 -0
     /* init CPUs */
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+    object_property_set_int(OBJECT(cpu), 1, "use-fpu", &error_abort);

Can you please add a comment on the values of use-fpu?

Ok, I will.

Thanks,

Alistair

Quoted 82 lines
Quoted 81 lines
hw/microblaze/petalogix_ml605_mmu.c +0 -0
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
target-microblaze/cpu-qom.h +1 -0
index dd04199..a6474f9 100644
@@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU {
     struct {
         bool stackprot;
         uint32_t base_vectors;
+        uint8_t usefpu;
     } cfg;
 
     CPUMBState env;
target-microblaze/cpu.c +6 -3
index aefdd7a..20c4b9b 100644
@@ -110,12 +110,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.stackprot ? PVR0_SPROT_MASK : 0);
+    env->pvr.regs[0] |= (cpu->cfg.stackprot ? 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);
@@ -161,6 +163,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
     DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
                      false),
+    DEFINE_PROP_UINT8("use-fpu", MicroBlazeCPU, cfg.usefpu, 2),
     DEFINE_PROP_END_OF_LIST(),
 };
 
target-microblaze/translate.c +3 -8
index bd10b40..8187700 100644
@@ -1411,15 +1411,11 @@ static void dec_rts(DisasContext *dc)
 
 static int dec_check_fpuv2(DisasContext *dc)
 {
-    int r;
-
-    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 +1424,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