Re: [Qemu-devel] [RFC v1 3/3] target-microblaze: Convert use-fpu to a CPU property

["Andreas Färber" <afaerber@suse.de>]

Am 15.05.2015 um 08:36 schrieb Alistair Francis:
> On Fri, May 15, 2015 at 4:32 PM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Thu, May 14, 2015 at 10:56 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> On Fri, May 15, 2015 at 3:52 PM, Peter Crosthwaite
>>> <peter.crosthwaite@xilinx.com> wrote:
>>>> On Thu, May 14, 2015 at 10:48 PM, Alistair Francis
>>>> <alistair.francis@xilinx.com> wrote:
>>>>> On Fri, May 15, 2015 at 3:22 PM, Peter Crosthwaite
>>>>> <peter.crosthwaite@xilinx.com> wrote:
>>>>>> On Wed, May 13, 2015 at 11:08 PM, Alistair Francis
>>>>>> <alistair.francis@xilinx.com> wrote:
>>>>>>>                          | 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.

All targets had everything in env originally, so that's not really an
argument for anything. You need to decide what makes sense for your
target - icount is an example using negative offsets in CPUState now,
whereas TCGv* variables declared via offset from env remained in env.

If moving something to the end of env is an option, then moving it to
MicroBlazeCPU (not CPUState!) would seem better, especially when it's
not supposed to be reset. Going from CPUMBState *env to MicroBlazeCPU
*cpu is a cheap offset operation, whereas CPUState *cs involves a QOM cast.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)