On Mon, May 18, 2015 at 1:12 AM, Andreas Färber <afaer...@suse.de> wrote: > Am 17.05.2015 um 14:26 schrieb Alistair Francis: >> On Fri, May 15, 2015 at 9:15 PM, Andreas Färber <afaer...@suse.de> wrote: >>> Am 15.05.2015 um 08:36 schrieb Alistair Francis: >>>> 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: >>>>>>>>>> | 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. >> >> I think env makes sense in this case, especially as the >> DEFINE_PROP_UINT8() macro defaults to setting variables in the env >> structure. Admittedly I haven't looked into it, so it might be just as >> easy to set variables in the MicroBlazeCPU. > > In fact the CPU properties operate on MicroBlazeCPU. The legacy > properties thus need an explicit env.foo as macro argument.
I confused myself with that last comment and forgot that we were talking about the pvr regs and not the configuration variables. So it doesn't matter what the DEFINE_PROP_UINT8() macro does. I'll send my patches now and we can keep discussing it there. Thanks, Alistair > > But as usual things can be done in multiple steps. > > Andreas > >> I have another patch set ready to send out tomorrow with all of the >> variables in env. We can decide from there if they should be moved to >> the MicroBlazeCPU struct. >> >> Thanks, >> >> Alistair > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB > 21284 (AG Nürnberg) >