Re: [Qemu-devel] [PATCH] Versatile Express: add modelling of NOR flash

[Francesco Lavra]

On 09/05/2012 09:07 PM, Francesco Lavra wrote:
> Hi,
> 
> On 09/05/2012 10:47 AM, Peter Maydell wrote:
>> On 5 September 2012 06:16, Stefan Weil <address@hidden> wrote:
>>> Am 04.09.2012 19:08, schrieb Francesco Lavra:
>>>>       /* VE_NORFLASH0ALIAS: not modelled */
>>>
>>>
>>> What about that alias? It's not difficult to add it, too.
>>> Just look for memory_region_init_alias in the code to
>>> see how it is done (hw/mips_malta.c has an alias region
>>> for flash).
>>
>> It's painful because you might also have to add the logic for
>> letting the guest map and unmap the alias (which implies
>> implementing a whole section of the A15 board we don't currently
>> bother with, the SCC registers). I'd need to check the board
>> documentation more carefully to see if we can get away with
>> always mapping that area as the flash alias.
> 
> Documentation at
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0503c/CHDEFDJF.html
> says that the entire first 512 MB can be mapped to either SMC (which is
> the default) or AXI, so if AXI is selected neither of the 2 flash banks
> is visible. Also, the same doc says that it's possible to map either
> NOR0 (default) or NOR1 to the address 0x00000000. This implies that in
> the A Series memory map VE_NORFLASH0 should be at 0x08000000 and
> VE_NORFLASH0ALIAS at 0x00000000, not the other way around (by the way,
> this is also how U-Boot defines the memory for the A5 CoreTile). Maybe
> worth a patch?
> 
> If we can get way with always aliasing to flash 0, the actual
> implementation of the alias is made difficult by the fact that
> memory_region_init_alias() needs the MemoryRegion of the aliased memory,
> and the daughterboard-specific initialization is done in a function
> which doesn't have access to that MemoryRegion. So we can either:
> 1. move initialization of common flash modelling before
> daughterboard-specific initialization and pass the relevant MemoryRegion
> to the daughterboard-specific init function
> 2. add another field to VEDBoardInfo which tells if the alias capability
> is implemented, and use this info in vexpress_common_init() to define
> the alias if appropriate
> Or we can simply deem this alias not worth the trouble, which is what I
> thought before sending the patch... Let me know your thoughts.
> 
>>
>> (Also we'd need to fix the current problem with the
>> motherboard address map arrays that there's no way to
>> distinguish "peripheral not present on this board" from
>> "peripheral at address 0", since the A9 board doesn't have
>> the flash alias.)
>>
>> More to the point, this is the third attempt at doing this.
>> Previously Liming Wang sent a patch:
>>  http://patchwork.ozlabs.org/patch/147905/
>> and Jagan sent a two-patch set:
>>  http://patchwork.ozlabs.org/patch/171812/
>>  http://patchwork.ozlabs.org/patch/171814/
>>
>> both of which failed in the code review stage. Francesco,
>> can you check that you haven't fallen into any of the
>> same problems they did, please?
> 
> I read the reviews of previous attempts, and in fact there is a fix
> which can be easily done, i.e. replacing the calls to drive_get() with
> drive_get_next(). Will do that in v2, but first the above points need to
> be addressed.
> 
> Thanks,
> Francesco

Ping?
http://thread.gmane.org/gmane.comp.emulators.qemu/168461