Re: [Qemu-devel] [PATCH] pflash_cfi01/pflash_cfi02: convert to memory API

[Avi Kivity <avi@redhat.com>]

On 08/28/2011 09:14 PM, Edgar E. Iglesias wrote:
> On Sun, Aug 28, 2011 at 06:43:36PM +0300, Avi Kivity wrote:
> >  cfi02 is annoying in that is ignores some address bits; we probably
> >  want explicit support in the memory API for that.
> >
> >  In order to get the correct opaque into the MemoryRegion object, the
> >  allocation scheme is changed so that the flash emulation code allocates
> >  memory, instead of the caller.  This clears a FIXME in the flash code.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
> >  ---
> >
> >  This is a rework of the pflash conversion to the memory API.  It has changed
> >  significantly - the change in allocation described above - so please review
> >  carefully.
> >
> >  The mips_malta change also changes behavious - previously on of the two
> >  aliases was mapped as rom/device while the other was mapped as plain rom.
> >  Now both aliases are mapped as rom/device.  I'm guessing that this is the
> >  right behaviour, and the old behaviour was just an implementation limitation,
> >  but if someone can check, that would be even better.  That file has the most
> >  sensitive changes so please review it extra carefully.
>
> Regarding the flash mapping, your description matches my understanding of it.
> There is a difference between the 0x1fc and the 0x1e0 mappings though, in that
> reading from 0x1fc00010 does not map to the flash but instead gets decoded
> into a read from a revision register. To read from the flash at 0x10, you
> need to go via the 0x1e0 mapping.
>
> That was never modelled by QEMU, instead the malta board writes into the
> backing ram of the flash area, see:
> /* Board ID = 0x420 (Malta Board with CoreLV)
>     XXX: theoretically 0x1e000010 should map to flash and 0x1fc00010 should
>     map to the board ID. */
> stl_p(memory_region_get_ram_ptr(bios) + 0x10, 0x00000420);

This can be modelled correctly, using a container and a new region for 
0x10-0x14.  I'll live that as an exercise to the interested writer.

> I assume this will continue to work even after your changes but it would be
> good to test it. I can check it.
>
> A problem, with my compiler I see this:
> qemu/hw/mips_malta.c: In function ‘mips_malta_init’:
> qemu/hw/mips_malta.c:860:37: error: ‘bios’ may be used uninitialized in this function [-Werror=uninitialized]
>

Thanks, I have a fix for this and will post an updated patch once the 
other issues are resolved.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.