qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH v3] target-arm: Fix resetting issues on ARMv7-M CPUs
Date: Sat, 6 Sep 2014 21:00:26 +1000

On Sat, Sep 6, 2014 at 8:42 PM, Peter Maydell <address@hidden> wrote:
> 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.
>

Does scheduling this post-reset stuff in a one-shot expired timer solve this?

By default the ARM CPU would need to be ->halted, then on reset() you
set an expired timer with handler which does SP, PC etc and clears
->halted. This if-else ROM awareness should then evaporate.

Getting slightly more radical, that may be applicable to the arm/boot
bootloader too.

>>> +            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.

Yep, thats what I was thinking but your reasonings are more correct than mine.

Regards,
Peter

>
> thanks
> -- PMM
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]