Message ID | 1314180683-8227-12-git-send-email-avi@redhat.com |
---|---|
State | New |
Headers | show |
On 24 August 2011 11:11, Avi Kivity <avi@redhat.com> wrote: > @@ -108,9 +111,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) > static void integratorcm_do_remap(integratorcm_state *s, int flash) > { > if (flash) { > - cpu_register_physical_memory(0, 0x100000, IO_MEM_RAM); > + if (s->flash_mapped) { > + sysbus_del_memory(&s->busdev, &s->flash); > + s->flash_mapped = false; > + } > } else { > - cpu_register_physical_memory(0, 0x100000, s->flash_offset | IO_MEM_RAM); > + if (!s->flash_mapped) { > + sysbus_add_memory_overlap(&s->busdev, 0, &s->flash, 1); > + s->flash_mapped = true; > + } > } This is introducing a new field in the device state which is actually just tracking s->cm_init bit 2. At least it would be, except that in integratorcm_set_ctrl this line: s->cm_init = (s->cm_init & ~ 5) | (value ^ 5); appears to be using ^ when it meant & ... This isn't a nak -- I'm happy for this to get committed and I'll fix things up later, since the device doesn't have a VMStateDescription that would be broken by the extra state field. (Or if I get round to it before the series gets committed I'll send you a fix to squash into this patch.) -- PMM
On 08/24/2011 02:22 PM, Peter Maydell wrote: > On 24 August 2011 11:11, Avi Kivity<avi@redhat.com> wrote: > > @@ -108,9 +111,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) > > static void integratorcm_do_remap(integratorcm_state *s, int flash) > > { > > if (flash) { > > - cpu_register_physical_memory(0, 0x100000, IO_MEM_RAM); > > + if (s->flash_mapped) { > > + sysbus_del_memory(&s->busdev,&s->flash); > > + s->flash_mapped = false; > > + } > > } else { > > - cpu_register_physical_memory(0, 0x100000, s->flash_offset | IO_MEM_RAM); > > + if (!s->flash_mapped) { > > + sysbus_add_memory_overlap(&s->busdev, 0,&s->flash, 1); > > + s->flash_mapped = true; > > + } > > } > > This is introducing a new field in the device state which is actually > just tracking s->cm_init bit 2. At least it would be, except that > in integratorcm_set_ctrl this line: > s->cm_init = (s->cm_init& ~ 5) | (value ^ 5); > > appears to be using ^ when it meant& ... > > This isn't a nak -- I'm happy for this to get committed and I'll fix > things up later, since the device doesn't have a VMStateDescription > that would be broken by the extra state field. Even with vmstate, nothing would break since the field would be recovered on restore. > (Or if I get round to > it before the series gets committed I'll send you a fix to squash > into this patch.) That would be best. Please post the &/^ fixup separately (if needed) since it isn't strictly related to the conversion.
diff --git a/hw/integratorcp.c b/hw/integratorcp.c index 3d4b339..720cc02 100644 --- a/hw/integratorcp.c +++ b/hw/integratorcp.c @@ -7,6 +7,8 @@ * This code is licensed under the GPL */ +#include <glib.h> + #include "sysbus.h" #include "primecell.h" #include "devices.h" @@ -17,7 +19,8 @@ typedef struct { SysBusDevice busdev; uint32_t memsz; - uint32_t flash_offset; + MemoryRegion flash; + bool flash_mapped; uint32_t cm_osc; uint32_t cm_ctrl; uint32_t cm_lock; @@ -108,9 +111,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) static void integratorcm_do_remap(integratorcm_state *s, int flash) { if (flash) { - cpu_register_physical_memory(0, 0x100000, IO_MEM_RAM); + if (s->flash_mapped) { + sysbus_del_memory(&s->busdev, &s->flash); + s->flash_mapped = false; + } } else { - cpu_register_physical_memory(0, 0x100000, s->flash_offset | IO_MEM_RAM); + if (!s->flash_mapped) { + sysbus_add_memory_overlap(&s->busdev, 0, &s->flash, 1); + s->flash_mapped = true; + } } //??? tlb_flush (cpu_single_env, 1); } @@ -252,7 +261,8 @@ static int integratorcm_init(SysBusDevice *dev) } memcpy(integrator_spd + 73, "QEMU-MEMORY", 11); s->cm_init = 0x00000112; - s->flash_offset = qemu_ram_alloc(NULL, "integrator.flash", 0x100000); + memory_region_init_ram(&s->flash, NULL, "integrator.flash", 0x100000); + s->flash_mapped = false; iomemtype = cpu_register_io_memory(integratorcm_readfn, integratorcm_writefn, s, @@ -458,7 +468,8 @@ static void integratorcp_init(MemoryRegion *address_space_mem, const char *initrd_filename, const char *cpu_model) { CPUState *env; - ram_addr_t ram_offset; + MemoryRegion *ram = g_new(MemoryRegion, 1); + MemoryRegion *ram_alias = g_new(MemoryRegion, 1); qemu_irq pic[32]; qemu_irq *cpu_pic; DeviceState *dev; @@ -471,13 +482,14 @@ static void integratorcp_init(MemoryRegion *address_space_mem, fprintf(stderr, "Unable to find CPU definition\n"); exit(1); } - ram_offset = qemu_ram_alloc(NULL, "integrator.ram", ram_size); + memory_region_init_ram(ram, NULL, "integrator.ram", ram_size); /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash. */ /* ??? RAM should repeat to fill physical memory space. */ /* SDRAM at address zero*/ - cpu_register_physical_memory(0, ram_size, ram_offset | IO_MEM_RAM); + memory_region_add_subregion(address_space_mem, 0, ram); /* And again at address 0x80000000 */ - cpu_register_physical_memory(0x80000000, ram_size, ram_offset | IO_MEM_RAM); + memory_region_init_alias(ram_alias, "ram.alias", ram, 0, ram_size); + memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias); dev = qdev_create(NULL, "integrator_core"); qdev_prop_set_uint32(dev, "memsz", ram_size >> 20);
Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/integratorcp.c | 28 ++++++++++++++++++++-------- 1 files changed, 20 insertions(+), 8 deletions(-)