Re: [Qemu-arm] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Peter Maydell]

On 10 October 2017 at 10:19, Cédric Le Goater <address@hidden> wrote:
> On 10/06/2017 05:10 PM, Peter Maydell wrote:
>>> +    mc->ignore_memory_transaction_failures = true;
>>
>> Please don't set this flag for new board models, it is only
>> for our legacy existing ones. Instead implement any devices
>> that you need for guest code to boot (stub them out with
>> create_unimplemented_device() if you like).
>
> I have dug into this a little more and it seems that it is
> required for the Aspeed bootloader (a modified u-boot) which
> uses static variables in early init phases. So legacy firmwares
> won't work in QEMU but will on real HW.
>
> It's fine with me but what is the goal of the approach ? Force
> SoC providers into fixing their FW when they use QEMU ?

The goal is to model hardware correctly. Hardware gives
aborts if you touch a physical address with no device there,
and so QEMU's model should do the same. If you have guest
code that touches a physical address and blows up because
of an abort (but doesn't when run on h/w) then either:
 * it is trying to probe a device that exists in real h/w:
   you need to provide a stub implementation in QEMU
 * the SoC's bus fabric really doesn't pass aborts back
   to the CPU; I think this is unlikely, but you can model
   it at the SoC level with a suitable default memory region

The purpose of the flag is purely for existing board models,
where it is impossible to enable the correct (abort)
behaviour without possibly breaking guest code images that
work for people using released QEMU code. On a new board
model we don't have that problem and we can get it right
from the start. If we don't get it right from the start
then we will never have a chance to fix it in future.

Our mismodelling of this (not turning accesses to invalid
addresses into CPU aborts) meant that in the past it was
possible to be lazy when implementing a board model and
just not model half the hardware at all. Now it isn't,
but I don't think that adding a set of calls to
create_unimplemented_device() is a significant imposition.

thanks
-- PMM