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

[Peter Crosthwaite]

On Mon, May 18, 2015 at 4:13 PM, Alistair Francis
<address@hidden> wrote:
> 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 <address@hidden>
> ---
>
> 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(-)
>
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 67e3182..555bc4c 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -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);

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.

> +    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 <address@hidden>

Regards,
Peter

>      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);
>  }
> diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
> index 4ea04ac..e4c1cde 100644
> --- a/target-microblaze/cpu.h
> +++ b/target-microblaze/cpu.h
> @@ -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
>
>