Patchwork [v2] microblaze: Add internal base vectors reg

login
register
mail settings
Submitter Edgar Iglesias
Date April 24, 2013, 11:31 a.m.
Message ID <1366803094-11619-1-git-send-email-edgar.iglesias@gmail.com>
Download mbox | patch
Permalink /patch/239166/
State New
Headers show

Comments

Edgar Iglesias - April 24, 2013, 11:31 a.m.
From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>

Configurable at CPU synthesis/instantiation.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---

Peter Crosthwaite commented off-list that it would be a good idea
to make this a device property. I agree but am a bit unsure of
how it should look. Any comments on this attempt?

v2:
Make base_vectors a CPU/device property.


 target-microblaze/cpu-qom.h |    1 +
 target-microblaze/cpu.c     |    8 ++++++++
 target-microblaze/helper.c  |    8 ++++----
 3 files changed, 13 insertions(+), 4 deletions(-)
Peter Crosthwaite - April 26, 2013, 2:06 a.m.
On Wed, Apr 24, 2013 at 9:31 PM,  <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
>
> Configurable at CPU synthesis/instantiation.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>
> Peter Crosthwaite commented off-list that it would be a good idea
> to make this a device property. I agree but am a bit unsure of
> how it should look. Any comments on this attempt?
>
> v2:
> Make base_vectors a CPU/device property.
>
>
>  target-microblaze/cpu-qom.h |    1 +
>  target-microblaze/cpu.c     |    8 ++++++++
>  target-microblaze/helper.c  |    8 ++++----
>  3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
> index aa51cf6..ce92a4e 100644
> --- a/target-microblaze/cpu-qom.h
> +++ b/target-microblaze/cpu-qom.h
> @@ -56,6 +56,7 @@ typedef struct MicroBlazeCPUClass {
>  typedef struct MicroBlazeCPU {
>      /*< private >*/
>      CPUState parent_obj;
> +    uint32_t base_vectors;
>      /*< public >*/
>
>      CPUMBState env;
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 0f4293d..404f82c 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -22,6 +22,7 @@
>
>  #include "cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
>
>
> @@ -119,6 +120,11 @@ static const VMStateDescription vmstate_mb_cpu = {
>      .unmigratable = 1,
>  };
>
> +static Property mb_properties[] = {
> +    DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void mb_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -133,6 +139,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
>
>      cc->do_interrupt = mb_cpu_do_interrupt;
>      dc->vmsd = &vmstate_mb_cpu;
> +
> +    dc->props = mb_properties;
>  }
>
>  static const TypeInfo mb_cpu_type_info = {
> diff --git a/target-microblaze/helper.c b/target-microblaze/helper.c
> index a0416d0..0dd669d 100644
> --- a/target-microblaze/helper.c
> +++ b/target-microblaze/helper.c
> @@ -152,7 +152,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
>                            env->sregs[SR_ESR], env->iflags);
>              log_cpu_state_mask(CPU_LOG_INT, env, 0);
>              env->iflags &= ~(IMM_FLAG | D_FLAG);
> -            env->sregs[SR_PC] = 0x20;
> +            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
>              break;
>
>          case EXCP_MMU:
> @@ -192,7 +192,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, env, 0);
>              env->iflags &= ~(IMM_FLAG | D_FLAG);
> -            env->sregs[SR_PC] = 0x20;
> +            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
>              break;
>
>          case EXCP_IRQ:
> @@ -233,7 +233,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
>              env->sregs[SR_MSR] |= t;
>
>              env->regs[14] = env->sregs[SR_PC];
> -            env->sregs[SR_PC] = 0x10;
> +            env->sregs[SR_PC] = cpu->base_vectors + 0x10;
>              //log_cpu_state_mask(CPU_LOG_INT, env, 0);
>              break;
>
> @@ -252,7 +252,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
>              if (env->exception_index == EXCP_HW_BREAK) {
>                  env->regs[16] = env->sregs[SR_PC];
>                  env->sregs[SR_MSR] |= MSR_BIP;
> -                env->sregs[SR_PC] = 0x18;
> +                env->sregs[SR_PC] = cpu->base_vectors + 0x18;
>              } else
>                  env->sregs[SR_PC] = env->btarget;
>              break;
> --
> 1.7.10.4
>
>
Andreas Färber - April 26, 2013, 11:48 a.m.
Hi,

Am 24.04.2013 13:31, schrieb edgar.iglesias@gmail.com:
> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> 
> Configurable at CPU synthesis/instantiation.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
> 
> Peter Crosthwaite commented off-list that it would be a good idea
> to make this a device property. I agree but am a bit unsure of
> how it should look. Any comments on this attempt?
> 
> v2:
> Make base_vectors a CPU/device property.

The patch looks good, expect that I don't see any machine or subclass
setting that property?

If you want to use it from the command line, you would either need to
add custom parsing code to -cpu, use -global or dive into experimenting
with -device <your-cpu-type>,xlnx.base-vectors=value. Me at least I have
been preparing this through code review and fixes but not yet actually
tried it for some QOM'ified target since x86 isn't yet.

Cheers,
Andreas

> 
> 
>  target-microblaze/cpu-qom.h |    1 +
>  target-microblaze/cpu.c     |    8 ++++++++
>  target-microblaze/helper.c  |    8 ++++----
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
> index aa51cf6..ce92a4e 100644
> --- a/target-microblaze/cpu-qom.h
> +++ b/target-microblaze/cpu-qom.h
> @@ -56,6 +56,7 @@ typedef struct MicroBlazeCPUClass {
>  typedef struct MicroBlazeCPU {
>      /*< private >*/
>      CPUState parent_obj;
> +    uint32_t base_vectors;
>      /*< public >*/
>  
>      CPUMBState env;
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 0f4293d..404f82c 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -22,6 +22,7 @@
>  
>  #include "cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
>  
>  
> @@ -119,6 +120,11 @@ static const VMStateDescription vmstate_mb_cpu = {
>      .unmigratable = 1,
>  };
>  
> +static Property mb_properties[] = {
> +    DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void mb_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -133,6 +139,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
>  
>      cc->do_interrupt = mb_cpu_do_interrupt;
>      dc->vmsd = &vmstate_mb_cpu;
> +
> +    dc->props = mb_properties;
>  }
>  
>  static const TypeInfo mb_cpu_type_info = {
> diff --git a/target-microblaze/helper.c b/target-microblaze/helper.c
> index a0416d0..0dd669d 100644
> --- a/target-microblaze/helper.c
> +++ b/target-microblaze/helper.c
> @@ -152,7 +152,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
>                            env->sregs[SR_ESR], env->iflags);
>              log_cpu_state_mask(CPU_LOG_INT, env, 0);
>              env->iflags &= ~(IMM_FLAG | D_FLAG);
> -            env->sregs[SR_PC] = 0x20;
> +            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
>              break;
>  
>          case EXCP_MMU:
> @@ -192,7 +192,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, env, 0);
>              env->iflags &= ~(IMM_FLAG | D_FLAG);
> -            env->sregs[SR_PC] = 0x20;
> +            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
>              break;
>  
>          case EXCP_IRQ:
> @@ -233,7 +233,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
>              env->sregs[SR_MSR] |= t;
>  
>              env->regs[14] = env->sregs[SR_PC];
> -            env->sregs[SR_PC] = 0x10;
> +            env->sregs[SR_PC] = cpu->base_vectors + 0x10;
>              //log_cpu_state_mask(CPU_LOG_INT, env, 0);
>              break;
>  
> @@ -252,7 +252,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
>              if (env->exception_index == EXCP_HW_BREAK) {
>                  env->regs[16] = env->sregs[SR_PC];
>                  env->sregs[SR_MSR] |= MSR_BIP;
> -                env->sregs[SR_PC] = 0x18;
> +                env->sregs[SR_PC] = cpu->base_vectors + 0x18;
>              } else
>                  env->sregs[SR_PC] = env->btarget;
>              break;
>
Edgar Iglesias - April 26, 2013, 12:11 p.m.
On Fri, Apr 26, 2013 at 01:48:45PM +0200, Andreas Färber wrote:
> Hi,
> 
> Am 24.04.2013 13:31, schrieb edgar.iglesias@gmail.com:
> > From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> > 
> > Configurable at CPU synthesis/instantiation.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > ---
> > 
> > Peter Crosthwaite commented off-list that it would be a good idea
> > to make this a device property. I agree but am a bit unsure of
> > how it should look. Any comments on this attempt?
> > 
> > v2:
> > Make base_vectors a CPU/device property.
> 
> The patch looks good, expect that I don't see any machine or subclass
> setting that property?
> 
> If you want to use it from the command line, you would either need to
> add custom parsing code to -cpu, use -global or dive into experimenting
> with -device <your-cpu-type>,xlnx.base-vectors=value. Me at least I have
> been preparing this through code review and fixes but not yet actually
> tried it for some QOM'ified target since x86 isn't yet.

Hi Andreas,

Right, the use of the property is currently out of tree.

The MicroBlaze core has tons of options that are set at synthesis time.
We currently support lots of variations in QEMU allthough only one or
two are used by machines in upstream. The CPU model looks at the version
regs (PVR) to check for features. Machines need to setup up PVRs according
to the CPU setup they emulate.

Now that CPUs can have qom props, we can make PVR regs optional and no
longer dictate the feature set.

Cheers,
Edgar

Patch

diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
index aa51cf6..ce92a4e 100644
--- a/target-microblaze/cpu-qom.h
+++ b/target-microblaze/cpu-qom.h
@@ -56,6 +56,7 @@  typedef struct MicroBlazeCPUClass {
 typedef struct MicroBlazeCPU {
     /*< private >*/
     CPUState parent_obj;
+    uint32_t base_vectors;
     /*< public >*/
 
     CPUMBState env;
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 0f4293d..404f82c 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -22,6 +22,7 @@ 
 
 #include "cpu.h"
 #include "qemu-common.h"
+#include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 
 
@@ -119,6 +120,11 @@  static const VMStateDescription vmstate_mb_cpu = {
     .unmigratable = 1,
 };
 
+static Property mb_properties[] = {
+    DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void mb_cpu_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -133,6 +139,8 @@  static void mb_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->do_interrupt = mb_cpu_do_interrupt;
     dc->vmsd = &vmstate_mb_cpu;
+
+    dc->props = mb_properties;
 }
 
 static const TypeInfo mb_cpu_type_info = {
diff --git a/target-microblaze/helper.c b/target-microblaze/helper.c
index a0416d0..0dd669d 100644
--- a/target-microblaze/helper.c
+++ b/target-microblaze/helper.c
@@ -152,7 +152,7 @@  void mb_cpu_do_interrupt(CPUState *cs)
                           env->sregs[SR_ESR], env->iflags);
             log_cpu_state_mask(CPU_LOG_INT, env, 0);
             env->iflags &= ~(IMM_FLAG | D_FLAG);
-            env->sregs[SR_PC] = 0x20;
+            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
             break;
 
         case EXCP_MMU:
@@ -192,7 +192,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, env, 0);
             env->iflags &= ~(IMM_FLAG | D_FLAG);
-            env->sregs[SR_PC] = 0x20;
+            env->sregs[SR_PC] = cpu->base_vectors + 0x20;
             break;
 
         case EXCP_IRQ:
@@ -233,7 +233,7 @@  void mb_cpu_do_interrupt(CPUState *cs)
             env->sregs[SR_MSR] |= t;
 
             env->regs[14] = env->sregs[SR_PC];
-            env->sregs[SR_PC] = 0x10;
+            env->sregs[SR_PC] = cpu->base_vectors + 0x10;
             //log_cpu_state_mask(CPU_LOG_INT, env, 0);
             break;
 
@@ -252,7 +252,7 @@  void mb_cpu_do_interrupt(CPUState *cs)
             if (env->exception_index == EXCP_HW_BREAK) {
                 env->regs[16] = env->sregs[SR_PC];
                 env->sregs[SR_MSR] |= MSR_BIP;
-                env->sregs[SR_PC] = 0x18;
+                env->sregs[SR_PC] = cpu->base_vectors + 0x18;
             } else
                 env->sregs[SR_PC] = env->btarget;
             break;