Patchwork [11/24] integratorcp: convert to memory API (RAM/flash only)

login
register
mail settings
Submitter Avi Kivity
Date Aug. 24, 2011, 10:11 a.m.
Message ID <1314180683-8227-12-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/111313/
State New
Headers show

Comments

Avi Kivity - Aug. 24, 2011, 10:11 a.m.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/integratorcp.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)
Peter Maydell - Aug. 24, 2011, 11:22 a.m.
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
Avi Kivity - Aug. 24, 2011, 11:26 a.m.
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.

Patch

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);