Re: [Qemu-devel] [PATCH v3] target-arm: Fix resetting issues on ARMv7-M CPUs

[Peter Maydell]

On 6 September 2014 03:45, Peter Crosthwaite
<address@hidden> wrote:
> CC Alistair who is into ARMv7M
>
> On Sat, Sep 6, 2014 at 4:39 AM, Martin Galvan
> <address@hidden> wrote:
>> When calling qemu_system_reset after startup on a Cortex-M
>> CPU, the initial values of PC, MSP and the Thumb bit weren't being set
>> correctly if the vector table was in ROM. In particular, since Thumb was 0, a
>> Usage Fault would arise immediately after trying to execute any instruction
>> on a Cortex-M.

> Longer term (not this series) this could perhaps be better handled by
> reset callback order dependance. I.e. this reset handler is dependant
> on the ROM loader reset handler. We have similar issues with CPU reset
> ordering around the bootloader objects (such as hw/arm/boot.c) so I
> think this is more fuel on the fire for properly handling of reset
> deps.

In general I dislike how we handle M-profile reset of SP/PC, because
really what the hardware does is "CPU reads the SP and PC as
the first thing it does *after* reset", not "reads at point of reset".
The distinction starts to matter if you consider situations like
connecting with a gdb stub and using gdb to change the vector
table contents. But M-profile doesn't really rank high enough
in my list of priorities for me to think about a better approach.

>> +            initial_msp = ldl_p(rom);
>> +            initial_pc = ldl_p(rom + 4);
>> +        } else {
>> +            /* Address zero not covered by a ROM blob, or the ROM blob
>> +             * is in non-modifiable memory and this is a second reset after
>> +             * it got copied into memory. In the latter case, rom_ptr
>> +             * will return a NULL pointer and we should use ldl_phys 
>> instead.
>> +             */
>> +            initial_msp = ldl_phys(s->as, 0);
>> +            initial_pc = ldl_phys(s->as, 4);
>>          }
>> +
>> +        env->regs[13] = initial_msp & 0xFFFFFFFC;
>> +        env->regs[15] = initial_pc & ~1;
>> +        env->thumb = initial_pc & 1; /* Must always be 1 for ARMv7-M */
>
> I though some M profile variants supported non-thumb too? If it is a
> must, then is should report an error of some form.

No, all M profile cores have Thumb only, but the architecture
specifies the behaviour you get if a vector table entry doesn't
have the lowest bit set: the CPU reads the PC and clears the
Thumb bit in its PSR, then on the first attempt to execute an
instruction it takes a UsageFault (as it would for any attempt
to execute an instruction when the Thumb mode bit is clear).
You can think of it as the CPU knowing about the concept of
ARM vs Thumb mode but having no actual instructions in the
ARM decoder (though the detail of exactly what exception
we take is not quite what that would imply).

[A UsageFault taken immediately out of reset is going to be
escalated to a HardFault, but that's just following the usual
priority escalation rules, or at least it would be if we actually
implemented them...]

So the comment is actually incorrect and we should probably
delete it.

thanks
-- PMM