Re: [Qemu-arm] [PATCH v2 02/26] armv7m: Undo armv7m.hack

[Michael Davidsaver <mdavidsaver@gmail.com>]

On 12/17/2015 10:38 AM, Peter Maydell wrote:
> On 3 December 2015 at 00:18, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
>> Add CPU unassigned access handler in place of special
>> MemoryRegion to catch exception returns.
>>
>> The unassigned handler will signal other faults as either
>> prefetch or data exceptions, with the FSR code 0x8 to
>> distinguish them from memory translation faults (0xd).
>> Future code will make use of this distinction when
>> deciding to raise BusFault or MemManage exceptions.
> This patch breaks my Stellaris test image -- instead of starting
> it just sits there with a black screen.
>
> I've put a copy of that test image up at
>   http://people.linaro.org/~peter.maydell/stellaris.tgz
> You can run it with path/to/stellaris/runme path/to/qemu-system-arm .

There were several issues.  Two bugs (wrong IRQ enabled and systick not
enabled) and a "feature" (access to unimplemented registers for a PWM
controller is now a BusFault).

As a workaround for the "feature" I add a low priority MemoryRegion from
0x40000000 -> 0x40ffffff which completes all reads with zero and logs. 
Please advise on how this should be handled.

With these changes both test programs appear to run correctly, although
the http server example has painfully slow load times and seems to hit
an out of memory condition if I look at it wrong.  Is this expected? 
(and the blub on the buttons page about "xml technology" is priceless)

I can see that the http server example spends some time attempting MII
operations on the NIC.  As these aren't modeled it spins and eventually
gives up.

> ...
> We could use a comment here (a) explaining what we're doing and (b)
> mentioning that this isn't architecturally correct -- ideally we
> should catch these exception exits on execution of the jump insn, not
> by letting the jump execute and then trapping when we actually try to
> execute at the magic addresses. 

Will do.

>> ...
>> @@ -294,19 +313,9 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>>          cc->do_interrupt(cs);
>>          ret = true;
>>      }
>> -    /* ARMv7-M interrupt return works by loading a magic value
>> -     * into the PC.  On real hardware the load causes the
>> -     * return to occur.  The qemu implementation performs the
>> -     * jump normally, then does the exception return when the
>> -     * CPU tries to execute code at the magic address.
>> -     * This will cause the magic PC value to be pushed to
>> -     * the stack if an interrupt occurred at the wrong time.
>> -     * We avoid this by disabling interrupts when
>> -     * pc contains a magic address.
>> -     */
>>      if (interrupt_request & CPU_INTERRUPT_HARD
>>          && !(env->daif & PSTATE_I)
>> -        && (env->regs[15] < 0xfffffff0)) {
>> +            ) {
> Can we really drop this change? The thing it's guarding against
> (interrupt comes in while the PC is this not-really-an-address
> value) can still happen whether we catch the attempt to execute
> in translate.c or via the unassigned-access hook.

I don't think the M-profile case in gen_intermediate_code() in
translate.c can ever be reached without first hitting the unassigned
memory handler.  Before the code can be translated, the page containing
it must be loaded.  Such loads no longer succeed.

Put more literally, gen_intermediate_code() is only called from
tb_gen_code() where it comes after a call to get_page_addr_code(),
wherein the unassigned handler calls cpu_loop_exit().

I've replaced the M case for EXCP_EXCEPTION_EXIT in
gen_intermediate_code() with an assert.  So far it hasn't failed.