All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Minimal RAM API support
@ 2010-10-29 16:38 ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 16:38 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: kvm, alex.williamson, mst, chrisw, ddutile

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.
Thanks,

Alex

---

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


 Makefile.target |    1 +
 cpu-common.h    |    2 +
 hw/pc.c         |   12 ++++----
 memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h        |   23 +++++++++++++++
 5 files changed, 114 insertions(+), 6 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH 0/2] Minimal RAM API support
@ 2010-10-29 16:38 ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 16:38 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: chrisw, alex.williamson, ddutile, kvm, mst

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.
Thanks,

Alex

---

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


 Makefile.target |    1 +
 cpu-common.h    |    2 +
 hw/pc.c         |   12 ++++----
 memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h        |   23 +++++++++++++++
 5 files changed, 114 insertions(+), 6 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 1/2] Minimal RAM API support
  2010-10-29 16:38 ` [Qemu-devel] " Alex Williamson
@ 2010-10-29 16:39   ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 16:39 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: kvm, alex.williamson, mst, chrisw, ddutile

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.target |    1 +
 cpu-common.h    |    2 +
 memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h        |   23 +++++++++++++++
 4 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

diff --git a/Makefile.target b/Makefile.target
index c48cbcc..e4e2eb4 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
 obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
+obj-y += memory.o
 LIBS+=-lz
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
diff --git a/cpu-common.h b/cpu-common.h
index a543b5d..6aa2738 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -23,6 +23,8 @@
 /* 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..86947fb
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,82 @@
+/*
+ * RAM API
+ *
+ *  Copyright Red Hat, Inc. 2010
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "memory.h"
+#include "range.h"
+
+QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
+
+static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
+                                       ram_addr_t size)
+{
+    QemuRamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots.slots, next) {
+        if (slot->start_addr == start_addr && slot->size == size) {
+            return slot;
+        }
+
+        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
+            abort();
+        }
+    }
+
+    return NULL;
+}
+
+void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                       ram_addr_t phys_offset)
+{
+    QemuRamSlot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    assert(!qemu_ram_find_slot(start_addr, size));
+
+    slot = qemu_mallocz(sizeof(QemuRamSlot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
+
+    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
+}
+
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    QemuRamSlot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    slot = qemu_ram_find_slot(start_addr, size);
+    assert(slot != NULL);
+
+    QLIST_REMOVE(slot, next);
+    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
+
+    return;
+}
diff --git a/memory.h b/memory.h
new file mode 100644
index 0000000..91e552e
--- /dev/null
+++ b/memory.h
@@ -0,0 +1,23 @@
+#ifndef QEMU_MEMORY_H
+#define QEMU_MEMORY_H
+
+#include "qemu-common.h"
+#include "cpu-common.h"
+
+typedef struct QemuRamSlot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    void *host;
+    QLIST_ENTRY(QemuRamSlot) next;
+} QemuRamSlot;
+
+typedef struct QemuRamSlots {
+    QLIST_HEAD(slots, QemuRamSlot) slots;
+} QemuRamSlots;
+extern QemuRamSlots ram_slots;
+
+void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                       ram_addr_t phys_offset);
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+#endif


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH 1/2] Minimal RAM API support
@ 2010-10-29 16:39   ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 16:39 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: chrisw, alex.williamson, ddutile, kvm, mst

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.target |    1 +
 cpu-common.h    |    2 +
 memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h        |   23 +++++++++++++++
 4 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

diff --git a/Makefile.target b/Makefile.target
index c48cbcc..e4e2eb4 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
 obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
+obj-y += memory.o
 LIBS+=-lz
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
diff --git a/cpu-common.h b/cpu-common.h
index a543b5d..6aa2738 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -23,6 +23,8 @@
 /* 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..86947fb
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,82 @@
+/*
+ * RAM API
+ *
+ *  Copyright Red Hat, Inc. 2010
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "memory.h"
+#include "range.h"
+
+QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
+
+static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
+                                       ram_addr_t size)
+{
+    QemuRamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots.slots, next) {
+        if (slot->start_addr == start_addr && slot->size == size) {
+            return slot;
+        }
+
+        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
+            abort();
+        }
+    }
+
+    return NULL;
+}
+
+void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                       ram_addr_t phys_offset)
+{
+    QemuRamSlot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    assert(!qemu_ram_find_slot(start_addr, size));
+
+    slot = qemu_mallocz(sizeof(QemuRamSlot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
+
+    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
+}
+
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    QemuRamSlot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    slot = qemu_ram_find_slot(start_addr, size);
+    assert(slot != NULL);
+
+    QLIST_REMOVE(slot, next);
+    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
+
+    return;
+}
diff --git a/memory.h b/memory.h
new file mode 100644
index 0000000..91e552e
--- /dev/null
+++ b/memory.h
@@ -0,0 +1,23 @@
+#ifndef QEMU_MEMORY_H
+#define QEMU_MEMORY_H
+
+#include "qemu-common.h"
+#include "cpu-common.h"
+
+typedef struct QemuRamSlot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    void *host;
+    QLIST_ENTRY(QemuRamSlot) next;
+} QemuRamSlot;
+
+typedef struct QemuRamSlots {
+    QLIST_HEAD(slots, QemuRamSlot) slots;
+} QemuRamSlots;
+extern QemuRamSlots ram_slots;
+
+void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                       ram_addr_t phys_offset);
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+#endif

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 2/2] RAM API: Make use of it for x86 PC
  2010-10-29 16:38 ` [Qemu-devel] " Alex Williamson
@ 2010-10-29 16:39   ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 16:39 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: kvm, alex.williamson, mst, chrisw, ddutile

Register the actual VM RAM using the new API

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

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

diff --git a/hw/pc.c b/hw/pc.c
index 69b13bf..0ea6d10 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
+    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
+                      ram_addr + 0x100000);
 #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
 


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH 2/2] RAM API: Make use of it for x86 PC
@ 2010-10-29 16:39   ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 16:39 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: chrisw, alex.williamson, ddutile, kvm, mst

Register the actual VM RAM using the new API

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

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

diff --git a/hw/pc.c b/hw/pc.c
index 69b13bf..0ea6d10 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
+    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
+                      ram_addr + 0x100000);
 #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
 

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
  2010-10-29 16:39   ` [Qemu-devel] " Alex Williamson
@ 2010-10-29 19:57     ` Blue Swirl
  -1 siblings, 0 replies; 53+ messages in thread
From: Blue Swirl @ 2010-10-29 19:57 UTC (permalink / raw)
  To: Alex Williamson; +Cc: qemu-devel, anthony, chrisw, ddutile, kvm, mst

On Fri, Oct 29, 2010 at 4:39 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.target |    1 +
>  cpu-common.h    |    2 +
>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h        |   23 +++++++++++++++
>  4 files changed, 108 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
>
> diff --git a/Makefile.target b/Makefile.target
> index c48cbcc..e4e2eb4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>  obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> +obj-y += memory.o

Please move this to Makefile.objs to compile the object in hwlib.
There are no target dependencies.

>  LIBS+=-lz
>
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> diff --git a/cpu-common.h b/cpu-common.h
> index a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>  /* 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..86947fb
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,82 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };

Please avoid global state. This is not used elsewhere, so it could be
static. But instead the API should take a state parameter
(RAMSlotState *) so that no static state is needed.

> +
> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..91e552e
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,23 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    void *host;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;

This definition should be in memory.c.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
@ 2010-10-29 19:57     ` Blue Swirl
  0 siblings, 0 replies; 53+ messages in thread
From: Blue Swirl @ 2010-10-29 19:57 UTC (permalink / raw)
  To: Alex Williamson; +Cc: chrisw, kvm, mst, qemu-devel, ddutile

On Fri, Oct 29, 2010 at 4:39 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.target |    1 +
>  cpu-common.h    |    2 +
>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h        |   23 +++++++++++++++
>  4 files changed, 108 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
>
> diff --git a/Makefile.target b/Makefile.target
> index c48cbcc..e4e2eb4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>  obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> +obj-y += memory.o

Please move this to Makefile.objs to compile the object in hwlib.
There are no target dependencies.

>  LIBS+=-lz
>
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> diff --git a/cpu-common.h b/cpu-common.h
> index a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>  /* 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..86947fb
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,82 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };

Please avoid global state. This is not used elsewhere, so it could be
static. But instead the API should take a state parameter
(RAMSlotState *) so that no static state is needed.

> +
> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..91e552e
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,23 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    void *host;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;

This definition should be in memory.c.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
  2010-10-29 19:57     ` Blue Swirl
@ 2010-10-29 20:15       ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 20:15 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel, anthony, chrisw, ddutile, kvm, mst

On Fri, 2010-10-29 at 19:57 +0000, Blue Swirl wrote:
> On Fri, Oct 29, 2010 at 4:39 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.target |    1 +
> >  cpu-common.h    |    2 +
> >  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  memory.h        |   23 +++++++++++++++
> >  4 files changed, 108 insertions(+), 0 deletions(-)
> >  create mode 100644 memory.c
> >  create mode 100644 memory.h
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index c48cbcc..e4e2eb4 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
> >  obj-y += rwhandler.o
> >  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
> >  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> > +obj-y += memory.o
> 
> Please move this to Makefile.objs to compile the object in hwlib.
> There are no target dependencies.

Ok, will do.

> >  LIBS+=-lz
> >
> >  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> > diff --git a/cpu-common.h b/cpu-common.h
> > index a543b5d..6aa2738 100644
> > --- a/cpu-common.h
> > +++ b/cpu-common.h
> > @@ -23,6 +23,8 @@
> >  /* 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..86947fb
> > --- /dev/null
> > +++ b/memory.c
> > @@ -0,0 +1,82 @@
> > +/*
> > + * RAM API
> > + *
> > + *  Copyright Red Hat, Inc. 2010
> > + *
> > + * Authors:
> > + *  Alex Williamson <alex.williamson@redhat.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#include "memory.h"
> > +#include "range.h"
> > +
> > +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
> 
> Please avoid global state. This is not used elsewhere, so it could be
> static. But instead the API should take a state parameter
> (RAMSlotState *) so that no static state is needed.

The reason for this not being static is that the vfio driver I'm working
on walks it.  Also the reason for the definition being in memory.h
instead of memory.c as you've noted below.  Probably better to solve
that usage by creating an interface that calls a function pointer for
each entry... I'll work on that.  Thanks,

Alex

> > +
> > +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> > +                                       ram_addr_t size)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> > +        if (slot->start_addr == start_addr && slot->size == size) {
> > +            return slot;
> > +        }
> > +
> > +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> > +            abort();
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> > +                       ram_addr_t phys_offset)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    if (!size) {
> > +        return;
> > +    }
> > +
> > +    assert(!qemu_ram_find_slot(start_addr, size));
> > +
> > +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> > +
> > +    slot->start_addr = start_addr;
> > +    slot->size = size;
> > +    slot->offset = phys_offset;
> > +
> > +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> > +
> > +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> > +}
> > +
> > +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    if (!size) {
> > +        return;
> > +    }
> > +
> > +    slot = qemu_ram_find_slot(start_addr, size);
> > +    assert(slot != NULL);
> > +
> > +    QLIST_REMOVE(slot, next);
> > +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> > +
> > +    return;
> > +}
> > diff --git a/memory.h b/memory.h
> > new file mode 100644
> > index 0000000..91e552e
> > --- /dev/null
> > +++ b/memory.h
> > @@ -0,0 +1,23 @@
> > +#ifndef QEMU_MEMORY_H
> > +#define QEMU_MEMORY_H
> > +
> > +#include "qemu-common.h"
> > +#include "cpu-common.h"
> > +
> > +typedef struct QemuRamSlot {
> > +    target_phys_addr_t start_addr;
> > +    ram_addr_t size;
> > +    ram_addr_t offset;
> > +    void *host;
> > +    QLIST_ENTRY(QemuRamSlot) next;
> > +} QemuRamSlot;
> > +
> > +typedef struct QemuRamSlots {
> > +    QLIST_HEAD(slots, QemuRamSlot) slots;
> > +} QemuRamSlots;
> 
> This definition should be in memory.c.




^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
@ 2010-10-29 20:15       ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-10-29 20:15 UTC (permalink / raw)
  To: Blue Swirl; +Cc: chrisw, kvm, mst, qemu-devel, ddutile

On Fri, 2010-10-29 at 19:57 +0000, Blue Swirl wrote:
> On Fri, Oct 29, 2010 at 4:39 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.target |    1 +
> >  cpu-common.h    |    2 +
> >  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  memory.h        |   23 +++++++++++++++
> >  4 files changed, 108 insertions(+), 0 deletions(-)
> >  create mode 100644 memory.c
> >  create mode 100644 memory.h
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index c48cbcc..e4e2eb4 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
> >  obj-y += rwhandler.o
> >  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
> >  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> > +obj-y += memory.o
> 
> Please move this to Makefile.objs to compile the object in hwlib.
> There are no target dependencies.

Ok, will do.

> >  LIBS+=-lz
> >
> >  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> > diff --git a/cpu-common.h b/cpu-common.h
> > index a543b5d..6aa2738 100644
> > --- a/cpu-common.h
> > +++ b/cpu-common.h
> > @@ -23,6 +23,8 @@
> >  /* 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..86947fb
> > --- /dev/null
> > +++ b/memory.c
> > @@ -0,0 +1,82 @@
> > +/*
> > + * RAM API
> > + *
> > + *  Copyright Red Hat, Inc. 2010
> > + *
> > + * Authors:
> > + *  Alex Williamson <alex.williamson@redhat.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#include "memory.h"
> > +#include "range.h"
> > +
> > +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
> 
> Please avoid global state. This is not used elsewhere, so it could be
> static. But instead the API should take a state parameter
> (RAMSlotState *) so that no static state is needed.

The reason for this not being static is that the vfio driver I'm working
on walks it.  Also the reason for the definition being in memory.h
instead of memory.c as you've noted below.  Probably better to solve
that usage by creating an interface that calls a function pointer for
each entry... I'll work on that.  Thanks,

Alex

> > +
> > +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> > +                                       ram_addr_t size)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> > +        if (slot->start_addr == start_addr && slot->size == size) {
> > +            return slot;
> > +        }
> > +
> > +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> > +            abort();
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> > +                       ram_addr_t phys_offset)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    if (!size) {
> > +        return;
> > +    }
> > +
> > +    assert(!qemu_ram_find_slot(start_addr, size));
> > +
> > +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> > +
> > +    slot->start_addr = start_addr;
> > +    slot->size = size;
> > +    slot->offset = phys_offset;
> > +
> > +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> > +
> > +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> > +}
> > +
> > +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    if (!size) {
> > +        return;
> > +    }
> > +
> > +    slot = qemu_ram_find_slot(start_addr, size);
> > +    assert(slot != NULL);
> > +
> > +    QLIST_REMOVE(slot, next);
> > +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> > +
> > +    return;
> > +}
> > diff --git a/memory.h b/memory.h
> > new file mode 100644
> > index 0000000..91e552e
> > --- /dev/null
> > +++ b/memory.h
> > @@ -0,0 +1,23 @@
> > +#ifndef QEMU_MEMORY_H
> > +#define QEMU_MEMORY_H
> > +
> > +#include "qemu-common.h"
> > +#include "cpu-common.h"
> > +
> > +typedef struct QemuRamSlot {
> > +    target_phys_addr_t start_addr;
> > +    ram_addr_t size;
> > +    ram_addr_t offset;
> > +    void *host;
> > +    QLIST_ENTRY(QemuRamSlot) next;
> > +} QemuRamSlot;
> > +
> > +typedef struct QemuRamSlots {
> > +    QLIST_HEAD(slots, QemuRamSlot) slots;
> > +} QemuRamSlots;
> 
> This definition should be in memory.c.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
  2010-10-29 16:39   ` [Qemu-devel] " Alex Williamson
@ 2010-11-01  2:17     ` Isaku Yamahata
  -1 siblings, 0 replies; 53+ messages in thread
From: Isaku Yamahata @ 2010-11-01  2:17 UTC (permalink / raw)
  To: Alex Williamson; +Cc: qemu-devel, anthony, chrisw, ddutile, kvm, mst

On Fri, Oct 29, 2010 at 10:39:03AM -0600, 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.target |    1 +
>  cpu-common.h    |    2 +
>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h        |   23 +++++++++++++++
>  4 files changed, 108 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index c48cbcc..e4e2eb4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>  obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> +obj-y += memory.o
>  LIBS+=-lz
>  
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> diff --git a/cpu-common.h b/cpu-common.h
> index a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>  /* 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..86947fb
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,82 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
> +
> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}

qemu_free(slot) is necessary.


> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..91e552e
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,23 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    void *host;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;
> +extern QemuRamSlots ram_slots;
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset);
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
> +#endif
> 
> 

-- 
yamahata

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
@ 2010-11-01  2:17     ` Isaku Yamahata
  0 siblings, 0 replies; 53+ messages in thread
From: Isaku Yamahata @ 2010-11-01  2:17 UTC (permalink / raw)
  To: Alex Williamson; +Cc: chrisw, kvm, mst, qemu-devel, ddutile

On Fri, Oct 29, 2010 at 10:39:03AM -0600, 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.target |    1 +
>  cpu-common.h    |    2 +
>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h        |   23 +++++++++++++++
>  4 files changed, 108 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index c48cbcc..e4e2eb4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>  obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> +obj-y += memory.o
>  LIBS+=-lz
>  
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> diff --git a/cpu-common.h b/cpu-common.h
> index a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>  /* 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..86947fb
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,82 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
> +
> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}

qemu_free(slot) is necessary.


> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..91e552e
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,23 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    void *host;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;
> +extern QemuRamSlots ram_slots;
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset);
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
> +#endif
> 
> 

-- 
yamahata

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
  2010-11-01  2:17     ` Isaku Yamahata
@ 2010-11-01  2:32       ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01  2:32 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: chrisw, kvm, mst, qemu-devel, ddutile

On Sun, Oct 31, 2010 at 8:17 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Fri, Oct 29, 2010 at 10:39:03AM -0600, 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.target |    1 +
>>  cpu-common.h    |    2 +
>>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  memory.h        |   23 +++++++++++++++
>>  4 files changed, 108 insertions(+), 0 deletions(-)
>>  create mode 100644 memory.c
>>  create mode 100644 memory.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index c48cbcc..e4e2eb4 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>>  obj-y += rwhandler.o
>>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
>> +obj-y += memory.o
>>  LIBS+=-lz
>>
>>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>> diff --git a/cpu-common.h b/cpu-common.h
>> index a543b5d..6aa2738 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -23,6 +23,8 @@
>>  /* 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..86947fb
>> --- /dev/null
>> +++ b/memory.c
>> @@ -0,0 +1,82 @@
>> +/*
>> + * RAM API
>> + *
>> + *  Copyright Red Hat, Inc. 2010
>> + *
>> + * Authors:
>> + *  Alex Williamson <alex.williamson@redhat.com>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include "memory.h"
>> +#include "range.h"
>> +
>> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
>> +
>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>> +                                       ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
>> +        if (slot->start_addr == start_addr && slot->size == size) {
>> +            return slot;
>> +        }
>> +
>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>> +            abort();
>> +        }
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
>> +                       ram_addr_t phys_offset)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>> +    assert(!qemu_ram_find_slot(start_addr, size));
>> +
>> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
>> +
>> +    slot->start_addr = start_addr;
>> +    slot->size = size;
>> +    slot->offset = phys_offset;
>> +
>> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
>> +
>> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
>> +}
>> +
>> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>> +    slot = qemu_ram_find_slot(start_addr, size);
>> +    assert(slot != NULL);
>> +
>> +    QLIST_REMOVE(slot, next);
>> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
>> +
>> +    return;
>> +}
>
> qemu_free(slot) is necessary.

Thank you!

Alex

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Minimal RAM API support
@ 2010-11-01  2:32       ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01  2:32 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: chrisw, ddutile, qemu-devel, kvm, mst

On Sun, Oct 31, 2010 at 8:17 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Fri, Oct 29, 2010 at 10:39:03AM -0600, 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.target |    1 +
>>  cpu-common.h    |    2 +
>>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  memory.h        |   23 +++++++++++++++
>>  4 files changed, 108 insertions(+), 0 deletions(-)
>>  create mode 100644 memory.c
>>  create mode 100644 memory.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index c48cbcc..e4e2eb4 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>>  obj-y += rwhandler.o
>>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
>> +obj-y += memory.o
>>  LIBS+=-lz
>>
>>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>> diff --git a/cpu-common.h b/cpu-common.h
>> index a543b5d..6aa2738 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -23,6 +23,8 @@
>>  /* 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..86947fb
>> --- /dev/null
>> +++ b/memory.c
>> @@ -0,0 +1,82 @@
>> +/*
>> + * RAM API
>> + *
>> + *  Copyright Red Hat, Inc. 2010
>> + *
>> + * Authors:
>> + *  Alex Williamson <alex.williamson@redhat.com>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include "memory.h"
>> +#include "range.h"
>> +
>> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
>> +
>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>> +                                       ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
>> +        if (slot->start_addr == start_addr && slot->size == size) {
>> +            return slot;
>> +        }
>> +
>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>> +            abort();
>> +        }
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
>> +                       ram_addr_t phys_offset)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>> +    assert(!qemu_ram_find_slot(start_addr, size));
>> +
>> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
>> +
>> +    slot->start_addr = start_addr;
>> +    slot->size = size;
>> +    slot->offset = phys_offset;
>> +
>> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
>> +
>> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
>> +}
>> +
>> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>> +    slot = qemu_ram_find_slot(start_addr, size);
>> +    assert(slot != NULL);
>> +
>> +    QLIST_REMOVE(slot, next);
>> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
>> +
>> +    return;
>> +}
>
> qemu_free(slot) is necessary.

Thank you!

Alex

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 0/2] Minimal RAM API support
  2010-10-29 16:38 ` [Qemu-devel] " Alex Williamson
@ 2010-11-01 15:13   ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01 15:13 UTC (permalink / raw)
  To: qemu-devel, anthony, blauwirbel
  Cc: kvm, alex.williamson, mst, chrisw, ddutile

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.  Thanks,

Alex

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.
Thanks,

Alex

---

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       |   12 +++---
 memory.c      |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h      |   18 +++++++++
 5 files changed, 136 insertions(+), 6 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH v2 0/2] Minimal RAM API support
@ 2010-11-01 15:13   ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01 15:13 UTC (permalink / raw)
  To: qemu-devel, anthony, blauwirbel
  Cc: chrisw, alex.williamson, ddutile, kvm, mst

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.  Thanks,

Alex

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.
Thanks,

Alex

---

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       |   12 +++---
 memory.c      |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h      |   18 +++++++++
 5 files changed, 136 insertions(+), 6 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 1/2] Minimal RAM API support
  2010-11-01 15:13   ` [Qemu-devel] " Alex Williamson
@ 2010-11-01 15:14     ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01 15:14 UTC (permalink / raw)
  To: qemu-devel, anthony, blauwirbel
  Cc: kvm, alex.williamson, mst, chrisw, ddutile

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      |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h      |   18 +++++++++
 4 files changed, 130 insertions(+), 0 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

diff --git a/Makefile.objs b/Makefile.objs
index f07fb01..33fae0b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -154,6 +154,7 @@ hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.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 a543b5d..6aa2738 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -23,6 +23,8 @@
 /* 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..2895082
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,109 @@
+/*
+ * RAM API
+ *
+ *  Copyright Red Hat, Inc. 2010
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "memory.h"
+#include "range.h"
+
+typedef struct QemuRamSlot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    QLIST_ENTRY(QemuRamSlot) next;
+} QemuRamSlot;
+
+typedef struct QemuRamSlots {
+    QLIST_HEAD(slots, QemuRamSlot) slots;
+} QemuRamSlots;
+
+static QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
+
+static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
+                                       ram_addr_t size)
+{
+    QemuRamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots.slots, next) {
+        if (slot->start_addr == start_addr && slot->size == size) {
+            return slot;
+        }
+
+        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
+            abort();
+        }
+    }
+
+    return NULL;
+}
+
+int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                       ram_addr_t phys_offset)
+{
+    QemuRamSlot *slot;
+
+    if (!size) {
+        return -EINVAL;
+    }
+
+    assert(!qemu_ram_find_slot(start_addr, size));
+
+    slot = qemu_mallocz(sizeof(QemuRamSlot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slots.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)
+{
+    QemuRamSlot *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)
+{
+    QemuRamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots.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..0c17ff9
--- /dev/null
+++ b/memory.h
@@ -0,0 +1,18 @@
+#ifndef QEMU_MEMORY_H
+#define QEMU_MEMORY_H
+
+#include "qemu-common.h"
+#include "cpu-common.h"
+
+int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                      ram_addr_t phys_offset);
+
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+
+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);
+
+int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn);
+#endif


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
@ 2010-11-01 15:14     ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01 15:14 UTC (permalink / raw)
  To: qemu-devel, anthony, blauwirbel
  Cc: chrisw, alex.williamson, ddutile, kvm, mst

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      |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h      |   18 +++++++++
 4 files changed, 130 insertions(+), 0 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

diff --git a/Makefile.objs b/Makefile.objs
index f07fb01..33fae0b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -154,6 +154,7 @@ hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.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 a543b5d..6aa2738 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -23,6 +23,8 @@
 /* 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..2895082
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,109 @@
+/*
+ * RAM API
+ *
+ *  Copyright Red Hat, Inc. 2010
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "memory.h"
+#include "range.h"
+
+typedef struct QemuRamSlot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    QLIST_ENTRY(QemuRamSlot) next;
+} QemuRamSlot;
+
+typedef struct QemuRamSlots {
+    QLIST_HEAD(slots, QemuRamSlot) slots;
+} QemuRamSlots;
+
+static QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
+
+static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
+                                       ram_addr_t size)
+{
+    QemuRamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots.slots, next) {
+        if (slot->start_addr == start_addr && slot->size == size) {
+            return slot;
+        }
+
+        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
+            abort();
+        }
+    }
+
+    return NULL;
+}
+
+int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                       ram_addr_t phys_offset)
+{
+    QemuRamSlot *slot;
+
+    if (!size) {
+        return -EINVAL;
+    }
+
+    assert(!qemu_ram_find_slot(start_addr, size));
+
+    slot = qemu_mallocz(sizeof(QemuRamSlot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slots.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)
+{
+    QemuRamSlot *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)
+{
+    QemuRamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slots.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..0c17ff9
--- /dev/null
+++ b/memory.h
@@ -0,0 +1,18 @@
+#ifndef QEMU_MEMORY_H
+#define QEMU_MEMORY_H
+
+#include "qemu-common.h"
+#include "cpu-common.h"
+
+int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                      ram_addr_t phys_offset);
+
+void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+
+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);
+
+int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn);
+#endif

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-01 15:13   ` [Qemu-devel] " Alex Williamson
@ 2010-11-01 15:14     ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01 15:14 UTC (permalink / raw)
  To: qemu-devel, anthony, blauwirbel
  Cc: kvm, alex.williamson, mst, chrisw, ddutile

Register the actual VM RAM using the new API

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

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

diff --git a/hw/pc.c b/hw/pc.c
index 69b13bf..0ea6d10 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
+    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
+                      ram_addr + 0x100000);
 #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
 


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-01 15:14     ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-01 15:14 UTC (permalink / raw)
  To: qemu-devel, anthony, blauwirbel
  Cc: chrisw, alex.williamson, ddutile, kvm, mst

Register the actual VM RAM using the new API

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

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

diff --git a/hw/pc.c b/hw/pc.c
index 69b13bf..0ea6d10 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
+    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
+                      ram_addr + 0x100000);
 #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
 

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
  2010-11-01 15:14     ` [Qemu-devel] " Alex Williamson
@ 2010-11-16 14:55       ` Anthony Liguori
  -1 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-16 14:55 UTC (permalink / raw)
  To: Alex Williamson; +Cc: qemu-devel, blauwirbel, chrisw, ddutile, kvm, mst

On 11/01/2010 10:14 AM, 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      |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   memory.h      |   18 +++++++++
>   4 files changed, 130 insertions(+), 0 deletions(-)
>   create mode 100644 memory.c
>   create mode 100644 memory.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index f07fb01..33fae0b 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -154,6 +154,7 @@ hw-obj-y += vl.o loader.o
>   hw-obj-y += virtio.o virtio-console.o
>   hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.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 a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>   /* 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..2895082
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,109 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson<alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;
>    

No need for all of the 'Qemu' prefixes.

> +
> +static QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
>    

Might be nicer to just typedef the extra struct away.

> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot,&ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr&&  slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
>    

Should display a message before aborting.

> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return -EINVAL;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.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)
> +{
> +    QemuRamSlot *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)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot,&ram_slots.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..0c17ff9
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,18 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
>    

Header needs copyright and would be nice to have some comments 
explaining these functions.

Regards,

Anthony Liguori

> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                      ram_addr_t phys_offset);
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
> +
> +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);
> +
> +int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn);
> +#endif
>
>
>
>    


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
@ 2010-11-16 14:55       ` Anthony Liguori
  0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-16 14:55 UTC (permalink / raw)
  To: Alex Williamson; +Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/01/2010 10:14 AM, 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      |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   memory.h      |   18 +++++++++
>   4 files changed, 130 insertions(+), 0 deletions(-)
>   create mode 100644 memory.c
>   create mode 100644 memory.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index f07fb01..33fae0b 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -154,6 +154,7 @@ hw-obj-y += vl.o loader.o
>   hw-obj-y += virtio.o virtio-console.o
>   hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.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 a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>   /* 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..2895082
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,109 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson<alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;
>    

No need for all of the 'Qemu' prefixes.

> +
> +static QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
>    

Might be nicer to just typedef the extra struct away.

> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot,&ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr&&  slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
>    

Should display a message before aborting.

> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return -EINVAL;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.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)
> +{
> +    QemuRamSlot *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)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot,&ram_slots.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..0c17ff9
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,18 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
>    

Header needs copyright and would be nice to have some comments 
explaining these functions.

Regards,

Anthony Liguori

> +int qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                      ram_addr_t phys_offset);
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
> +
> +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);
> +
> +int qemu_ram_for_each_slot(void *opaque, qemu_ram_for_each_slot_fn fn);
> +#endif
>
>
>
>    

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-01 15:14     ` [Qemu-devel] " Alex Williamson
@ 2010-11-16 14:58       ` Anthony Liguori
  -1 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-16 14:58 UTC (permalink / raw)
  To: Alex Williamson; +Cc: qemu-devel, blauwirbel, chrisw, ddutile, kvm, mst

On 11/01/2010 10:14 AM, Alex Williamson wrote:
> Register the actual VM RAM using the new API
>
> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> ---
>
>   hw/pc.c |   12 ++++++------
>   1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 69b13bf..0ea6d10 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
> +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
> +                      ram_addr + 0x100000);
>   #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);
>       }
>    

Take a look at the memory shadowing in the i440fx.  The regions of 
memory in the BIOS area can temporarily become RAM.

That's because there is normally RAM backing this space but the memory 
controller redirects writes to the ROM space.

Not sure the best way to handle this, but the basic concept is, RAM 
always exists but if a device tries to access it, it may or may not be 
accessible as RAM at any given point in time.

Regards,

Anthony Liguori

>   #endif
>
>
>
>
>    


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-16 14:58       ` Anthony Liguori
  0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-16 14:58 UTC (permalink / raw)
  To: Alex Williamson; +Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/01/2010 10:14 AM, Alex Williamson wrote:
> Register the actual VM RAM using the new API
>
> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> ---
>
>   hw/pc.c |   12 ++++++------
>   1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 69b13bf..0ea6d10 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
> +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
> +                      ram_addr + 0x100000);
>   #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);
>       }
>    

Take a look at the memory shadowing in the i440fx.  The regions of 
memory in the BIOS area can temporarily become RAM.

That's because there is normally RAM backing this space but the memory 
controller redirects writes to the ROM space.

Not sure the best way to handle this, but the basic concept is, RAM 
always exists but if a device tries to access it, it may or may not be 
accessible as RAM at any given point in time.

Regards,

Anthony Liguori

>   #endif
>
>
>
>
>    

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
  2010-11-16 14:55       ` Anthony Liguori
@ 2010-11-16 15:02         ` Alexander Graf
  -1 siblings, 0 replies; 53+ messages in thread
From: Alexander Graf @ 2010-11-16 15:02 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile


On 16.11.2010, at 15:55, Anthony Liguori wrote:

> On 11/01/2010 10:14 AM, 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>
>> ---
> 

[...]

> 
>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>> +                                       ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    QLIST_FOREACH(slot,&ram_slots.slots, next) {
>> +        if (slot->start_addr == start_addr&&  slot->size == size) {
>> +            return slot;
>> +        }
>> +
>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>> +            abort();
>>   
> 
> Should display a message before aborting.

Why not use hw_error?


Alex

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
@ 2010-11-16 15:02         ` Alexander Graf
  0 siblings, 0 replies; 53+ messages in thread
From: Alexander Graf @ 2010-11-16 15:02 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile


On 16.11.2010, at 15:55, Anthony Liguori wrote:

> On 11/01/2010 10:14 AM, 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>
>> ---
> 

[...]

> 
>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>> +                                       ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    QLIST_FOREACH(slot,&ram_slots.slots, next) {
>> +        if (slot->start_addr == start_addr&&  slot->size == size) {
>> +            return slot;
>> +        }
>> +
>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>> +            abort();
>>   
> 
> Should display a message before aborting.

Why not use hw_error?


Alex

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
  2010-11-16 15:02         ` Alexander Graf
@ 2010-11-16 15:08           ` Anthony Liguori
  -1 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-16 15:08 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/16/2010 09:02 AM, Alexander Graf wrote:
>>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>>> +                                       ram_addr_t size)
>>> +{
>>> +    QemuRamSlot *slot;
>>> +
>>> +    QLIST_FOREACH(slot,&ram_slots.slots, next) {
>>> +        if (slot->start_addr == start_addr&&   slot->size == size) {
>>> +            return slot;
>>> +        }
>>> +
>>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>>> +            abort();
>>>
>>>        
>> Should display a message before aborting.
>>      
> Why not use hw_error?
>    

Another good suggestion.

Regards,

Anthony Liguori

> Alex--
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] Minimal RAM API support
@ 2010-11-16 15:08           ` Anthony Liguori
  0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-16 15:08 UTC (permalink / raw)
  To: Alexander Graf
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On 11/16/2010 09:02 AM, Alexander Graf wrote:
>>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>>> +                                       ram_addr_t size)
>>> +{
>>> +    QemuRamSlot *slot;
>>> +
>>> +    QLIST_FOREACH(slot,&ram_slots.slots, next) {
>>> +        if (slot->start_addr == start_addr&&   slot->size == size) {
>>> +            return slot;
>>> +        }
>>> +
>>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>>> +            abort();
>>>
>>>        
>> Should display a message before aborting.
>>      
> Why not use hw_error?
>    

Another good suggestion.

Regards,

Anthony Liguori

> Alex--
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-16 14:58       ` Anthony Liguori
@ 2010-11-16 21:24         ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-16 21:24 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel, blauwirbel, chrisw, ddutile, kvm, mst

On Tue, 2010-11-16 at 08:58 -0600, Anthony Liguori wrote:
> On 11/01/2010 10:14 AM, Alex Williamson wrote:
> > Register the actual VM RAM using the new API
> >
> > Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> > ---
> >
> >   hw/pc.c |   12 ++++++------
> >   1 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 69b13bf..0ea6d10 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
> > +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
> > +                      ram_addr + 0x100000);
> >   #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);
> >       }
> >    
> 
> Take a look at the memory shadowing in the i440fx.  The regions of 
> memory in the BIOS area can temporarily become RAM.
> 
> That's because there is normally RAM backing this space but the memory 
> controller redirects writes to the ROM space.
> 
> Not sure the best way to handle this, but the basic concept is, RAM 
> always exists but if a device tries to access it, it may or may not be 
> accessible as RAM at any given point in time.

Gack.  For the benefit of those that want to join the fun without
digging up the spec, these magic flippable segments the i440fx can
toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
64k segment from 0xf0000 to 0xfffff.  There are read-enable and
write-enable bits for each, so the chipset can be configured to read
from the bios and write to memory (to setup BIOS-RAM caching), and read
from memory and write to the bios (to enable BIOS-RAM caching).  The
other bit combinations are also available.

For my purpose in using this to program the IOMMU with guest physical to
host virtual addresses for device assignment, it doesn't really matter
since there should never be a DMA in this range of memory.  But for a
general RAM API, I'm not sure either.  I'm tempted to say that while
this is in fact a use of RAM, the RAM is never presented to the guest as
usable system memory (E820_RAM for x86), and should therefore be
excluded from the RAM API if we're using it only to track regions that
are actual guest usable physical memory.

We had talked on irc that pc.c should be registering 0x0 to
below_4g_mem_size as ram, but now I tend to disagree with that.  The
memory backing 0xa0000-0x100000 is present, but it's not presented to
the guest as usable RAM.  What's your strict definition of what the RAM
API includes?  Is it only what the guest could consider usable RAM or
does it also include quirky chipset accelerator features like this
(everything with a guest physical address)?  Thanks,

Alex


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-16 21:24         ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-16 21:24 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On Tue, 2010-11-16 at 08:58 -0600, Anthony Liguori wrote:
> On 11/01/2010 10:14 AM, Alex Williamson wrote:
> > Register the actual VM RAM using the new API
> >
> > Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> > ---
> >
> >   hw/pc.c |   12 ++++++------
> >   1 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 69b13bf..0ea6d10 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
> > +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
> > +                      ram_addr + 0x100000);
> >   #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);
> >       }
> >    
> 
> Take a look at the memory shadowing in the i440fx.  The regions of 
> memory in the BIOS area can temporarily become RAM.
> 
> That's because there is normally RAM backing this space but the memory 
> controller redirects writes to the ROM space.
> 
> Not sure the best way to handle this, but the basic concept is, RAM 
> always exists but if a device tries to access it, it may or may not be 
> accessible as RAM at any given point in time.

Gack.  For the benefit of those that want to join the fun without
digging up the spec, these magic flippable segments the i440fx can
toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
64k segment from 0xf0000 to 0xfffff.  There are read-enable and
write-enable bits for each, so the chipset can be configured to read
from the bios and write to memory (to setup BIOS-RAM caching), and read
from memory and write to the bios (to enable BIOS-RAM caching).  The
other bit combinations are also available.

For my purpose in using this to program the IOMMU with guest physical to
host virtual addresses for device assignment, it doesn't really matter
since there should never be a DMA in this range of memory.  But for a
general RAM API, I'm not sure either.  I'm tempted to say that while
this is in fact a use of RAM, the RAM is never presented to the guest as
usable system memory (E820_RAM for x86), and should therefore be
excluded from the RAM API if we're using it only to track regions that
are actual guest usable physical memory.

We had talked on irc that pc.c should be registering 0x0 to
below_4g_mem_size as ram, but now I tend to disagree with that.  The
memory backing 0xa0000-0x100000 is present, but it's not presented to
the guest as usable RAM.  What's your strict definition of what the RAM
API includes?  Is it only what the guest could consider usable RAM or
does it also include quirky chipset accelerator features like this
(everything with a guest physical address)?  Thanks,

Alex

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-16 21:24         ` Alex Williamson
@ 2010-11-17  9:31           ` Gleb Natapov
  -1 siblings, 0 replies; 53+ messages in thread
From: Gleb Natapov @ 2010-11-17  9:31 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Anthony Liguori, qemu-devel, blauwirbel, chrisw, ddutile, kvm, mst

On Tue, Nov 16, 2010 at 02:24:06PM -0700, Alex Williamson wrote:
> On Tue, 2010-11-16 at 08:58 -0600, Anthony Liguori wrote:
> > On 11/01/2010 10:14 AM, Alex Williamson wrote:
> > > Register the actual VM RAM using the new API
> > >
> > > Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> > > ---
> > >
> > >   hw/pc.c |   12 ++++++------
> > >   1 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hw/pc.c b/hw/pc.c
> > > index 69b13bf..0ea6d10 100644
> > > --- a/hw/pc.c
> > > +++ b/hw/pc.c
> > > @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
> > > +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
> > > +                      ram_addr + 0x100000);
> > >   #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);
> > >       }
> > >    
> > 
> > Take a look at the memory shadowing in the i440fx.  The regions of 
> > memory in the BIOS area can temporarily become RAM.
> > 
> > That's because there is normally RAM backing this space but the memory 
> > controller redirects writes to the ROM space.
> > 
> > Not sure the best way to handle this, but the basic concept is, RAM 
> > always exists but if a device tries to access it, it may or may not be 
> > accessible as RAM at any given point in time.
> 
> Gack.  For the benefit of those that want to join the fun without
> digging up the spec, these magic flippable segments the i440fx can
> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
> write-enable bits for each, so the chipset can be configured to read
> from the bios and write to memory (to setup BIOS-RAM caching), and read
> from memory and write to the bios (to enable BIOS-RAM caching).  The
> other bit combinations are also available.
> 
There is also 0xa0000−0xbffff which is usually part of framebuffer, but
chipset can be configured to access this memory as RAM when CPU is in
SMM mode.

> For my purpose in using this to program the IOMMU with guest physical to
> host virtual addresses for device assignment, it doesn't really matter
> since there should never be a DMA in this range of memory.  But for a
IIRC spec defines for each range of memory if it is accessed from PCI bus.

> general RAM API, I'm not sure either.  I'm tempted to say that while
> this is in fact a use of RAM, the RAM is never presented to the guest as
> usable system memory (E820_RAM for x86), and should therefore be
> excluded from the RAM API if we're using it only to track regions that
> are actual guest usable physical memory.
A guest is no only OS (like Windows or Linux), but the bios code is also part
of the guest and it can access all of this memory.

> 
> We had talked on irc that pc.c should be registering 0x0 to
> below_4g_mem_size as ram, but now I tend to disagree with that.  The
> memory backing 0xa0000-0x100000 is present, but it's not presented to
> the guest as usable RAM.
It is, during SMM, if bios configured chipset to do so.
 
>                          What's your strict definition of what the RAM
> API includes?  Is it only what the guest could consider usable RAM or
> does it also include quirky chipset accelerator features like this
> (everything with a guest physical address)?  Thanks,
> 

--
			Gleb.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-17  9:31           ` Gleb Natapov
  0 siblings, 0 replies; 53+ messages in thread
From: Gleb Natapov @ 2010-11-17  9:31 UTC (permalink / raw)
  To: Alex Williamson; +Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On Tue, Nov 16, 2010 at 02:24:06PM -0700, Alex Williamson wrote:
> On Tue, 2010-11-16 at 08:58 -0600, Anthony Liguori wrote:
> > On 11/01/2010 10:14 AM, Alex Williamson wrote:
> > > Register the actual VM RAM using the new API
> > >
> > > Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> > > ---
> > >
> > >   hw/pc.c |   12 ++++++------
> > >   1 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hw/pc.c b/hw/pc.c
> > > index 69b13bf..0ea6d10 100644
> > > --- a/hw/pc.c
> > > +++ b/hw/pc.c
> > > @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
> > > +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
> > > +                      ram_addr + 0x100000);
> > >   #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);
> > >       }
> > >    
> > 
> > Take a look at the memory shadowing in the i440fx.  The regions of 
> > memory in the BIOS area can temporarily become RAM.
> > 
> > That's because there is normally RAM backing this space but the memory 
> > controller redirects writes to the ROM space.
> > 
> > Not sure the best way to handle this, but the basic concept is, RAM 
> > always exists but if a device tries to access it, it may or may not be 
> > accessible as RAM at any given point in time.
> 
> Gack.  For the benefit of those that want to join the fun without
> digging up the spec, these magic flippable segments the i440fx can
> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
> write-enable bits for each, so the chipset can be configured to read
> from the bios and write to memory (to setup BIOS-RAM caching), and read
> from memory and write to the bios (to enable BIOS-RAM caching).  The
> other bit combinations are also available.
> 
There is also 0xa0000−0xbffff which is usually part of framebuffer, but
chipset can be configured to access this memory as RAM when CPU is in
SMM mode.

> For my purpose in using this to program the IOMMU with guest physical to
> host virtual addresses for device assignment, it doesn't really matter
> since there should never be a DMA in this range of memory.  But for a
IIRC spec defines for each range of memory if it is accessed from PCI bus.

> general RAM API, I'm not sure either.  I'm tempted to say that while
> this is in fact a use of RAM, the RAM is never presented to the guest as
> usable system memory (E820_RAM for x86), and should therefore be
> excluded from the RAM API if we're using it only to track regions that
> are actual guest usable physical memory.
A guest is no only OS (like Windows or Linux), but the bios code is also part
of the guest and it can access all of this memory.

> 
> We had talked on irc that pc.c should be registering 0x0 to
> below_4g_mem_size as ram, but now I tend to disagree with that.  The
> memory backing 0xa0000-0x100000 is present, but it's not presented to
> the guest as usable RAM.
It is, during SMM, if bios configured chipset to do so.
 
>                          What's your strict definition of what the RAM
> API includes?  Is it only what the guest could consider usable RAM or
> does it also include quirky chipset accelerator features like this
> (everything with a guest physical address)?  Thanks,
> 

--
			Gleb.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-16 21:24         ` Alex Williamson
  (?)
  (?)
@ 2010-11-17 23:42         ` Anthony Liguori
  2010-11-18 15:22             ` Avi Kivity
  2010-11-18 15:51             ` Gleb Natapov
  -1 siblings, 2 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-17 23:42 UTC (permalink / raw)
  To: Alex Williamson; +Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/16/2010 03:24 PM, Alex Williamson wrote:
> On Tue, 2010-11-16 at 08:58 -0600, Anthony Liguori wrote:
>    
>> On 11/01/2010 10:14 AM, Alex Williamson wrote:
>>      
>>> Register the actual VM RAM using the new API
>>>
>>> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
>>> ---
>>>
>>>    hw/pc.c |   12 ++++++------
>>>    1 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 69b13bf..0ea6d10 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -912,14 +912,14 @@ 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, 0xa0000, ram_addr);
>>> +    qemu_ram_register(0x100000, below_4g_mem_size - 0x100000,
>>> +                      ram_addr + 0x100000);
>>>    #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);
>>>        }
>>>
>>>        
>> Take a look at the memory shadowing in the i440fx.  The regions of
>> memory in the BIOS area can temporarily become RAM.
>>
>> That's because there is normally RAM backing this space but the memory
>> controller redirects writes to the ROM space.
>>
>> Not sure the best way to handle this, but the basic concept is, RAM
>> always exists but if a device tries to access it, it may or may not be
>> accessible as RAM at any given point in time.
>>      
> Gack.  For the benefit of those that want to join the fun without
> digging up the spec, these magic flippable segments the i440fx can
> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
> write-enable bits for each, so the chipset can be configured to read
> from the bios and write to memory (to setup BIOS-RAM caching), and read
> from memory and write to the bios (to enable BIOS-RAM caching).  The
> other bit combinations are also available.
>    

Yup.  As Gleb mentions, there's the SDRAM register which controls 
whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but KVM 
explicitly disabled SMM support).

> For my purpose in using this to program the IOMMU with guest physical to
> host virtual addresses for device assignment, it doesn't really matter
> since there should never be a DMA in this range of memory.  But for a
> general RAM API, I'm not sure either.  I'm tempted to say that while
> this is in fact a use of RAM, the RAM is never presented to the guest as
> usable system memory (E820_RAM for x86), and should therefore be
> excluded from the RAM API if we're using it only to track regions that
> are actual guest usable physical memory.
>
> We had talked on irc that pc.c should be registering 0x0 to
> below_4g_mem_size as ram, but now I tend to disagree with that.  The
> memory backing 0xa0000-0x100000 is present, but it's not presented to
> the guest as usable RAM.  What's your strict definition of what the RAM
> API includes?  Is it only what the guest could consider usable RAM or
> does it also include quirky chipset accelerator features like this
> (everything with a guest physical address)?  Thanks,
>    

Today we model on flat space that's a mixed of device memory, RAM, or 
ROM.  This is not how machines work and the limitations of this model is 
holding us back.

IRL, there's a block of RAM that's connected to a memory controller.  
The CPU is also connected to the memory controller.  Devices are 
connected to another controller which is in turn connected to the memory 
controller.  There may, in fact, be more than one controller between a 
device and the memory controller.

A controller may change the way a device sees memory in arbitrary ways.  
In fact, two controllers accessing the same page might see something 
totally different.

The idea behind the RAM API is to begin to establish this hierarchy.  
RAM is not what any particular device sees--it's actual RAM.  IOW, the 
RAM API should represent what address mapping I would get if I talked 
directly to DIMMs.

This is not what RamBlock is even though the name would suggest 
otherwise.  RamBlocks are anything that qemu represents as cache 
consistency directly accessable memory.  Device ROMs and areas of device 
RAM are all allocated from the RamBlock space.

So the very first task of a RAM API is to simplify differentiate these 
two things.  Once we have the base RAM API, we can start adding the 
proper APIs that sit on top of it (like a PCI memory API).

Regards,

Anthony Liguori

> Alex
>
>
>    


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-17 23:42         ` Anthony Liguori
@ 2010-11-18 15:22             ` Avi Kivity
  2010-11-18 15:51             ` Gleb Natapov
  1 sibling, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-11-18 15:22 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>> Gack.  For the benefit of those that want to join the fun without
>> digging up the spec, these magic flippable segments the i440fx can
>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>> write-enable bits for each, so the chipset can be configured to read
>> from the bios and write to memory (to setup BIOS-RAM caching), and read
>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>> other bit combinations are also available.
>
> Yup.  As Gleb mentions, there's the SDRAM register which controls 
> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
> KVM explicitly disabled SMM support).

KVM not supporting SMM is a bug (albeit one that is likely to remain 
unresolved for a while).  Let's pretend that kvm smm support is not an 
issue.

IIUC, SMM means that there two memory maps when the cpu accesses memory, 
one for SMM, one for non-SMM.

>
>> For my purpose in using this to program the IOMMU with guest physical to
>> host virtual addresses for device assignment, it doesn't really matter
>> since there should never be a DMA in this range of memory.  But for a
>> general RAM API, I'm not sure either.  I'm tempted to say that while
>> this is in fact a use of RAM, the RAM is never presented to the guest as
>> usable system memory (E820_RAM for x86), and should therefore be
>> excluded from the RAM API if we're using it only to track regions that
>> are actual guest usable physical memory.
>>
>> We had talked on irc that pc.c should be registering 0x0 to
>> below_4g_mem_size as ram, but now I tend to disagree with that.  The
>> memory backing 0xa0000-0x100000 is present, but it's not presented to
>> the guest as usable RAM.  What's your strict definition of what the RAM
>> API includes?  Is it only what the guest could consider usable RAM or
>> does it also include quirky chipset accelerator features like this
>> (everything with a guest physical address)?  Thanks,
>
> Today we model on flat space that's a mixed of device memory, RAM, or 
> ROM.  This is not how machines work and the limitations of this model 
> is holding us back.
>
> IRL, there's a block of RAM that's connected to a memory controller.  
> The CPU is also connected to the memory controller.  Devices are 
> connected to another controller which is in turn connected to the 
> memory controller.  There may, in fact, be more than one controller 
> between a device and the memory controller.
>
> A controller may change the way a device sees memory in arbitrary 
> ways.  In fact, two controllers accessing the same page might see 
> something totally different.
>
> The idea behind the RAM API is to begin to establish this hierarchy.  
> RAM is not what any particular device sees--it's actual RAM.  IOW, the 
> RAM API should represent what address mapping I would get if I talked 
> directly to DIMMs.
>
> This is not what RamBlock is even though the name would suggest 
> otherwise.  RamBlocks are anything that qemu represents as cache 
> consistency directly accessable memory.  Device ROMs and areas of 
> device RAM are all allocated from the RamBlock space.
>
> So the very first task of a RAM API is to simplify differentiate these 
> two things.  Once we have the base RAM API, we can start adding the 
> proper APIs that sit on top of it (like a PCI memory API).

Things aren't that bad - a ram_addr_t and a physical address are already 
different things, so we already have one level of translation.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 15:22             ` Avi Kivity
  0 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-11-18 15:22 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>> Gack.  For the benefit of those that want to join the fun without
>> digging up the spec, these magic flippable segments the i440fx can
>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>> write-enable bits for each, so the chipset can be configured to read
>> from the bios and write to memory (to setup BIOS-RAM caching), and read
>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>> other bit combinations are also available.
>
> Yup.  As Gleb mentions, there's the SDRAM register which controls 
> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
> KVM explicitly disabled SMM support).

KVM not supporting SMM is a bug (albeit one that is likely to remain 
unresolved for a while).  Let's pretend that kvm smm support is not an 
issue.

IIUC, SMM means that there two memory maps when the cpu accesses memory, 
one for SMM, one for non-SMM.

>
>> For my purpose in using this to program the IOMMU with guest physical to
>> host virtual addresses for device assignment, it doesn't really matter
>> since there should never be a DMA in this range of memory.  But for a
>> general RAM API, I'm not sure either.  I'm tempted to say that while
>> this is in fact a use of RAM, the RAM is never presented to the guest as
>> usable system memory (E820_RAM for x86), and should therefore be
>> excluded from the RAM API if we're using it only to track regions that
>> are actual guest usable physical memory.
>>
>> We had talked on irc that pc.c should be registering 0x0 to
>> below_4g_mem_size as ram, but now I tend to disagree with that.  The
>> memory backing 0xa0000-0x100000 is present, but it's not presented to
>> the guest as usable RAM.  What's your strict definition of what the RAM
>> API includes?  Is it only what the guest could consider usable RAM or
>> does it also include quirky chipset accelerator features like this
>> (everything with a guest physical address)?  Thanks,
>
> Today we model on flat space that's a mixed of device memory, RAM, or 
> ROM.  This is not how machines work and the limitations of this model 
> is holding us back.
>
> IRL, there's a block of RAM that's connected to a memory controller.  
> The CPU is also connected to the memory controller.  Devices are 
> connected to another controller which is in turn connected to the 
> memory controller.  There may, in fact, be more than one controller 
> between a device and the memory controller.
>
> A controller may change the way a device sees memory in arbitrary 
> ways.  In fact, two controllers accessing the same page might see 
> something totally different.
>
> The idea behind the RAM API is to begin to establish this hierarchy.  
> RAM is not what any particular device sees--it's actual RAM.  IOW, the 
> RAM API should represent what address mapping I would get if I talked 
> directly to DIMMs.
>
> This is not what RamBlock is even though the name would suggest 
> otherwise.  RamBlocks are anything that qemu represents as cache 
> consistency directly accessable memory.  Device ROMs and areas of 
> device RAM are all allocated from the RamBlock space.
>
> So the very first task of a RAM API is to simplify differentiate these 
> two things.  Once we have the base RAM API, we can start adding the 
> proper APIs that sit on top of it (like a PCI memory API).

Things aren't that bad - a ram_addr_t and a physical address are already 
different things, so we already have one level of translation.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-18 15:22             ` Avi Kivity
@ 2010-11-18 15:46               ` Anthony Liguori
  -1 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-18 15:46 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/18/2010 09:22 AM, Avi Kivity wrote:
> On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>>> Gack.  For the benefit of those that want to join the fun without
>>> digging up the spec, these magic flippable segments the i440fx can
>>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>>> write-enable bits for each, so the chipset can be configured to read
>>> from the bios and write to memory (to setup BIOS-RAM caching), and read
>>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>>> other bit combinations are also available.
>>
>> Yup.  As Gleb mentions, there's the SDRAM register which controls 
>> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
>> KVM explicitly disabled SMM support).
>
> KVM not supporting SMM is a bug (albeit one that is likely to remain 
> unresolved for a while).  Let's pretend that kvm smm support is not an 
> issue.
>
> IIUC, SMM means that there two memory maps when the cpu accesses 
> memory, one for SMM, one for non-SMM.

No.  That's not what it means.  With the i440fx, when the CPU accesses 
0xa0000, it gets forwarded to the PCI bus no different than an access to 
0xe0000.

If the CPU asserts the EXF4#/Ab7# signal, then the i440fx directs CPU 
accesses to 0xa0000 to RAM instead of the PCI bus.

Alternatively, if the SMRAM register is activated, then the i440fx will 
redirect 0xa0000 to RAM regardless of whether the CPU asserts that 
signal.  That means that even without KVM supporting SMM, this mode can 
happen.

In general, the memory controller can redirect IO accesses to RAM or to 
the PCI bus.  The PCI bus may redirect the access to the ISA bus.

>>> For my purpose in using this to program the IOMMU with guest 
>>> physical to
>>> host virtual addresses for device assignment, it doesn't really matter
>>> since there should never be a DMA in this range of memory.  But for a
>>> general RAM API, I'm not sure either.  I'm tempted to say that while
>>> this is in fact a use of RAM, the RAM is never presented to the 
>>> guest as
>>> usable system memory (E820_RAM for x86), and should therefore be
>>> excluded from the RAM API if we're using it only to track regions that
>>> are actual guest usable physical memory.
>>>
>>> We had talked on irc that pc.c should be registering 0x0 to
>>> below_4g_mem_size as ram, but now I tend to disagree with that.  The
>>> memory backing 0xa0000-0x100000 is present, but it's not presented to
>>> the guest as usable RAM.  What's your strict definition of what the RAM
>>> API includes?  Is it only what the guest could consider usable RAM or
>>> does it also include quirky chipset accelerator features like this
>>> (everything with a guest physical address)?  Thanks,
>>
>> Today we model on flat space that's a mixed of device memory, RAM, or 
>> ROM.  This is not how machines work and the limitations of this model 
>> is holding us back.
>>
>> IRL, there's a block of RAM that's connected to a memory controller.  
>> The CPU is also connected to the memory controller.  Devices are 
>> connected to another controller which is in turn connected to the 
>> memory controller.  There may, in fact, be more than one controller 
>> between a device and the memory controller.
>>
>> A controller may change the way a device sees memory in arbitrary 
>> ways.  In fact, two controllers accessing the same page might see 
>> something totally different.
>>
>> The idea behind the RAM API is to begin to establish this hierarchy.  
>> RAM is not what any particular device sees--it's actual RAM.  IOW, 
>> the RAM API should represent what address mapping I would get if I 
>> talked directly to DIMMs.
>>
>> This is not what RamBlock is even though the name would suggest 
>> otherwise.  RamBlocks are anything that qemu represents as cache 
>> consistency directly accessable memory.  Device ROMs and areas of 
>> device RAM are all allocated from the RamBlock space.
>>
>> So the very first task of a RAM API is to simplify differentiate 
>> these two things.  Once we have the base RAM API, we can start adding 
>> the proper APIs that sit on top of it (like a PCI memory API).
>
> Things aren't that bad - a ram_addr_t and a physical address are 
> already different things, so we already have one level of translation.

Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
internal implementation detail.

Regards,

Anthony Liguori



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 15:46               ` Anthony Liguori
  0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-18 15:46 UTC (permalink / raw)
  To: Avi Kivity
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On 11/18/2010 09:22 AM, Avi Kivity wrote:
> On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>>> Gack.  For the benefit of those that want to join the fun without
>>> digging up the spec, these magic flippable segments the i440fx can
>>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>>> write-enable bits for each, so the chipset can be configured to read
>>> from the bios and write to memory (to setup BIOS-RAM caching), and read
>>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>>> other bit combinations are also available.
>>
>> Yup.  As Gleb mentions, there's the SDRAM register which controls 
>> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
>> KVM explicitly disabled SMM support).
>
> KVM not supporting SMM is a bug (albeit one that is likely to remain 
> unresolved for a while).  Let's pretend that kvm smm support is not an 
> issue.
>
> IIUC, SMM means that there two memory maps when the cpu accesses 
> memory, one for SMM, one for non-SMM.

No.  That's not what it means.  With the i440fx, when the CPU accesses 
0xa0000, it gets forwarded to the PCI bus no different than an access to 
0xe0000.

If the CPU asserts the EXF4#/Ab7# signal, then the i440fx directs CPU 
accesses to 0xa0000 to RAM instead of the PCI bus.

Alternatively, if the SMRAM register is activated, then the i440fx will 
redirect 0xa0000 to RAM regardless of whether the CPU asserts that 
signal.  That means that even without KVM supporting SMM, this mode can 
happen.

In general, the memory controller can redirect IO accesses to RAM or to 
the PCI bus.  The PCI bus may redirect the access to the ISA bus.

>>> For my purpose in using this to program the IOMMU with guest 
>>> physical to
>>> host virtual addresses for device assignment, it doesn't really matter
>>> since there should never be a DMA in this range of memory.  But for a
>>> general RAM API, I'm not sure either.  I'm tempted to say that while
>>> this is in fact a use of RAM, the RAM is never presented to the 
>>> guest as
>>> usable system memory (E820_RAM for x86), and should therefore be
>>> excluded from the RAM API if we're using it only to track regions that
>>> are actual guest usable physical memory.
>>>
>>> We had talked on irc that pc.c should be registering 0x0 to
>>> below_4g_mem_size as ram, but now I tend to disagree with that.  The
>>> memory backing 0xa0000-0x100000 is present, but it's not presented to
>>> the guest as usable RAM.  What's your strict definition of what the RAM
>>> API includes?  Is it only what the guest could consider usable RAM or
>>> does it also include quirky chipset accelerator features like this
>>> (everything with a guest physical address)?  Thanks,
>>
>> Today we model on flat space that's a mixed of device memory, RAM, or 
>> ROM.  This is not how machines work and the limitations of this model 
>> is holding us back.
>>
>> IRL, there's a block of RAM that's connected to a memory controller.  
>> The CPU is also connected to the memory controller.  Devices are 
>> connected to another controller which is in turn connected to the 
>> memory controller.  There may, in fact, be more than one controller 
>> between a device and the memory controller.
>>
>> A controller may change the way a device sees memory in arbitrary 
>> ways.  In fact, two controllers accessing the same page might see 
>> something totally different.
>>
>> The idea behind the RAM API is to begin to establish this hierarchy.  
>> RAM is not what any particular device sees--it's actual RAM.  IOW, 
>> the RAM API should represent what address mapping I would get if I 
>> talked directly to DIMMs.
>>
>> This is not what RamBlock is even though the name would suggest 
>> otherwise.  RamBlocks are anything that qemu represents as cache 
>> consistency directly accessable memory.  Device ROMs and areas of 
>> device RAM are all allocated from the RamBlock space.
>>
>> So the very first task of a RAM API is to simplify differentiate 
>> these two things.  Once we have the base RAM API, we can start adding 
>> the proper APIs that sit on top of it (like a PCI memory API).
>
> Things aren't that bad - a ram_addr_t and a physical address are 
> already different things, so we already have one level of translation.

Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
internal implementation detail.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-17 23:42         ` Anthony Liguori
@ 2010-11-18 15:51             ` Gleb Natapov
  2010-11-18 15:51             ` Gleb Natapov
  1 sibling, 0 replies; 53+ messages in thread
From: Gleb Natapov @ 2010-11-18 15:51 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On Wed, Nov 17, 2010 at 05:42:28PM -0600, Anthony Liguori wrote:
> >For my purpose in using this to program the IOMMU with guest physical to
> >host virtual addresses for device assignment, it doesn't really matter
> >since there should never be a DMA in this range of memory.  But for a
> >general RAM API, I'm not sure either.  I'm tempted to say that while
> >this is in fact a use of RAM, the RAM is never presented to the guest as
> >usable system memory (E820_RAM for x86), and should therefore be
> >excluded from the RAM API if we're using it only to track regions that
> >are actual guest usable physical memory.
> >
> >We had talked on irc that pc.c should be registering 0x0 to
> >below_4g_mem_size as ram, but now I tend to disagree with that.  The
> >memory backing 0xa0000-0x100000 is present, but it's not presented to
> >the guest as usable RAM.  What's your strict definition of what the RAM
> >API includes?  Is it only what the guest could consider usable RAM or
> >does it also include quirky chipset accelerator features like this
> >(everything with a guest physical address)?  Thanks,
> 
> Today we model on flat space that's a mixed of device memory, RAM,
> or ROM.  This is not how machines work and the limitations of this
> model is holding us back.
> 
> IRL, there's a block of RAM that's connected to a memory controller.
> The CPU is also connected to the memory controller.  Devices are
> connected to another controller which is in turn connected to the
> memory controller.  There may, in fact, be more than one controller
> between a device and the memory controller.
> 
> A controller may change the way a device sees memory in arbitrary
> ways.  In fact, two controllers accessing the same page might see
> something totally different.
> 
> The idea behind the RAM API is to begin to establish this hierarchy.
> RAM is not what any particular device sees--it's actual RAM.  IOW,
> the RAM API should represent what address mapping I would get if I
> talked directly to DIMMs.
> 
> This is not what RamBlock is even though the name would suggest
> otherwise.  RamBlocks are anything that qemu represents as cache
> consistency directly accessable memory.  Device ROMs and areas of
> device RAM are all allocated from the RamBlock space.
> 
> So the very first task of a RAM API is to simplify differentiate
> these two things.  Once we have the base RAM API, we can start
> adding the proper APIs that sit on top of it (like a PCI memory
> API).
> 
+1 for all above. What happens when device access some address is
completely different from what happens when CPU access the same address
(or even another device on another bus). For instance how MSI is
implemented now CPU can send MSI by writing to 0xfee00000 memory range.
I do not think you can do that on real HW.

--
			Gleb.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 15:51             ` Gleb Natapov
  0 siblings, 0 replies; 53+ messages in thread
From: Gleb Natapov @ 2010-11-18 15:51 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On Wed, Nov 17, 2010 at 05:42:28PM -0600, Anthony Liguori wrote:
> >For my purpose in using this to program the IOMMU with guest physical to
> >host virtual addresses for device assignment, it doesn't really matter
> >since there should never be a DMA in this range of memory.  But for a
> >general RAM API, I'm not sure either.  I'm tempted to say that while
> >this is in fact a use of RAM, the RAM is never presented to the guest as
> >usable system memory (E820_RAM for x86), and should therefore be
> >excluded from the RAM API if we're using it only to track regions that
> >are actual guest usable physical memory.
> >
> >We had talked on irc that pc.c should be registering 0x0 to
> >below_4g_mem_size as ram, but now I tend to disagree with that.  The
> >memory backing 0xa0000-0x100000 is present, but it's not presented to
> >the guest as usable RAM.  What's your strict definition of what the RAM
> >API includes?  Is it only what the guest could consider usable RAM or
> >does it also include quirky chipset accelerator features like this
> >(everything with a guest physical address)?  Thanks,
> 
> Today we model on flat space that's a mixed of device memory, RAM,
> or ROM.  This is not how machines work and the limitations of this
> model is holding us back.
> 
> IRL, there's a block of RAM that's connected to a memory controller.
> The CPU is also connected to the memory controller.  Devices are
> connected to another controller which is in turn connected to the
> memory controller.  There may, in fact, be more than one controller
> between a device and the memory controller.
> 
> A controller may change the way a device sees memory in arbitrary
> ways.  In fact, two controllers accessing the same page might see
> something totally different.
> 
> The idea behind the RAM API is to begin to establish this hierarchy.
> RAM is not what any particular device sees--it's actual RAM.  IOW,
> the RAM API should represent what address mapping I would get if I
> talked directly to DIMMs.
> 
> This is not what RamBlock is even though the name would suggest
> otherwise.  RamBlocks are anything that qemu represents as cache
> consistency directly accessable memory.  Device ROMs and areas of
> device RAM are all allocated from the RamBlock space.
> 
> So the very first task of a RAM API is to simplify differentiate
> these two things.  Once we have the base RAM API, we can start
> adding the proper APIs that sit on top of it (like a PCI memory
> API).
> 
+1 for all above. What happens when device access some address is
completely different from what happens when CPU access the same address
(or even another device on another bus). For instance how MSI is
implemented now CPU can send MSI by writing to 0xfee00000 memory range.
I do not think you can do that on real HW.

--
			Gleb.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-18 15:46               ` Anthony Liguori
@ 2010-11-18 15:57                 ` Avi Kivity
  -1 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-11-18 15:57 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/18/2010 05:46 PM, Anthony Liguori wrote:
> On 11/18/2010 09:22 AM, Avi Kivity wrote:
>> On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>>>> Gack.  For the benefit of those that want to join the fun without
>>>> digging up the spec, these magic flippable segments the i440fx can
>>>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>>>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>>>> write-enable bits for each, so the chipset can be configured to read
>>>> from the bios and write to memory (to setup BIOS-RAM caching), and 
>>>> read
>>>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>>>> other bit combinations are also available.
>>>
>>> Yup.  As Gleb mentions, there's the SDRAM register which controls 
>>> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
>>> KVM explicitly disabled SMM support).
>>
>> KVM not supporting SMM is a bug (albeit one that is likely to remain 
>> unresolved for a while).  Let's pretend that kvm smm support is not 
>> an issue.
>>
>> IIUC, SMM means that there two memory maps when the cpu accesses 
>> memory, one for SMM, one for non-SMM.
>
> No.  That's not what it means.  With the i440fx, when the CPU accesses 
> 0xa0000, it gets forwarded to the PCI bus no different than an access 
> to 0xe0000.
>
> If the CPU asserts the EXF4#/Ab7# signal, then the i440fx directs CPU 
> accesses to 0xa0000 to RAM instead of the PCI bus.

That's what "two memory maps" mean.  If you have one cpu in SMM and 
another outside SMM, then those two maps are active simultaneously.

>
> Alternatively, if the SMRAM register is activated, then the i440fx 
> will redirect 0xa0000 to RAM regardless of whether the CPU asserts 
> that signal.  That means that even without KVM supporting SMM, this 
> mode can happen.

That's a single memory map that is modified under hardware control, it's 
no different than BARs and such.

>> Things aren't that bad - a ram_addr_t and a physical address are 
>> already different things, so we already have one level of translation.
>
> Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
> internal implementation detail.
>

Does it matter?  We can say those are addresses on the memory bus.  
Since they are not observable anyway, who cares if the correspond with 
reality or not?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 15:57                 ` Avi Kivity
  0 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-11-18 15:57 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On 11/18/2010 05:46 PM, Anthony Liguori wrote:
> On 11/18/2010 09:22 AM, Avi Kivity wrote:
>> On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>>>> Gack.  For the benefit of those that want to join the fun without
>>>> digging up the spec, these magic flippable segments the i440fx can
>>>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>>>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>>>> write-enable bits for each, so the chipset can be configured to read
>>>> from the bios and write to memory (to setup BIOS-RAM caching), and 
>>>> read
>>>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>>>> other bit combinations are also available.
>>>
>>> Yup.  As Gleb mentions, there's the SDRAM register which controls 
>>> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
>>> KVM explicitly disabled SMM support).
>>
>> KVM not supporting SMM is a bug (albeit one that is likely to remain 
>> unresolved for a while).  Let's pretend that kvm smm support is not 
>> an issue.
>>
>> IIUC, SMM means that there two memory maps when the cpu accesses 
>> memory, one for SMM, one for non-SMM.
>
> No.  That's not what it means.  With the i440fx, when the CPU accesses 
> 0xa0000, it gets forwarded to the PCI bus no different than an access 
> to 0xe0000.
>
> If the CPU asserts the EXF4#/Ab7# signal, then the i440fx directs CPU 
> accesses to 0xa0000 to RAM instead of the PCI bus.

That's what "two memory maps" mean.  If you have one cpu in SMM and 
another outside SMM, then those two maps are active simultaneously.

>
> Alternatively, if the SMRAM register is activated, then the i440fx 
> will redirect 0xa0000 to RAM regardless of whether the CPU asserts 
> that signal.  That means that even without KVM supporting SMM, this 
> mode can happen.

That's a single memory map that is modified under hardware control, it's 
no different than BARs and such.

>> Things aren't that bad - a ram_addr_t and a physical address are 
>> already different things, so we already have one level of translation.
>
> Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
> internal implementation detail.
>

Does it matter?  We can say those are addresses on the memory bus.  
Since they are not observable anyway, who cares if the correspond with 
reality or not?

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-18 15:57                 ` Avi Kivity
@ 2010-11-18 16:09                   ` Anthony Liguori
  -1 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-18 16:09 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/18/2010 09:57 AM, Avi Kivity wrote:
> On 11/18/2010 05:46 PM, Anthony Liguori wrote:
>> On 11/18/2010 09:22 AM, Avi Kivity wrote:
>>> On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>>>>> Gack.  For the benefit of those that want to join the fun without
>>>>> digging up the spec, these magic flippable segments the i440fx can
>>>>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>>>>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>>>>> write-enable bits for each, so the chipset can be configured to read
>>>>> from the bios and write to memory (to setup BIOS-RAM caching), and 
>>>>> read
>>>>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>>>>> other bit combinations are also available.
>>>>
>>>> Yup.  As Gleb mentions, there's the SDRAM register which controls 
>>>> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
>>>> KVM explicitly disabled SMM support).
>>>
>>> KVM not supporting SMM is a bug (albeit one that is likely to remain 
>>> unresolved for a while).  Let's pretend that kvm smm support is not 
>>> an issue.
>>>
>>> IIUC, SMM means that there two memory maps when the cpu accesses 
>>> memory, one for SMM, one for non-SMM.
>>
>> No.  That's not what it means.  With the i440fx, when the CPU 
>> accesses 0xa0000, it gets forwarded to the PCI bus no different than 
>> an access to 0xe0000.
>>
>> If the CPU asserts the EXF4#/Ab7# signal, then the i440fx directs CPU 
>> accesses to 0xa0000 to RAM instead of the PCI bus.
>
> That's what "two memory maps" mean.  If you have one cpu in SMM and 
> another outside SMM, then those two maps are active simultaneously.

I'm not sure if more modern memory controllers do special things here, 
but for the i440fx, if any CPU asserts SMM mode, then any memory access 
to that space is going to access SMRAM.

>>
>> Alternatively, if the SMRAM register is activated, then the i440fx 
>> will redirect 0xa0000 to RAM regardless of whether the CPU asserts 
>> that signal.  That means that even without KVM supporting SMM, this 
>> mode can happen.
>
> That's a single memory map that is modified under hardware control, 
> it's no different than BARs and such.

There is a single block of RAM.

The memory controller may either forward an address unmodified to the 
RAM block or it may forward the address to the PCI bus[1].  A non CPU 
access goes through a controller hierarchy and may be modified while it 
transverses the hierarchy.

So really, we should have a big chunk of RAM that we associate with a 
guest, with a list of intercepts that changes as the devices are 
modified.  Instead of having that list dispatch directly to a device, we 
should send all intercepted accesses to the memory controller and let 
the memory controller propagate out the access to the appropriate device.

[1] The except is access to the local APIC.  That's handled directly by 
the CPU (or immediately outside of the CPU before the access gets to the 
memory controller if the local APIC is external to the CPU).

>>> Things aren't that bad - a ram_addr_t and a physical address are 
>>> already different things, so we already have one level of translation.
>>
>> Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
>> internal implementation detail.
>>
>
> Does it matter?  We can say those are addresses on the memory bus.  
> Since they are not observable anyway, who cares if the correspond with 
> reality or not?

It matters a lot because the life cycle of RAM is different from the 
life cycle of ROM.

For instance, the original goal was to madvise(MADV_DONTNEED) RAM on 
reboot.  You can't do that to ROM because the contents matter.

But for PV devices, we can be loose in how we define the way the devices 
interact with the rest of the system.  For instance, we can say that 
virtio-pci devices are directly connected to RAM and do not go through 
the memory controllers.  That means we could get stable mappings of the 
virtio ring.

Regards,

Anthony Liguori



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 16:09                   ` Anthony Liguori
  0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-11-18 16:09 UTC (permalink / raw)
  To: Avi Kivity
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On 11/18/2010 09:57 AM, Avi Kivity wrote:
> On 11/18/2010 05:46 PM, Anthony Liguori wrote:
>> On 11/18/2010 09:22 AM, Avi Kivity wrote:
>>> On 11/18/2010 01:42 AM, Anthony Liguori wrote:
>>>>> Gack.  For the benefit of those that want to join the fun without
>>>>> digging up the spec, these magic flippable segments the i440fx can
>>>>> toggle are 12 fixed 16k segments from 0xc0000 to 0xeffff and a single
>>>>> 64k segment from 0xf0000 to 0xfffff.  There are read-enable and
>>>>> write-enable bits for each, so the chipset can be configured to read
>>>>> from the bios and write to memory (to setup BIOS-RAM caching), and 
>>>>> read
>>>>> from memory and write to the bios (to enable BIOS-RAM caching).  The
>>>>> other bit combinations are also available.
>>>>
>>>> Yup.  As Gleb mentions, there's the SDRAM register which controls 
>>>> whether 0xa0000 is mapped to PCI or whether it's mapped to RAM (but 
>>>> KVM explicitly disabled SMM support).
>>>
>>> KVM not supporting SMM is a bug (albeit one that is likely to remain 
>>> unresolved for a while).  Let's pretend that kvm smm support is not 
>>> an issue.
>>>
>>> IIUC, SMM means that there two memory maps when the cpu accesses 
>>> memory, one for SMM, one for non-SMM.
>>
>> No.  That's not what it means.  With the i440fx, when the CPU 
>> accesses 0xa0000, it gets forwarded to the PCI bus no different than 
>> an access to 0xe0000.
>>
>> If the CPU asserts the EXF4#/Ab7# signal, then the i440fx directs CPU 
>> accesses to 0xa0000 to RAM instead of the PCI bus.
>
> That's what "two memory maps" mean.  If you have one cpu in SMM and 
> another outside SMM, then those two maps are active simultaneously.

I'm not sure if more modern memory controllers do special things here, 
but for the i440fx, if any CPU asserts SMM mode, then any memory access 
to that space is going to access SMRAM.

>>
>> Alternatively, if the SMRAM register is activated, then the i440fx 
>> will redirect 0xa0000 to RAM regardless of whether the CPU asserts 
>> that signal.  That means that even without KVM supporting SMM, this 
>> mode can happen.
>
> That's a single memory map that is modified under hardware control, 
> it's no different than BARs and such.

There is a single block of RAM.

The memory controller may either forward an address unmodified to the 
RAM block or it may forward the address to the PCI bus[1].  A non CPU 
access goes through a controller hierarchy and may be modified while it 
transverses the hierarchy.

So really, we should have a big chunk of RAM that we associate with a 
guest, with a list of intercepts that changes as the devices are 
modified.  Instead of having that list dispatch directly to a device, we 
should send all intercepted accesses to the memory controller and let 
the memory controller propagate out the access to the appropriate device.

[1] The except is access to the local APIC.  That's handled directly by 
the CPU (or immediately outside of the CPU before the access gets to the 
memory controller if the local APIC is external to the CPU).

>>> Things aren't that bad - a ram_addr_t and a physical address are 
>>> already different things, so we already have one level of translation.
>>
>> Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
>> internal implementation detail.
>>
>
> Does it matter?  We can say those are addresses on the memory bus.  
> Since they are not observable anyway, who cares if the correspond with 
> reality or not?

It matters a lot because the life cycle of RAM is different from the 
life cycle of ROM.

For instance, the original goal was to madvise(MADV_DONTNEED) RAM on 
reboot.  You can't do that to ROM because the contents matter.

But for PV devices, we can be loose in how we define the way the devices 
interact with the rest of the system.  For instance, we can say that 
virtio-pci devices are directly connected to RAM and do not go through 
the memory controllers.  That means we could get stable mappings of the 
virtio ring.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-18 16:09                   ` Anthony Liguori
@ 2010-11-18 16:18                     ` Avi Kivity
  -1 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-11-18 16:18 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Alex Williamson, chrisw, kvm, mst, qemu-devel, blauwirbel, ddutile

On 11/18/2010 06:09 PM, Anthony Liguori wrote:
>> That's what "two memory maps" mean.  If you have one cpu in SMM and 
>> another outside SMM, then those two maps are active simultaneously.
>
>
> I'm not sure if more modern memory controllers do special things here, 
> but for the i440fx, if any CPU asserts SMM mode, then any memory 
> access to that space is going to access SMRAM.

How does SMP work then?

> SMM Space Open (DOPEN). When DOPEN=1 and DLCK=0, SMM space DRAM is 
> made visible even
> when CPU cycle does not indicate SMM mode access via EXF4#/Ab7# 
> signal. This is intended to help
> BIOS initialize SMM space. Software should ensure that DOPEN=1 is 
> mutually exclusive with DCLS=1.
> When DLCK is set to a 1, DOPEN is set to 0 and becomes read only.

The words "cpu cycle does not indicate SMM mode" seem to say that SMM 
accesses are made on a per-transaction basis, or so my lawyers tell me.


>
>>>
>>> Alternatively, if the SMRAM register is activated, then the i440fx 
>>> will redirect 0xa0000 to RAM regardless of whether the CPU asserts 
>>> that signal.  That means that even without KVM supporting SMM, this 
>>> mode can happen.
>>
>> That's a single memory map that is modified under hardware control, 
>> it's no different than BARs and such.
>
> There is a single block of RAM.
>
> The memory controller may either forward an address unmodified to the 
> RAM block or it may forward the address to the PCI bus[1].  A non CPU 
> access goes through a controller hierarchy and may be modified while 
> it transverses the hierarchy.
>
> So really, we should have a big chunk of RAM that we associate with a 
> guest, with a list of intercepts that changes as the devices are 
> modified.  Instead of having that list dispatch directly to a device, 
> we should send all intercepted accesses to the memory controller and 
> let the memory controller propagate out the access to the appropriate 
> device.
>
> [1] The except is access to the local APIC.  That's handled directly 
> by the CPU (or immediately outside of the CPU before the access gets 
> to the memory controller if the local APIC is external to the CPU).
>

Agree.  However the point with SMM is that the dispatch is made not only 
based on the address, but also based on SMM mode (and, unfortunately, 
can also be different based on read vs write).

>>>> Things aren't that bad - a ram_addr_t and a physical address are 
>>>> already different things, so we already have one level of translation.
>>>
>>> Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
>>> internal implementation detail.
>>>
>>
>> Does it matter?  We can say those are addresses on the memory bus.  
>> Since they are not observable anyway, who cares if the correspond 
>> with reality or not?
>
> It matters a lot because the life cycle of RAM is different from the 
> life cycle of ROM.
>
> For instance, the original goal was to madvise(MADV_DONTNEED) RAM on 
> reboot.  You can't do that to ROM because the contents matter.

I don't think you can do that to RAM either.

>
> But for PV devices, we can be loose in how we define the way the 
> devices interact with the rest of the system.  For instance, we can 
> say that virtio-pci devices are directly connected to RAM and do not 
> go through the memory controllers.  That means we could get stable 
> mappings of the virtio ring.

That wouldn't work once we have an iommu and start to assign them to 
nested guests.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 16:18                     ` Avi Kivity
  0 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-11-18 16:18 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: chrisw, kvm, mst, qemu-devel, blauwirbel, Alex Williamson, ddutile

On 11/18/2010 06:09 PM, Anthony Liguori wrote:
>> That's what "two memory maps" mean.  If you have one cpu in SMM and 
>> another outside SMM, then those two maps are active simultaneously.
>
>
> I'm not sure if more modern memory controllers do special things here, 
> but for the i440fx, if any CPU asserts SMM mode, then any memory 
> access to that space is going to access SMRAM.

How does SMP work then?

> SMM Space Open (DOPEN). When DOPEN=1 and DLCK=0, SMM space DRAM is 
> made visible even
> when CPU cycle does not indicate SMM mode access via EXF4#/Ab7# 
> signal. This is intended to help
> BIOS initialize SMM space. Software should ensure that DOPEN=1 is 
> mutually exclusive with DCLS=1.
> When DLCK is set to a 1, DOPEN is set to 0 and becomes read only.

The words "cpu cycle does not indicate SMM mode" seem to say that SMM 
accesses are made on a per-transaction basis, or so my lawyers tell me.


>
>>>
>>> Alternatively, if the SMRAM register is activated, then the i440fx 
>>> will redirect 0xa0000 to RAM regardless of whether the CPU asserts 
>>> that signal.  That means that even without KVM supporting SMM, this 
>>> mode can happen.
>>
>> That's a single memory map that is modified under hardware control, 
>> it's no different than BARs and such.
>
> There is a single block of RAM.
>
> The memory controller may either forward an address unmodified to the 
> RAM block or it may forward the address to the PCI bus[1].  A non CPU 
> access goes through a controller hierarchy and may be modified while 
> it transverses the hierarchy.
>
> So really, we should have a big chunk of RAM that we associate with a 
> guest, with a list of intercepts that changes as the devices are 
> modified.  Instead of having that list dispatch directly to a device, 
> we should send all intercepted accesses to the memory controller and 
> let the memory controller propagate out the access to the appropriate 
> device.
>
> [1] The except is access to the local APIC.  That's handled directly 
> by the CPU (or immediately outside of the CPU before the access gets 
> to the memory controller if the local APIC is external to the CPU).
>

Agree.  However the point with SMM is that the dispatch is made not only 
based on the address, but also based on SMM mode (and, unfortunately, 
can also be different based on read vs write).

>>>> Things aren't that bad - a ram_addr_t and a physical address are 
>>>> already different things, so we already have one level of translation.
>>>
>>> Yeah, but ram_addr_t doesn't model anything meaningful IRL.  It's an 
>>> internal implementation detail.
>>>
>>
>> Does it matter?  We can say those are addresses on the memory bus.  
>> Since they are not observable anyway, who cares if the correspond 
>> with reality or not?
>
> It matters a lot because the life cycle of RAM is different from the 
> life cycle of ROM.
>
> For instance, the original goal was to madvise(MADV_DONTNEED) RAM on 
> reboot.  You can't do that to ROM because the contents matter.

I don't think you can do that to RAM either.

>
> But for PV devices, we can be loose in how we define the way the 
> devices interact with the rest of the system.  For instance, we can 
> say that virtio-pci devices are directly connected to RAM and do not 
> go through the memory controllers.  That means we could get stable 
> mappings of the virtio ring.

That wouldn't work once we have an iommu and start to assign them to 
nested guests.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
  2010-11-18 16:18                     ` Avi Kivity
@ 2010-11-18 16:35                       ` Michael S. Tsirkin
  -1 siblings, 0 replies; 53+ messages in thread
From: Michael S. Tsirkin @ 2010-11-18 16:35 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Anthony Liguori, Alex Williamson, chrisw, kvm, qemu-devel,
	blauwirbel, ddutile

On Thu, Nov 18, 2010 at 06:18:06PM +0200, Avi Kivity wrote:
> >But for PV devices, we can be loose in how we define the way the
> >devices interact with the rest of the system.  For instance, we
> >can say that virtio-pci devices are directly connected to RAM and
> >do not go through the memory controllers.  That means we could get
> >stable mappings of the virtio ring.
> 
> That wouldn't work once we have an iommu and start to assign them to
> nested guests.

Yea. Not sure whether I'm worried about that though.
Mixing in all the problems inherent in nested virt, PV and assigned
devices seems especially masochistic.

> -- 
> error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 16:35                       ` Michael S. Tsirkin
  0 siblings, 0 replies; 53+ messages in thread
From: Michael S. Tsirkin @ 2010-11-18 16:35 UTC (permalink / raw)
  To: Avi Kivity; +Cc: chrisw, kvm, qemu-devel, blauwirbel, Alex Williamson, ddutile

On Thu, Nov 18, 2010 at 06:18:06PM +0200, Avi Kivity wrote:
> >But for PV devices, we can be loose in how we define the way the
> >devices interact with the rest of the system.  For instance, we
> >can say that virtio-pci devices are directly connected to RAM and
> >do not go through the memory controllers.  That means we could get
> >stable mappings of the virtio ring.
> 
> That wouldn't work once we have an iommu and start to assign them to
> nested guests.

Yea. Not sure whether I'm worried about that though.
Mixing in all the problems inherent in nested virt, PV and assigned
devices seems especially masochistic.

> -- 
> error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v3 0/2] Minimal RAM API support
  2010-11-01 15:13   ` [Qemu-devel] " Alex Williamson
@ 2010-11-18 21:41     ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-18 21:41 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: kvm, alex.williamson

v3:

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

Let me know if there are any further issues.  Thanks,

Alex

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.  Thanks,

Alex

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.
Thanks,

Alex

---

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

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH v3 0/2] Minimal RAM API support
@ 2010-11-18 21:41     ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-18 21:41 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: alex.williamson, kvm

v3:

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

Let me know if there are any further issues.  Thanks,

Alex

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.  Thanks,

Alex

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.
Thanks,

Alex

---

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

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v3 1/2] Minimal RAM API support
  2010-11-18 21:41     ` [Qemu-devel] " Alex Williamson
@ 2010-11-18 21:41       ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-18 21:41 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: kvm, 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 f07fb01..33fae0b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -154,6 +154,7 @@ hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.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 a543b5d..6aa2738 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -23,6 +23,8 @@
 /* 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 */


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH v3 1/2] Minimal RAM API support
@ 2010-11-18 21:41       ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-18 21:41 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: alex.williamson, kvm

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 f07fb01..33fae0b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -154,6 +154,7 @@ hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.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 a543b5d..6aa2738 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -23,6 +23,8 @@
 /* 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 */

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 2/2] RAM API: Make use of it for x86 PC
  2010-11-18 21:41     ` [Qemu-devel] " Alex Williamson
@ 2010-11-18 21:41       ` Alex Williamson
  -1 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-18 21:41 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: kvm, 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 69b13bf..fb7ee21 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -912,14 +912,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
 


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [Qemu-devel] [PATCH v3 2/2] RAM API: Make use of it for x86 PC
@ 2010-11-18 21:41       ` Alex Williamson
  0 siblings, 0 replies; 53+ messages in thread
From: Alex Williamson @ 2010-11-18 21:41 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: alex.williamson, kvm

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 69b13bf..fb7ee21 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -912,14 +912,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
 

^ permalink raw reply related	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2010-11-18 21:42 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-29 16:38 [PATCH 0/2] Minimal RAM API support Alex Williamson
2010-10-29 16:38 ` [Qemu-devel] " Alex Williamson
2010-10-29 16:39 ` [PATCH 1/2] " Alex Williamson
2010-10-29 16:39   ` [Qemu-devel] " Alex Williamson
2010-10-29 19:57   ` Blue Swirl
2010-10-29 19:57     ` Blue Swirl
2010-10-29 20:15     ` Alex Williamson
2010-10-29 20:15       ` Alex Williamson
2010-11-01  2:17   ` Isaku Yamahata
2010-11-01  2:17     ` Isaku Yamahata
2010-11-01  2:32     ` Alex Williamson
2010-11-01  2:32       ` Alex Williamson
2010-10-29 16:39 ` [PATCH 2/2] RAM API: Make use of it for x86 PC Alex Williamson
2010-10-29 16:39   ` [Qemu-devel] " Alex Williamson
2010-11-01 15:13 ` [PATCH v2 0/2] Minimal RAM API support Alex Williamson
2010-11-01 15:13   ` [Qemu-devel] " Alex Williamson
2010-11-01 15:14   ` [PATCH v2 1/2] " Alex Williamson
2010-11-01 15:14     ` [Qemu-devel] " Alex Williamson
2010-11-16 14:55     ` Anthony Liguori
2010-11-16 14:55       ` Anthony Liguori
2010-11-16 15:02       ` Alexander Graf
2010-11-16 15:02         ` Alexander Graf
2010-11-16 15:08         ` Anthony Liguori
2010-11-16 15:08           ` Anthony Liguori
2010-11-01 15:14   ` [PATCH v2 2/2] RAM API: Make use of it for x86 PC Alex Williamson
2010-11-01 15:14     ` [Qemu-devel] " Alex Williamson
2010-11-16 14:58     ` Anthony Liguori
2010-11-16 14:58       ` Anthony Liguori
2010-11-16 21:24       ` Alex Williamson
2010-11-16 21:24         ` Alex Williamson
2010-11-17  9:31         ` Gleb Natapov
2010-11-17  9:31           ` Gleb Natapov
2010-11-17 23:42         ` Anthony Liguori
2010-11-18 15:22           ` Avi Kivity
2010-11-18 15:22             ` Avi Kivity
2010-11-18 15:46             ` Anthony Liguori
2010-11-18 15:46               ` Anthony Liguori
2010-11-18 15:57               ` Avi Kivity
2010-11-18 15:57                 ` Avi Kivity
2010-11-18 16:09                 ` Anthony Liguori
2010-11-18 16:09                   ` Anthony Liguori
2010-11-18 16:18                   ` Avi Kivity
2010-11-18 16:18                     ` Avi Kivity
2010-11-18 16:35                     ` Michael S. Tsirkin
2010-11-18 16:35                       ` Michael S. Tsirkin
2010-11-18 15:51           ` Gleb Natapov
2010-11-18 15:51             ` Gleb Natapov
2010-11-18 21:41   ` [PATCH v3 0/2] Minimal RAM API support Alex Williamson
2010-11-18 21:41     ` [Qemu-devel] " Alex Williamson
2010-11-18 21:41     ` [PATCH v3 1/2] " Alex Williamson
2010-11-18 21:41       ` [Qemu-devel] " Alex Williamson
2010-11-18 21:41     ` [PATCH v3 2/2] RAM API: Make use of it for x86 PC Alex Williamson
2010-11-18 21:41       ` [Qemu-devel] " Alex Williamson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.