On Sat, Sep 6, 2014 at 8:00 AM, Peter Crosthwaite
<peter.crosthwa...@xilinx.com> wrote:
> On Sat, Sep 6, 2014 at 8:42 PM, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
>> On 6 September 2014 03:45, Peter Crosthwaite
>> <peter.crosthwa...@xilinx.com> wrote:
>>> CC Alistair who is into ARMv7M
>>>
>>> On Sat, Sep 6, 2014 at 4:39 AM, Martin Galvan
>>> <martin.gal...@tallertechnologies.com> wrote:
>>>> }
>>>>>
>>>>> if (env->cp15.c1_sys & SCTLR_V) {
>>>>> - env->regs[15] = 0xFFFF0000;
>>>>> + env->regs[15] = 0xFFFF0000;
>>>
>>> Out of scope change.
Oh, I must've changed that and forgotten it was gonna be included on
the patch. Still, quoting
http://wiki.qemu.org/Contribute/SubmitAPatch:
"It's OK to fix coding style issues in the immediate area (few lines)
of the lines you're changing. "
>>>> + 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.
Should I send a new patch version, just to remove that one comment?
--
Martín Galván
Software Engineer
Taller Technologies Argentina
San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina
Phone: 54 351 4217888 / +54 351 4218211
