Re: [Qemu-devel] [PATCH v3 31/35] microblaze: enable multi-arch

[Paolo Bonzini <pbonzini@redhat.com>]

On 18/07/2015 11:40, Peter Crosthwaite wrote:
> Multi-arch conversion consisting of:
>  * Compiling out all target-microblaze private contents of cpu.h
>    when doing multi-arch build
>  * Defining the QOM cpu hooks
>  * Add microblazeel to multi-support list
> 
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> I guess I could split to multi patches but it will bloat this series!
> 
> Changed since RFCv2:
> Remove macro undefs (obsoleted)
> Remove arch prefixing redefinitions of cpu-defs symbols (obsoleted)
> Compile out SR_PC in multi-arch
> Remove configury changes
> Remove arch-obj conversions
> ---
>  multi-support/microblazeel      |  0
>  target-microblaze/Makefile.objs |  6 +++---
>  target-microblaze/cpu-qom.h     |  2 ++
>  target-microblaze/cpu.c         |  2 ++
>  target-microblaze/cpu.h         | 28 ++++++++++++++++++++++++----
>  5 files changed, 31 insertions(+), 7 deletions(-)
>  create mode 100644 multi-support/microblazeel
> 
> diff --git a/multi-support/microblazeel b/multi-support/microblazeel
> new file mode 100644
> index 0000000..e69de29
> diff --git a/target-microblaze/Makefile.objs b/target-microblaze/Makefile.objs
> index f3d7b44..f70163d 100644
> --- a/target-microblaze/Makefile.objs
> +++ b/target-microblaze/Makefile.objs
> @@ -1,3 +1,3 @@
> -obj-y += translate.o op_helper.o helper.o cpu.o
> -obj-y += gdbstub.o
> -obj-$(CONFIG_SOFTMMU) += mmu.o
> +arch-obj-y += translate.o op_helper.o helper.o cpu.o
> +arch-obj-y += gdbstub.o
> +arch-obj-$(CONFIG_SOFTMMU) += mmu.o
> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
> index 34f6273..783095c 100644
> --- a/target-microblaze/cpu-qom.h
> +++ b/target-microblaze/cpu-qom.h
> @@ -79,9 +79,11 @@ static inline MicroBlazeCPU *mb_env_get_cpu(CPUMBState *env)
>      return container_of(env, MicroBlazeCPU, env);
>  }
>  
> +#ifndef TARGET_MULTI
>  #define ENV_GET_CPU(e) CPU(mb_env_get_cpu(e))
>  
>  #define ENV_OFFSET offsetof(MicroBlazeCPU, env)
> +#endif /* !TARGET_MULTI */

Would it make sense to split the headers in two instead of adding
#ifndefs?  In other words, move everything outside the #ifndef to
cpu-qom.h, and everything inside it to cpu.h.  Then include cpu-qom.h
only from the hw/ files.

Paolo

>  void mb_cpu_do_interrupt(CPUState *cs);
>  bool mb_cpu_exec_interrupt(CPUState *cs, int int_req);
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 9ac509a..d460f69 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -25,6 +25,7 @@
>  #include "qemu-common.h"
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
> +#include "tcg/tcg.h"
>  
>  static const struct {
>      const char *name;
> @@ -195,6 +196,7 @@ static void mb_cpu_initfn(Object *obj)
>      CPUMBState *env = &cpu->env;
>      static bool tcg_initialized;
>  
> +    CPU_SET_QOM_HOOKS(cs);
>      cs->env_ptr = env;
>      cpu_exec_init(cs, &error_abort);
>  
> diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
> index 9bac856..c3085d5 100644
> --- a/target-microblaze/cpu.h
> +++ b/target-microblaze/cpu.h
> @@ -35,6 +35,8 @@ typedef struct CPUMBState CPUMBState;
>  #include "mmu.h"
>  #endif
>  
> +#ifndef TARGET_MULTI
> +
>  #define EXCP_MMU        1
>  #define EXCP_IRQ        2
>  #define EXCP_BREAK      3
> @@ -44,13 +46,17 @@ typedef struct CPUMBState CPUMBState;
>  /* MicroBlaze-specific interrupt pending bits.  */
>  #define CPU_INTERRUPT_NMI       CPU_INTERRUPT_TGT_EXT_3
>  
> +#endif /* TARGET_MULTI */
> +
>  /* Meanings of the MBCPU object's two inbound GPIO lines */
>  #define MB_CPU_IRQ 0
>  #define MB_CPU_FIR 1
>  
> -/* Register aliases. R0 - R15 */
> -#define R_SP     1
> +#ifndef TARGET_MULTI
> +
> +/* Register aliases. R1 - R15 */
>  #define SR_PC    0
> +#define R_SP     1
>  #define SR_MSR   1
>  #define SR_EAR   3
>  #define SR_ESR   5
> @@ -111,6 +117,11 @@ typedef struct CPUMBState CPUMBState;
>  #define FSR_UF          (1<<1) /* Underflow */
>  #define FSR_DO          (1<<0) /* Denormalized operand error */
>  
> +/* The Microblaze bootloader configures some of the PVRs in a board specific
> + * way as a reset process. This should go away with PVR property QOMification
> + * and then the PVRs can be made private to CPUs.
> + */
> +
>  /* Version reg.  */
>  /* Basic PVR mask */
>  #define PVR0_PVR_FULL_MASK              0x80000000
> @@ -211,10 +222,14 @@ typedef struct CPUMBState CPUMBState;
>  /* MSR Reset value PVR mask */
>  #define PVR11_MSR_RESET_VALUE_MASK      0x000007FF
>  
> +#endif /* TARGET_MULTI */
> +
>  #define C_PVR_NONE                      0
>  #define C_PVR_BASIC                     1
>  #define C_PVR_FULL                      2
>  
> +#ifndef TARGET_MULTI
> +
>  /* CPU flags.  */
>  
>  /* Condition codes.  */
> @@ -225,14 +240,16 @@ typedef struct CPUMBState CPUMBState;
>  #define CC_NE  1
>  #define CC_EQ  0
>  
> -#define NB_MMU_MODES    3
> -
>  #define STREAM_EXCEPTION (1 << 0)
>  #define STREAM_ATOMIC    (1 << 1)
>  #define STREAM_TEST      (1 << 2)
>  #define STREAM_CONTROL   (1 << 3)
>  #define STREAM_NONBLOCK  (1 << 4)
>  
> +#endif /* TARGET_MULTI */
> +
> +#define NB_MMU_MODES    3
> +
>  struct CPUMBState {
>      uint32_t debug;
>      uint32_t btaken;
> @@ -278,6 +295,8 @@ struct CPUMBState {
>  
>  #include "cpu-qom.h"
>  
> +#ifndef TARGET_MULTI
> +
>  void mb_tcg_init(void);
>  MicroBlazeCPU *cpu_mb_init(const char *cpu_model);
>  /* you can call this signal handler from your SIGBUS and SIGSEGV
> @@ -339,4 +358,5 @@ void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
>  
>  #include "exec/exec-all.h"
>  
> +#endif /* !TARGET_MULTI */
>  #endif
>