On Thu, May 14, 2015 at 11:36 PM, Alistair Francis
<alistair.fran...@xilinx.com> wrote:
> On Fri, May 15, 2015 at 4:32 PM, Peter Crosthwaite
> <peter.crosthwa...@xilinx.com> wrote:
>> On Thu, May 14, 2015 at 10:56 PM, Alistair Francis
>> <alistair.fran...@xilinx.com> wrote:
>>> On Fri, May 15, 2015 at 3:52 PM, Peter Crosthwaite
>>> <peter.crosthwa...@xilinx.com> wrote:
>>>> On Thu, May 14, 2015 at 10:48 PM, Alistair Francis
>>>> <alistair.fran...@xilinx.com> wrote:
>>>>> On Fri, May 15, 2015 at 3:22 PM, Peter Crosthwaite
>>>>> <peter.crosthwa...@xilinx.com> wrote:
>>>>>> On Wed, May 13, 2015 at 11:08 PM, Alistair Francis
>>>>>> <alistair.fran...@xilinx.com> wrote:
>>>>>>> Originally the use-fpu PVR bits were manually set for each machine. This
>>>>>>> is a hassle and difficult to read, instead set them based on the CPU
>>>>>>> properties.
>>>>>>>
>>>>>>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
>>>>>>> ---
>>>>>>>
>>>>>>> hw/microblaze/petalogix_ml605_mmu.c | 4 ++--
>>>>>>> target-microblaze/cpu-qom.h | 1 +
>>>>>>> target-microblaze/cpu.c | 13 +++++++++++--
>>>>>>> target-microblaze/translate.c | 6 +++---
>>>>>>> 4 files changed, 17 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/hw/microblaze/petalogix_ml605_mmu.c
>>>>>>> b/hw/microblaze/petalogix_ml605_mmu.c
>>>>>>> index 48c264b..f4e1cc5 100644
>>>>>>> --- a/hw/microblaze/petalogix_ml605_mmu.c
>>>>>>> +++ b/hw/microblaze/petalogix_ml605_mmu.c
>>>>>>> @@ -71,9 +71,8 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu)
>>>>>>> env->pvr.regs[10] = 0x0e000000; /* virtex 6 */
>>>>>>> /* setup pvr to match kernel setting */
>>>>>>> env->pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK;
>>>>>>> - env->pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI;
>>>>>>> + env->pvr.regs[0] |= PVR0_ENDI;
>>>>>>> env->pvr.regs[0] = (env->pvr.regs[0] & ~PVR0_VERSION_MASK) | (0x14
>>>>>>> << 8);
>>>>>>> - env->pvr.regs[2] ^= PVR2_USE_FPU2_MASK;
>>>>>>
>>>>>> So I think this here will clear PVR2_FPU2 ...
>>>>>>
>>>>>>> env->pvr.regs[4] = 0xc56b8000;
>>>>>>> env->pvr.regs[5] = 0xc56be000;
>>>>>>> }
>>>>>>> @@ -95,6 +94,7 @@ petalogix_ml605_init(MachineState *machine)
>>>>>>>
>>>>>>> /* init CPUs */
>>>>>>> cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
>>>>>>> + object_property_set_int(OBJECT(cpu), 2, "xlnx.use-fpu",
>>>>>>> &error_abort);
>>>>>>
>>>>>> But here you are setting use-fpu to 2. Is this a functional change for
>>>>>> ML605 board?
>>>>>>
>>>>>>> object_property_set_bool(OBJECT(cpu), true, "realized",
>>>>>>> &error_abort);
>>>>>>>
>>>>>>> /* Attach emulated BRAM through the LMB. */
>>>>>>> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
>>>>>>> index 7bc5b81..ce3a574 100644
>>>>>>> --- a/target-microblaze/cpu-qom.h
>>>>>>> +++ b/target-microblaze/cpu-qom.h
>>>>>>> @@ -62,6 +62,7 @@ typedef struct MicroBlazeCPU {
>>>>>>> /* Microblaze Configuration Settings */
>>>>>>> struct {
>>>>>>> bool stackproc;
>>>>>>> + uint8_t usefpu;
>>>>>>> } cfg;
>>>>>>>
>>>>>>> CPUMBState env;
>>>>>>> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
>>>>>>> index c08da19..c3a9a5d 100644
>>>>>>> --- a/target-microblaze/cpu.c
>>>>>>> +++ b/target-microblaze/cpu.c
>>>>>>> @@ -89,10 +89,18 @@ static void mb_cpu_reset(CPUState *s)
>>>>>>> | PVR2_USE_DIV_MASK \
>>>>>>> | PVR2_USE_HW_MUL_MASK \
>>>>>>> | PVR2_USE_MUL64_MASK \
>>>>>>> - | PVR2_USE_FPU_MASK \
>>>>>>> - | PVR2_USE_FPU2_MASK \
>>>>>>
>>>>>> If I have this straight, this suggests the property default value
>>>>>> should be 2, and the ml605 board setting should be 1.
>>>>>
>>>>> You are correct, I will fix this.
>>>>>
>>>>>>
>>>>>>> | PVR2_FPU_EXC_MASK \
>>>>>>> | 0;
>>>>>>> +
>>>>>>> + if (cpu->cfg.usefpu) {
>>>>>>> + env->pvr.regs[0] |= PVR0_USE_FPU_MASK;
>>>>>>> + env->pvr.regs[2] |= PVR2_USE_FPU_MASK;
>>>>>>> +
>>>>>>> + if (cpu->cfg.usefpu > 1) {
>>>>>>> + env->pvr.regs[2] |= PVR2_USE_FPU2_MASK;
>>>>>>> + }
>>>>>>> + }
>>>>>>
>>>>>> This should be handled at realize time. See pl330_realize for example
>>>>>> of realize-time PVR ("cfg" in that case) population.
>>>>>
>>>>> But the env state gets wiped out at reset, so the information will be
>>>>> lost.
>>>>>
>>>>
>>>> Ahh, so the solution there is to do what ARM does and have a section
>>>> at the back of the env which survives reset. Check the memset in
>>>> arm_cpu_reset.
>>>
>>> Ok, just to clarify we want all of the pvr registers to be preserved on
>>> reset?
>>>
>>
>> yes. But something that just occured to me, does it make sense to move
>> it outside the env? into the CPUState? Andreas mentioned that fields
>> in the CPU state before the env can be used with negative env* offsets
>> by translated code. This means the PVR could just be pushed up to
>> CPUState.
>
> Do any other machines do that?
>
> I'm happy with leaving it in the env (partly because I just did it)
> and it also matches the way that ARM does it.
>
Yeh ARM style is fine with me too. We can figure out the env vs CPU thing later.
Regards,
Peter
> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> Thanks,
>>>
>>> Alistair
>>>
>>>>
>>>> Regards,
>>>> Peter
>>>>
>>>>> I can tidy up the if logic though.
>>>>>
>>>>>>
>>>>>> To avoid churn, it might be easiest to convert the PVRs in chunks one
>>>>>> by one (e.g. one patch for PVR0, then PVR2 etc etc. The existing |ing
>>>>>> can be moved into realize for settings not worth propertyifying.
>>>>>>
>>>>>>> +
>>>>>>> env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp
>>>>>>> family. */
>>>>>>> env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
>>>>>>>
>>>>>>> @@ -154,6 +162,7 @@ static Property mb_properties[] = {
>>>>>>> DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU,
>>>>>>> base_vectors, 0),
>>>>>>> DEFINE_PROP_BOOL("xlnx.use-stack-protection", MicroBlazeCPU,
>>>>>>> cfg.stackproc,
>>>>>>> true),
>>>>>>> + DEFINE_PROP_UINT8("xlnx.use-fpu", MicroBlazeCPU, cfg.usefpu, 1),
>>>>>>
>>>>>> No need for xlnx prefix (FDT generic should trim the prefix).
>>>>>
>>>>> I agree, but the base-vectors already had it. If the xlnx is unneeded
>>>>> I'll remove
>>>>> it from the base-vector as well.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Alistair
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Peter
>>>>>>
>>>>>>> DEFINE_PROP_END_OF_LIST(),
>>>>>>> };
>>>>>>>
>>>>>>> diff --git a/target-microblaze/translate.c
>>>>>>> b/target-microblaze/translate.c
>>>>>>> index 4068946..36cfc5c 100644
>>>>>>> --- a/target-microblaze/translate.c
>>>>>>> +++ b/target-microblaze/translate.c
>>>>>>> @@ -1415,11 +1415,11 @@ static int dec_check_fpuv2(DisasContext *dc)
>>>>>>>
>>>>>>> r = dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU2_MASK;
>>>>>>>
>>>>>>> - if (!r && (dc->tb_flags & MSR_EE_FLAG)) {
>>>>>>> + if ((dc->cpu->cfg.usefpu != 2) && (dc->tb_flags & MSR_EE_FLAG)) {
>>>>>>> tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_FPU);
>>>>>>> t_gen_raise_exception(dc, EXCP_HW_EXCP);
>>>>>>> }
>>>>>>> - return r;
>>>>>>> + return (dc->cpu->cfg.usefpu == 2) ? 0 : PVR2_USE_FPU2_MASK;
>>>>>>> }
>>>>>>>
>>>>>>> static void dec_fpu(DisasContext *dc)
>>>>>>> @@ -1428,7 +1428,7 @@ static void dec_fpu(DisasContext *dc)
>>>>>>>
>>>>>>> if ((dc->tb_flags & MSR_EE_FLAG)
>>>>>>> && (dc->cpu->env.pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)
>>>>>>> - && !((dc->cpu->env.pvr.regs[2] & PVR2_USE_FPU_MASK))) {
>>>>>>> + && (dc->cpu->cfg.usefpu != 1)) {
>>>>>>> tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_ILLEGAL_OP);
>>>>>>> t_gen_raise_exception(dc, EXCP_HW_EXCP);
>>>>>>> return;
>>>>>>> --
>>>>>>> 1.7.1
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
