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

[Sarah Harris]

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.

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.
I haven't found a list of which AVRs use which value (other than reading every 
datasheet).

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.

Kind regards,
Sarah Harris

[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);
> +}