[Qemu-devel] [PATCH v2] microblaze: Add internal base vectors reg

[Qemu-devel] [PATCH v2] microblaze: Add internal base vectors reg

[edgar.iglesias]

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(-)

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

Re: [Qemu-devel] [PATCH v2] microblaze: Add internal base vectors reg

[Peter Crosthwaite]

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

Re: [Qemu-devel] [PATCH v2] microblaze: Add internal base vectors reg

[Andreas Färber]

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;
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2] microblaze: Add internal base vectors reg

[Edgar E. Iglesias]

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