Re: [PATCH rc3 02/30] target/avr: Introduce AVR CPU class object

[Aleksandar Markovic]

On Wed, Jan 29, 2020 at 1:20 PM Sarah Harris <address@hidden> wrote:
>
> Hi,
>
> I think I've found a minor bug: the stack pointer should be initialised to 
> the size of SRAM in some or most cases.
> Currently, SP is initialised to zero.
>

Very good (that you spotted the bug)!

> It seems modern AVRs set SP to the size of SRAM (RAMEND) at power-on, though 
> a few older ones initialise to zero.
> The ATmega328 (from 2009) [1], ATmega2560 (from 2005) [2], ATtiny2313 (from 
> 2003) [6], and ATtiny85 (from 2005) [3] all use RAMEND.
> The ATmega8 (from 2001) [4], ATmega8535 (from 2002) [5], and AT90S8535 (from 
> 1998) [7] use zero.

It looks we'll have tremendous "fun" dealing with huge variety of AVR SoCs.

> I haven't found a list of which AVRs use which value (other than reading 
> every datasheet).

Yes, I find this to be a big practical problem, this glaring lack of
some summary documentation, and, unfortunately, this affects not only
this particular case (SP initialization). It appears to me that for
many features and details (especially regarding SoC devices) one will
have to carefully examine multiple documents, as you did in this
email.

>
> Given that GCC performs this initialisation in software anyway (so what the 
> hardware does doesn't matter), I think this is a minor issue.
> It will only affect hand written assembly programs that don't do their own 
> initialisation (which seems to be discouraged as not all resets are power-on 
> events).
> I'm not sure what, if anything, needs to be done about it but it might be 
> worth fixing now we're emulating specific chips.
>

Let's think a little bit about it. In my honest opinion, this may be
handled after the series merge, and if you guys agree, there is no
rush.

> Kind regards,
> Sarah Harris
>

Thanks!
Aleksandar

> [1] 
> http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-7810-Automotive-Microcontrollers-ATmega328P_Datasheet.pdf
>  (section 6.5.1)
> [2] 
> http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2549-8-bit-AVR-Microcontroller-ATmega640-1280-1281-2560-2561_datasheet.pdf
>  (section 7.6)
> [3] 
> http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2586-AVR-8-bit-Microcontroller-ATtiny25-ATtiny45-ATtiny85_Datasheet.pdf
>  (section 4.6.1)
> [4] 
> http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2486-8-bit-AVR-microcontroller-ATmega8_L_datasheet.pdf
>  (page 13)
> [5] http://ww1.microchip.com/downloads/en/DeviceDoc/doc2502.pdf (page 12)
> [6] 
> http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-2543-AVR-ATtiny2313_Datasheet.pdf
>  (page 11)
> [7] http://ww1.microchip.com/downloads/en/DeviceDoc/doc1041.pdf (page 20)
>
> On Sun, 26 Jan 2020 23:54:43 +0100
> Aleksandar Markovic <address@hidden> wrote:
>
> > +static void avr_cpu_reset(CPUState *cs)
> > +{
> > +    AVRCPU *cpu = AVR_CPU(cs);
> > +    AVRCPUClass *mcc = AVR_CPU_GET_CLASS(cpu);
> > +    CPUAVRState *env = &cpu->env;
> > +
> > +    mcc->parent_reset(cs);
> > +
> > +    env->pc_w = 0;
> > +    env->sregI = 1;
> > +    env->sregC = 0;
> > +    env->sregZ = 0;
> > +    env->sregN = 0;
> > +    env->sregV = 0;
> > +    env->sregS = 0;
> > +    env->sregH = 0;
> > +    env->sregT = 0;
> > +
> > +    env->rampD = 0;
> > +    env->rampX = 0;
> > +    env->rampY = 0;
> > +    env->rampZ = 0;
> > +    env->eind = 0;
> > +    env->sp = 0;
> > +
> > +    env->skip = 0;
> > +
> > +    memset(env->r, 0, sizeof(env->r));
> > +
> > +    tlb_flush(cs);
> > +}