Patchwork [5/5] memory: get rid of cpu_register_io_memory()

login
register
mail settings
Submitter Avi Kivity
Date March 8, 2012, 5:20 p.m.
Message ID <1331227233-18601-6-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/145610/
State New
Headers show

Comments

Avi Kivity - March 8, 2012, 5:20 p.m.
The return value of cpu_register_io_memory() is no longer used anywhere, so
we can remove it and all associated data and code.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 cpu-all.h       |    8 -------
 exec-all.h      |    1 -
 exec-obsolete.h |    3 --
 exec.c          |   57 -------------------------------------------------------
 memory.c        |    5 +---
 5 files changed, 1 insertions(+), 73 deletions(-)
TeLeMan - March 19, 2012, 4:52 a.m.
On Fri, Mar 9, 2012 at 01:20, Avi Kivity <avi@redhat.com> wrote:
> The return value of cpu_register_io_memory() is no longer used anywhere, so
> we can remove it and all associated data and code.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  cpu-all.h       |    8 -------
>  exec-all.h      |    1 -
>  exec-obsolete.h |    3 --
>  exec.c          |   57 -------------------------------------------------------
>  memory.c        |    5 +---
>  5 files changed, 1 insertions(+), 73 deletions(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 80e6d42..b87f2ce 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -498,14 +498,6 @@ extern RAMList ram_list;
>  extern const char *mem_path;
>  extern int mem_prealloc;
>
> -/* physical memory access */
> -
> -/* MMIO pages are identified by a combination of an IO device index and
> -   3 flags.  The ROMD code stores the page ram offset in iotlb entry,
> -   so only a limited number of ids are avaiable.  */
> -
> -#define IO_MEM_NB_ENTRIES  (1 << TARGET_PAGE_BITS)
> -
>  /* Flags stored in the low bits of the TLB virtual address.  These are
>    defined so that fast path ram access is all zeros.  */
>  /* Zero if TLB entry is valid.  */
> diff --git a/exec-all.h b/exec-all.h
> index 4e8c7f5..3ec60a2 100644
> --- a/exec-all.h
> +++ b/exec-all.h
> @@ -304,7 +304,6 @@ uint64_t io_mem_read(struct MemoryRegion *mr, target_phys_addr_t addr,
>                      unsigned size);
>  void io_mem_write(struct MemoryRegion *mr, target_phys_addr_t addr,
>                   uint64_t value, unsigned size);
> -extern struct MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
>
>  void tlb_fill(CPUState *env1, target_ulong addr, int is_write, int mmu_idx,
>               void *retaddr);
> diff --git a/exec-obsolete.h b/exec-obsolete.h
> index 4dbe476..792c831 100644
> --- a/exec-obsolete.h
> +++ b/exec-obsolete.h
> @@ -32,9 +32,6 @@ void qemu_ram_free(ram_addr_t addr);
>  void qemu_ram_free_from_ptr(ram_addr_t addr);
>
>  struct MemoryRegion;
> -int cpu_register_io_memory(MemoryRegion *mr);
> -void cpu_unregister_io_memory(int table_address);
> -
>  struct MemoryRegionSection;
>  void cpu_register_physical_memory_log(struct MemoryRegionSection *section,
>                                       bool readonly);
> diff --git a/exec.c b/exec.c
> index 6e14048..0c86bce 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -214,9 +214,6 @@ struct PhysPageEntry {
>  static void io_mem_init(void);
>  static void memory_map_init(void);
>
> -/* io memory support */
> -MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
> -static char io_mem_used[IO_MEM_NB_ENTRIES];
>  static MemoryRegion io_mem_watch;
>  #endif
>
> @@ -3503,53 +3500,6 @@ static subpage_t *subpage_init(target_phys_addr_t base)
>     return mmio;
>  }
>
> -static int get_free_io_mem_idx(void)
> -{
> -    int i;
> -
> -    for (i = 0; i<IO_MEM_NB_ENTRIES; i++)
> -        if (!io_mem_used[i]) {
> -            io_mem_used[i] = 1;
> -            return i;
> -        }
> -    fprintf(stderr, "RAN out out io_mem_idx, max %d !\n", IO_MEM_NB_ENTRIES);
> -    return -1;
> -}
> -
> -/* mem_read and mem_write are arrays of functions containing the
> -   function to access byte (index 0), word (index 1) and dword (index
> -   2). Functions can be omitted with a NULL function pointer.
> -   If io_index is non zero, the corresponding io zone is
> -   modified. If it is zero, a new io zone is allocated. The return
> -   value can be used with cpu_register_physical_memory(). (-1) is
> -   returned if error. */
> -static int cpu_register_io_memory_fixed(int io_index, MemoryRegion *mr)
> -{
> -    if (io_index <= 0) {
> -        io_index = get_free_io_mem_idx();
> -        if (io_index == -1)
> -            return io_index;
> -    } else {
> -        if (io_index >= IO_MEM_NB_ENTRIES)
> -            return -1;
> -    }
> -
> -    io_mem_region[io_index] = mr;
> -
> -    return io_index;
> -}
> -
> -int cpu_register_io_memory(MemoryRegion *mr)
> -{
> -    return cpu_register_io_memory_fixed(0, mr);
> -}
> -
> -void cpu_unregister_io_memory(int io_index)
> -{
> -    io_mem_region[io_index] = NULL;
> -    io_mem_used[io_index] = 0;
> -}
> -
>  static uint16_t dummy_section(MemoryRegion *mr)
>  {
>     MemoryRegionSection section = {
> @@ -3569,11 +3519,7 @@ MemoryRegion *iotlb_to_region(target_phys_addr_t index)
>
>  static void io_mem_init(void)
>  {
> -    int i;
> -
> -    /* Must be first: */
>     memory_region_init_io(&io_mem_ram, &error_mem_ops, NULL, "ram", UINT64_MAX);
> -    assert(io_mem_ram.ram_addr == 0);
>     memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
>     memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
>                           "unassigned", UINT64_MAX);
> @@ -3581,9 +3527,6 @@ static void io_mem_init(void)
>                           "notdirty", UINT64_MAX);
>     memory_region_init_io(&io_mem_subpage_ram, &subpage_ram_ops, NULL,
>                           "subpage-ram", UINT64_MAX);
> -    for (i=0; i<5; i++)
> -        io_mem_used[i] = 1;
> -
>     memory_region_init_io(&io_mem_watch, &watch_mem_ops, NULL,
>                           "watch", UINT64_MAX);
>  }
> diff --git a/memory.c b/memory.c
> index bc76f55..22b0352 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -781,13 +781,11 @@ static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
>
>  static void memory_region_destructor_iomem(MemoryRegion *mr)
>  {
> -    cpu_unregister_io_memory(mr->ram_addr);
>  }
>
>  static void memory_region_destructor_rom_device(MemoryRegion *mr)
>  {
>     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
> -    cpu_unregister_io_memory(mr->ram_addr & ~TARGET_PAGE_MASK);
>  }
>
>  static bool memory_region_wrong_endianness(MemoryRegion *mr)
> @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
>     mr->opaque = opaque;
>     mr->terminates = true;
>     mr->destructor = memory_region_destructor_iomem;
> -    mr->ram_addr = cpu_register_io_memory(mr);
> +    mr->ram_addr = ~(ram_addr_t)0;
Why not 0 but -1?

>  }
>
>  void memory_region_init_ram(MemoryRegion *mr,
> @@ -992,7 +990,6 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>     mr->rom_device = true;
>     mr->destructor = memory_region_destructor_rom_device;
>     mr->ram_addr = qemu_ram_alloc(size, mr);
> -    mr->ram_addr |= cpu_register_io_memory(mr);
>  }
>
>  static uint64_t invalid_read(void *opaque, target_phys_addr_t addr,
> --
> 1.7.9
>
>
Avi Kivity - March 19, 2012, 9:16 a.m.
On 03/19/2012 06:52 AM, TeLeMan wrote:
> >  static bool memory_region_wrong_endianness(MemoryRegion *mr)
> > @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
> >     mr->opaque = opaque;
> >     mr->terminates = true;
> >     mr->destructor = memory_region_destructor_iomem;
> > -    mr->ram_addr = cpu_register_io_memory(mr);
> > +    mr->ram_addr = ~(ram_addr_t)0;
> Why not 0 but -1?
>

To catch bugs.  In fact it triggered bugs (not the ones I wanted though).
TeLeMan - March 19, 2012, 10:37 a.m.
On Mon, Mar 19, 2012 at 17:16, Avi Kivity <avi@redhat.com> wrote:
> On 03/19/2012 06:52 AM, TeLeMan wrote:
>> >  static bool memory_region_wrong_endianness(MemoryRegion *mr)
>> > @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
>> >     mr->opaque = opaque;
>> >     mr->terminates = true;
>> >     mr->destructor = memory_region_destructor_iomem;
>> > -    mr->ram_addr = cpu_register_io_memory(mr);
>> > +    mr->ram_addr = ~(ram_addr_t)0;
>> Why not 0 but -1?
>>
>
> To catch bugs.  In fact it triggered bugs (not the ones I wanted though).
There may be BSOD on the guest windows xp after applying this patch.
>
> --
> error compiling committee.c: too many arguments to function
>
Avi Kivity - March 19, 2012, 10:48 a.m.
On 03/19/2012 12:37 PM, TeLeMan wrote:
> On Mon, Mar 19, 2012 at 17:16, Avi Kivity <avi@redhat.com> wrote:
> > On 03/19/2012 06:52 AM, TeLeMan wrote:
> >> >  static bool memory_region_wrong_endianness(MemoryRegion *mr)
> >> > @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
> >> >     mr->opaque = opaque;
> >> >     mr->terminates = true;
> >> >     mr->destructor = memory_region_destructor_iomem;
> >> > -    mr->ram_addr = cpu_register_io_memory(mr);
> >> > +    mr->ram_addr = ~(ram_addr_t)0;
> >> Why not 0 but -1?
> >>
> >
> > To catch bugs.  In fact it triggered bugs (not the ones I wanted though).
> There may be BSOD on the guest windows xp after applying this patch.
>

May? Where?

Patch

diff --git a/cpu-all.h b/cpu-all.h
index 80e6d42..b87f2ce 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -498,14 +498,6 @@  extern RAMList ram_list;
 extern const char *mem_path;
 extern int mem_prealloc;
 
-/* physical memory access */
-
-/* MMIO pages are identified by a combination of an IO device index and
-   3 flags.  The ROMD code stores the page ram offset in iotlb entry, 
-   so only a limited number of ids are avaiable.  */
-
-#define IO_MEM_NB_ENTRIES  (1 << TARGET_PAGE_BITS)
-
 /* Flags stored in the low bits of the TLB virtual address.  These are
    defined so that fast path ram access is all zeros.  */
 /* Zero if TLB entry is valid.  */
diff --git a/exec-all.h b/exec-all.h
index 4e8c7f5..3ec60a2 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -304,7 +304,6 @@  uint64_t io_mem_read(struct MemoryRegion *mr, target_phys_addr_t addr,
                      unsigned size);
 void io_mem_write(struct MemoryRegion *mr, target_phys_addr_t addr,
                   uint64_t value, unsigned size);
-extern struct MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
 
 void tlb_fill(CPUState *env1, target_ulong addr, int is_write, int mmu_idx,
               void *retaddr);
diff --git a/exec-obsolete.h b/exec-obsolete.h
index 4dbe476..792c831 100644
--- a/exec-obsolete.h
+++ b/exec-obsolete.h
@@ -32,9 +32,6 @@  void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_free_from_ptr(ram_addr_t addr);
 
 struct MemoryRegion;
-int cpu_register_io_memory(MemoryRegion *mr);
-void cpu_unregister_io_memory(int table_address);
-
 struct MemoryRegionSection;
 void cpu_register_physical_memory_log(struct MemoryRegionSection *section,
                                       bool readonly);
diff --git a/exec.c b/exec.c
index 6e14048..0c86bce 100644
--- a/exec.c
+++ b/exec.c
@@ -214,9 +214,6 @@  struct PhysPageEntry {
 static void io_mem_init(void);
 static void memory_map_init(void);
 
-/* io memory support */
-MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
-static char io_mem_used[IO_MEM_NB_ENTRIES];
 static MemoryRegion io_mem_watch;
 #endif
 
@@ -3503,53 +3500,6 @@  static subpage_t *subpage_init(target_phys_addr_t base)
     return mmio;
 }
 
-static int get_free_io_mem_idx(void)
-{
-    int i;
-
-    for (i = 0; i<IO_MEM_NB_ENTRIES; i++)
-        if (!io_mem_used[i]) {
-            io_mem_used[i] = 1;
-            return i;
-        }
-    fprintf(stderr, "RAN out out io_mem_idx, max %d !\n", IO_MEM_NB_ENTRIES);
-    return -1;
-}
-
-/* mem_read and mem_write are arrays of functions containing the
-   function to access byte (index 0), word (index 1) and dword (index
-   2). Functions can be omitted with a NULL function pointer.
-   If io_index is non zero, the corresponding io zone is
-   modified. If it is zero, a new io zone is allocated. The return
-   value can be used with cpu_register_physical_memory(). (-1) is
-   returned if error. */
-static int cpu_register_io_memory_fixed(int io_index, MemoryRegion *mr)
-{
-    if (io_index <= 0) {
-        io_index = get_free_io_mem_idx();
-        if (io_index == -1)
-            return io_index;
-    } else {
-        if (io_index >= IO_MEM_NB_ENTRIES)
-            return -1;
-    }
-
-    io_mem_region[io_index] = mr;
-
-    return io_index;
-}
-
-int cpu_register_io_memory(MemoryRegion *mr)
-{
-    return cpu_register_io_memory_fixed(0, mr);
-}
-
-void cpu_unregister_io_memory(int io_index)
-{
-    io_mem_region[io_index] = NULL;
-    io_mem_used[io_index] = 0;
-}
-
 static uint16_t dummy_section(MemoryRegion *mr)
 {
     MemoryRegionSection section = {
@@ -3569,11 +3519,7 @@  MemoryRegion *iotlb_to_region(target_phys_addr_t index)
 
 static void io_mem_init(void)
 {
-    int i;
-
-    /* Must be first: */
     memory_region_init_io(&io_mem_ram, &error_mem_ops, NULL, "ram", UINT64_MAX);
-    assert(io_mem_ram.ram_addr == 0);
     memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
     memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
                           "unassigned", UINT64_MAX);
@@ -3581,9 +3527,6 @@  static void io_mem_init(void)
                           "notdirty", UINT64_MAX);
     memory_region_init_io(&io_mem_subpage_ram, &subpage_ram_ops, NULL,
                           "subpage-ram", UINT64_MAX);
-    for (i=0; i<5; i++)
-        io_mem_used[i] = 1;
-
     memory_region_init_io(&io_mem_watch, &watch_mem_ops, NULL,
                           "watch", UINT64_MAX);
 }
diff --git a/memory.c b/memory.c
index bc76f55..22b0352 100644
--- a/memory.c
+++ b/memory.c
@@ -781,13 +781,11 @@  static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
 
 static void memory_region_destructor_iomem(MemoryRegion *mr)
 {
-    cpu_unregister_io_memory(mr->ram_addr);
 }
 
 static void memory_region_destructor_rom_device(MemoryRegion *mr)
 {
     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
-    cpu_unregister_io_memory(mr->ram_addr & ~TARGET_PAGE_MASK);
 }
 
 static bool memory_region_wrong_endianness(MemoryRegion *mr)
@@ -942,7 +940,7 @@  void memory_region_init_io(MemoryRegion *mr,
     mr->opaque = opaque;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_iomem;
-    mr->ram_addr = cpu_register_io_memory(mr);
+    mr->ram_addr = ~(ram_addr_t)0;
 }
 
 void memory_region_init_ram(MemoryRegion *mr,
@@ -992,7 +990,6 @@  void memory_region_init_rom_device(MemoryRegion *mr,
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
     mr->ram_addr = qemu_ram_alloc(size, mr);
-    mr->ram_addr |= cpu_register_io_memory(mr);
 }
 
 static uint64_t invalid_read(void *opaque, target_phys_addr_t addr,