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

[Edgar E. Iglesias]

On Mon, Aug 29, 2011 at 08:38:26AM +0300, Avi Kivity wrote:
> On 08/28/2011 11:37 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<address@hidden>
> >>  ---
> >
> >
> >
> >>  diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c
> >>  index 90e1301..b597304 100644
> >>  --- a/hw/pflash_cfi01.c
> >>  +++ b/hw/pflash_cfi01.c
> >>  @@ -40,6 +40,7 @@
> >>   #include "flash.h"
> >>   #include "block.h"
> >>   #include "qemu-timer.h"
> >>  +#include "exec-memory.h"
> >>
> >>   #define PFLASH_BUG(fmt, ...) \
> >>   do { \
> >>  @@ -74,8 +75,7 @@ struct pflash_t {
> >>       target_phys_addr_t counter;
> >>       unsigned int writeblock_size;
> >>       QEMUTimer *timer;
> >>  -    ram_addr_t off;
> >>  -    int fl_mem;
> >>  +    MemoryRegion mem;
> >>       void *storage;
> >>   };
> >>
> >>  @@ -89,8 +89,7 @@ static void pflash_timer (void *opaque)
> >>       if (pfl->bypass) {
> >>           pfl->wcycle = 2;
> >>       } else {
> >>  -        cpu_register_physical_memory(pfl->base, pfl->total_len,
> >>  -                        pfl->off | IO_MEM_ROMD | pfl->fl_mem);
> >>  +        memory_region_rom_device_set_readable(&pfl->mem, true);
> >>           pfl->wcycle = 0;
> >>       }
> >>       pfl->cmd = 0;
> >>  @@ -263,7 +262,7 @@ static void pflash_write(pflash_t *pfl, 
> >> target_phys_addr_t offset,
> >>
> >>       if (!pfl->wcycle) {
> >>           /* Set the device in I/O access mode */
> >>  -        cpu_register_physical_memory(pfl->base, pfl->total_len, 
> >> pfl->fl_mem);
> >>  +        memory_region_rom_device_set_readable(&pfl->mem, false);
> >>       }
> >
> >I get the impression that this one is not biting. Reads are not reaching
> >the IO callbacks at times when they should..
> >
> 
> It does bite, as I saw with gdb.  You can even see that from the
> qemu error - it complains about unimplemented command 0xf0, that
> comes from pflash_write() later on.

I wasn't clear enough maybe, the code in question is running, but the
set_readble is not having any effect AFAICT. The reads, that
should come through via IO callbacks when switching mode are
not getting through. Do you ever see pflash_read beeing called?

Cheers