[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RESEND v1 2/5] target-microblaze: Preserve the p
From: |
Alistair Francis |
Subject: |
Re: [Qemu-devel] [PATCH RESEND v1 2/5] target-microblaze: Preserve the pvr registers during reset |
Date: |
Thu, 28 May 2015 10:47:08 +1000 |
On Mon, May 25, 2015 at 1:53 PM, Peter Crosthwaite
<address@hidden> wrote:
> 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.
Yeah, it really struggled with this patch for some reason.
I can still remove them from this patch. I'm touching it anyway
>
>> + 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.
Same with this.
>
> Reviewed-by: Peter Crosthwaite <address@hidden>
Thanks
Alistair
>
> 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
>>
>>
>