[Qemu-devel] [RESEND PATCH v3 0/2] Minimal RAM API support

[Qemu-devel] [RESEND PATCH v3 0/2] Minimal RAM API support

[Alex Williamson]

No comments since v3, please apply.  Thanks,

Alex

v3:

 - Address review comments
 - pc registers all memory below 4G in one chunk

Let me know if there are any further issues.

v2:

 - Move to Makefile.objs
 - Move structures to memory.c and create a callback function
 - Fix memory leak

I haven't moved to the state parameter because there should only
be a single instance of this per VM.  The state parameter seems
like it would add complications in setup and function calling, but
maybe point me to an example if I'm off base.

v1:

For VFIO based device assignment, we need to know what guest memory
areas are actual RAM.  RAMBlocks have long since become a grab bag
of misc allocations, so aren't effective for this.  Anthony has had
a RAM API in mind for a while now that addresses this problem.  This
implements just enough of it so that we have an interface to get
actual guest memory physical addresses to setup the host IOMMU.  We
can continue building a full RAM API on top of this stub.

Anthony, feel free to add copyright to memory.c as it's based on
your initial implementation.  I had to add something since the file
in your branch just copies a header with Frabrice's copywrite.

---

Alex Williamson (2):
      RAM API: Make use of it for x86 PC
      Minimal RAM API support


 Makefile.objs |    1 +
 cpu-common.h  |    2 +
 hw/pc.c       |    9 ++---
 memory.c      |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h      |   44 ++++++++++++++++++++++++++
 5 files changed, 147 insertions(+), 6 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

[Qemu-devel] [RESEND PATCH v3 1/2] Minimal RAM API support

[Alex Williamson]

This adds a minimum chunk of Anthony's RAM API support so that we
can identify actual VM RAM versus all the other things that make
use of qemu_ram_alloc.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 Makefile.objs |    1 +
 cpu-common.h  |    2 +
 memory.c      |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h      |   44 ++++++++++++++++++++++++++
 4 files changed, 144 insertions(+), 0 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

diff --git a/Makefile.objs b/Makefile.objs
index cebb945..47f3c3a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -172,6 +172,7 @@ hw-obj-y += pci.o pci_bridge.o msix.o msi.o
 hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
 hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
 hw-obj-y += watchdog.o
+hw-obj-y += memory.o
 hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 hw-obj-$(CONFIG_ECC) += ecc.o
 hw-obj-$(CONFIG_NAND) += nand.o
diff --git a/cpu-common.h b/cpu-common.h
index 6d4a898..f08f93b 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -29,6 +29,8 @@ enum device_endian {
 /* address in the RAM (different from a physical address) */
 typedef unsigned long ram_addr_t;
 
+#include "memory.h"
+
 /* memory API */
 
 typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
diff --git a/memory.c b/memory.c
new file mode 100644
index 0000000..742776f
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,97 @@
+/*
+ * RAM API
+ *
+ *  Copyright Red Hat, Inc. 2010
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#include "memory.h"
+#include "range.h"
+
+typedef struct ram_slot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    QLIST_ENTRY(ram_slot) next;
+} ram_slot;
+
+static QLIST_HEAD(ram_slots, ram_slot) ram_slots =
+    QLIST_HEAD_INITIALIZER(ram_slots);
+
+static ram_slot *qemu_ram_find_slot(target_phys_addr_t start_addr,
+                                   ram_addr_t size)
+{
+    ram_slot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots, next) {
+        if (slot->start_addr == start_addr && slot->size == size) {
+            return slot;
+        }
+
+        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
+            hw_error("Ram range overlaps existing slot\n");
+        }
+    }
+
+    return NULL;
+}
+
+int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                      ram_addr_t phys_offset)
+{
+    ram_slot *slot;
+
+    if (!size) {
+        return -EINVAL;
+    }
+
+    assert(!qemu_ram_find_slot(start_addr, size));
+
+    slot = qemu_mallocz(sizeof(ram_slot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slots, slot, next);
+
+    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
+
+    return 0;
+}
+
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    ram_slot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    slot = qemu_ram_find_slot(start_addr, size);
+    assert(slot != NULL);
+
+    QLIST_REMOVE(slot, next);
+    qemu_free(slot);
+    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
+
+    return;
+}
+
+int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn)
+{
+    ram_slot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots, next) {
+        int ret = fn(opaque, slot->start_addr, slot->size, slot->offset);
+        if (ret) {
+            return ret;
+        }
+    }
+    return 0;
+}
diff --git a/memory.h b/memory.h
new file mode 100644
index 0000000..e7aa5cb
--- /dev/null
+++ b/memory.h
@@ -0,0 +1,44 @@
+#ifndef QEMU_MEMORY_H
+#define QEMU_MEMORY_H
+/*
+ * RAM API
+ *
+ *  Copyright Red Hat, Inc. 2010
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "cpu-common.h"
+
+typedef int (*qemu_ram_for_each_slot_fn)(void *opaque,
+                                         target_phys_addr_t start_addr,
+                                         ram_addr_t size,
+                                         ram_addr_t phys_offset);
+
+/**
+ * qemu_ram_register() : Register a region of guest physical memory
+ *
+ * The new region must not overlap an existing region.
+ */
+int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                      ram_addr_t phys_offset);
+
+/**
+ * qemu_ram_unregister() : Unregister a region of guest physical memory
+ */
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+
+/**
+ * qemu_ram_for_each_slot() : Call fn() on each registered region
+ *
+ * Stop on non-zero return from fn().
+ */
+int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn);
+
+#endif /* QEMU_MEMORY_H */

[Qemu-devel] [RESEND PATCH v3 2/2] RAM API: Make use of it for x86 PC

[Alex Williamson]

Register the actual VM RAM using the new API

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/pc.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index e1b2667..1554164 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
     /* allocate RAM */
     ram_addr = qemu_ram_alloc(NULL, "pc.ram",
                               below_4g_mem_size + above_4g_mem_size);
-    cpu_register_physical_memory(0, 0xa0000, ram_addr);
-    cpu_register_physical_memory(0x100000,
-                 below_4g_mem_size - 0x100000,
-                 ram_addr + 0x100000);
+    qemu_ram_register(0, below_4g_mem_size, ram_addr);
 #if TARGET_PHYS_ADDR_BITS > 32
     if (above_4g_mem_size > 0) {
-        cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size,
-                                     ram_addr + below_4g_mem_size);
+        qemu_ram_register(0x100000000ULL, above_4g_mem_size,
+                          ram_addr + below_4g_mem_size);
     }
 #endif
 

[Qemu-devel] Re: [RESEND PATCH v3 1/2] Minimal RAM API support

[Anthony Liguori]

On 12/13/2010 02:47 PM, Alex Williamson wrote:
> This adds a minimum chunk of Anthony's RAM API support so that we
> can identify actual VM RAM versus all the other things that make
> use of qemu_ram_alloc.
>
> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> ---
>
>   Makefile.objs |    1 +
>   cpu-common.h  |    2 +
>   memory.c      |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   memory.h      |   44 ++++++++++++++++++++++++++
>   4 files changed, 144 insertions(+), 0 deletions(-)
>   create mode 100644 memory.c
>   create mode 100644 memory.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index cebb945..47f3c3a 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -172,6 +172,7 @@ hw-obj-y += pci.o pci_bridge.o msix.o msi.o
>   hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
>   hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
>   hw-obj-y += watchdog.o
> +hw-obj-y += memory.o
>   hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
>   hw-obj-$(CONFIG_ECC) += ecc.o
>   hw-obj-$(CONFIG_NAND) += nand.o
> diff --git a/cpu-common.h b/cpu-common.h
> index 6d4a898..f08f93b 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -29,6 +29,8 @@ enum device_endian {
>   /* address in the RAM (different from a physical address) */
>   typedef unsigned long ram_addr_t;
>
> +#include "memory.h"
> +
>   /* memory API */
>
>   typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
> diff --git a/memory.c b/memory.c
> new file mode 100644
> index 0000000..742776f
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,97 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson<alex.williamson@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +typedef struct ram_slot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    QLIST_ENTRY(ram_slot) next;
> +} ram_slot;
> +
> +static QLIST_HEAD(ram_slots, ram_slot) ram_slots =
> +    QLIST_HEAD_INITIALIZER(ram_slots);
> +
> +static ram_slot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                   ram_addr_t size)
> +{
> +    ram_slot *slot;
> +
> +    QLIST_FOREACH(slot,&ram_slots, next) {
> +        if (slot->start_addr == start_addr&&  slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            hw_error("Ram range overlaps existing slot\n");
> +        }
> +    }
> +
> +    return NULL;
> +}
>
>    

CODING_STYLE.  RamSlot and drop the qemu_ prefix.

> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                      ram_addr_t phys_offset)
> +{
> +    ram_slot *slot;
> +
> +    if (!size) {
> +        return -EINVAL;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(ram_slot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +
> +    return 0;
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    ram_slot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    qemu_free(slot);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}
> +
> +int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn)
> +{
> +    ram_slot *slot;
> +
> +    QLIST_FOREACH(slot,&ram_slots, next) {
> +        int ret = fn(opaque, slot->start_addr, slot->size, slot->offset);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +    return 0;
> +}
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..e7aa5cb
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,44 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson<alex.williamson@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef int (*qemu_ram_for_each_slot_fn)(void *opaque,
> +                                         target_phys_addr_t start_addr,
> +                                         ram_addr_t size,
> +                                         ram_addr_t phys_offset);
> +
> +/**
> + * qemu_ram_register() : Register a region of guest physical memory
> + *
> + * The new region must not overlap an existing region.
> + */
> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                      ram_addr_t phys_offset);
> +
> +/**
> + * qemu_ram_unregister() : Unregister a region of guest physical memory
> + */
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
> +
> +/**
> + * qemu_ram_for_each_slot() : Call fn() on each registered region
> + *
> + * Stop on non-zero return from fn().
> + */
> +int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn);
> +
> +#endif /* QEMU_MEMORY_H */
>
>    

Re: [Qemu-devel] [RESEND PATCH v3 1/2] Minimal RAM API support

[Blue Swirl]

On Mon, Dec 13, 2010 at 8:47 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> This adds a minimum chunk of Anthony's RAM API support so that we
> can identify actual VM RAM versus all the other things that make
> use of qemu_ram_alloc.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
>  Makefile.objs |    1 +
>  cpu-common.h  |    2 +
>  memory.c      |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h      |   44 ++++++++++++++++++++++++++
>  4 files changed, 144 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index cebb945..47f3c3a 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -172,6 +172,7 @@ hw-obj-y += pci.o pci_bridge.o msix.o msi.o
>  hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
>  hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
>  hw-obj-y += watchdog.o
> +hw-obj-y += memory.o
>  hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
>  hw-obj-$(CONFIG_ECC) += ecc.o
>  hw-obj-$(CONFIG_NAND) += nand.o
> diff --git a/cpu-common.h b/cpu-common.h
> index 6d4a898..f08f93b 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -29,6 +29,8 @@ enum device_endian {
>  /* address in the RAM (different from a physical address) */
>  typedef unsigned long ram_addr_t;
>
> +#include "memory.h"
> +
>  /* memory API */
>
>  typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
> diff --git a/memory.c b/memory.c
> new file mode 100644
> index 0000000..742776f
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,97 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +typedef struct ram_slot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    QLIST_ENTRY(ram_slot) next;
> +} ram_slot;

Please see CODING_STYLE for structure naming.

> +
> +static QLIST_HEAD(ram_slots, ram_slot) ram_slots =
> +    QLIST_HEAD_INITIALIZER(ram_slots);
> +
> +static ram_slot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                   ram_addr_t size)
> +{
> +    ram_slot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            hw_error("Ram range overlaps existing slot\n");
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                      ram_addr_t phys_offset)
> +{
> +    ram_slot *slot;
> +
> +    if (!size) {
> +        return -EINVAL;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(ram_slot));

Since you initialize every field by hand later, this could be qemu_malloc().

> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +
> +    return 0;
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    ram_slot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    qemu_free(slot);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;

Useless.