[PATCH v4 0/2] Minimal RAM API support

[PATCH v4 0/2] Minimal RAM API support

[Alex Williamson]

Update per comments, Thanks,

Alex

v4:

 - ram_slot -> RamSlot (per CODING_STYLE)
 - drop qemu_ prefix from functions (per CODING_STYLE)
 - mallocz -> malloc
 - drop extraneous return from void function

v3:

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

Let me know if there are any further issues.

v2:

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

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

v1:

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

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

---

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


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

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

[Alex Williamson]

Update per comments, Thanks,

Alex

v4:

 - ram_slot -> RamSlot (per CODING_STYLE)
 - drop qemu_ prefix from functions (per CODING_STYLE)
 - mallocz -> malloc
 - drop extraneous return from void function

v3:

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

Let me know if there are any further issues.

v2:

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

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

v1:

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

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

---

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


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

[PATCH v4 1/2] Minimal RAM API support

[Alex Williamson]

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

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

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

diff --git a/Makefile.objs b/Makefile.objs
index cebb945..47f3c3a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -172,6 +172,7 @@ hw-obj-y += pci.o pci_bridge.o msix.o msi.o
 hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
 hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
 hw-obj-y += watchdog.o
+hw-obj-y += memory.o
 hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 hw-obj-$(CONFIG_ECC) += ecc.o
 hw-obj-$(CONFIG_NAND) += nand.o
diff --git a/cpu-common.h b/cpu-common.h
index 6d4a898..f08f93b 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -29,6 +29,8 @@ enum device_endian {
 /* address in the RAM (different from a physical address) */
 typedef unsigned long ram_addr_t;
 
+#include "memory.h"
+
 /* memory API */
 
 typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
diff --git a/memory.c b/memory.c
new file mode 100644
index 0000000..07cb020
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,94 @@
+/*
+ * 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 RamSlot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    QLIST_ENTRY(RamSlot) next;
+} RamSlot;
+
+static QLIST_HEAD(ram_slot_list, RamSlot) ram_slot_list =
+    QLIST_HEAD_INITIALIZER(ram_slot_list);
+
+static RamSlot *ram_find_slot(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    RamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slot_list, 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 ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                 ram_addr_t phys_offset)
+{
+    RamSlot *slot;
+
+    if (!size) {
+        return -EINVAL;
+    }
+
+    assert(!ram_find_slot(start_addr, size));
+
+    slot = qemu_malloc(sizeof(RamSlot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slot_list, slot, next);
+
+    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
+
+    return 0;
+}
+
+void ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    RamSlot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    slot = 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);
+}
+
+int ram_for_each_slot(void *opaque, ram_for_each_slot_fn fn)
+{
+    RamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slot_list, 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..98c85ea
--- /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 (*ram_for_each_slot_fn)(void *opaque,
+                                    target_phys_addr_t start_addr,
+                                    ram_addr_t size,
+                                    ram_addr_t phys_offset);
+
+/**
+ * ram_register() : Register a region of guest physical memory
+ *
+ * The new region must not overlap an existing region.
+ */
+int ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                 ram_addr_t phys_offset);
+
+/**
+ * ram_unregister() : Unregister a region of guest physical memory
+ */
+void ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+
+/**
+ * ram_for_each_slot() : Call fn() on each registered region
+ *
+ * Stop on non-zero return from fn().
+ */
+int ram_for_each_slot(void *opaque, ram_for_each_slot_fn fn);
+
+#endif /* QEMU_MEMORY_H */

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

[Alex Williamson]

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

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

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

diff --git a/Makefile.objs b/Makefile.objs
index cebb945..47f3c3a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -172,6 +172,7 @@ hw-obj-y += pci.o pci_bridge.o msix.o msi.o
 hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
 hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
 hw-obj-y += watchdog.o
+hw-obj-y += memory.o
 hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 hw-obj-$(CONFIG_ECC) += ecc.o
 hw-obj-$(CONFIG_NAND) += nand.o
diff --git a/cpu-common.h b/cpu-common.h
index 6d4a898..f08f93b 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -29,6 +29,8 @@ enum device_endian {
 /* address in the RAM (different from a physical address) */
 typedef unsigned long ram_addr_t;
 
+#include "memory.h"
+
 /* memory API */
 
 typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
diff --git a/memory.c b/memory.c
new file mode 100644
index 0000000..07cb020
--- /dev/null
+++ b/memory.c
@@ -0,0 +1,94 @@
+/*
+ * 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 RamSlot {
+    target_phys_addr_t start_addr;
+    ram_addr_t size;
+    ram_addr_t offset;
+    QLIST_ENTRY(RamSlot) next;
+} RamSlot;
+
+static QLIST_HEAD(ram_slot_list, RamSlot) ram_slot_list =
+    QLIST_HEAD_INITIALIZER(ram_slot_list);
+
+static RamSlot *ram_find_slot(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    RamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slot_list, 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 ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                 ram_addr_t phys_offset)
+{
+    RamSlot *slot;
+
+    if (!size) {
+        return -EINVAL;
+    }
+
+    assert(!ram_find_slot(start_addr, size));
+
+    slot = qemu_malloc(sizeof(RamSlot));
+
+    slot->start_addr = start_addr;
+    slot->size = size;
+    slot->offset = phys_offset;
+
+    QLIST_INSERT_HEAD(&ram_slot_list, slot, next);
+
+    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
+
+    return 0;
+}
+
+void ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
+{
+    RamSlot *slot;
+
+    if (!size) {
+        return;
+    }
+
+    slot = 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);
+}
+
+int ram_for_each_slot(void *opaque, ram_for_each_slot_fn fn)
+{
+    RamSlot *slot;
+
+    QLIST_FOREACH(slot, &ram_slot_list, 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..98c85ea
--- /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 (*ram_for_each_slot_fn)(void *opaque,
+                                    target_phys_addr_t start_addr,
+                                    ram_addr_t size,
+                                    ram_addr_t phys_offset);
+
+/**
+ * ram_register() : Register a region of guest physical memory
+ *
+ * The new region must not overlap an existing region.
+ */
+int ram_register(target_phys_addr_t start_addr, ram_addr_t size,
+                 ram_addr_t phys_offset);
+
+/**
+ * ram_unregister() : Unregister a region of guest physical memory
+ */
+void ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
+
+/**
+ * ram_for_each_slot() : Call fn() on each registered region
+ *
+ * Stop on non-zero return from fn().
+ */
+int ram_for_each_slot(void *opaque, ram_for_each_slot_fn fn);
+
+#endif /* QEMU_MEMORY_H */

[PATCH v4 2/2] RAM API: Make use of it for x86 PC

[Alex Williamson]

Register the actual VM RAM using the new API

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

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

diff --git a/hw/pc.c b/hw/pc.c
index e1b2667..87adca2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
     /* allocate RAM */
     ram_addr = qemu_ram_alloc(NULL, "pc.ram",
                               below_4g_mem_size + above_4g_mem_size);
-    cpu_register_physical_memory(0, 0xa0000, ram_addr);
-    cpu_register_physical_memory(0x100000,
-                 below_4g_mem_size - 0x100000,
-                 ram_addr + 0x100000);
+    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);
+        ram_register(0x100000000ULL, above_4g_mem_size,
+                     ram_addr + below_4g_mem_size);
     }
 #endif
 

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

[Alex Williamson]

Register the actual VM RAM using the new API

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

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

diff --git a/hw/pc.c b/hw/pc.c
index e1b2667..87adca2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
     /* allocate RAM */
     ram_addr = qemu_ram_alloc(NULL, "pc.ram",
                               below_4g_mem_size + above_4g_mem_size);
-    cpu_register_physical_memory(0, 0xa0000, ram_addr);
-    cpu_register_physical_memory(0x100000,
-                 below_4g_mem_size - 0x100000,
-                 ram_addr + 0x100000);
+    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);
+        ram_register(0x100000000ULL, above_4g_mem_size,
+                     ram_addr + below_4g_mem_size);
     }
 #endif
 

Re: [PATCH v4 2/2] RAM API: Make use of it for x86 PC

[Avi Kivity]

On 12/13/2010 11:24 PM, Alex Williamson wrote:
> Register the actual VM RAM using the new API
>
>
> @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
>       /* allocate RAM */
>       ram_addr = qemu_ram_alloc(NULL, "pc.ram",
>                                 below_4g_mem_size + above_4g_mem_size);
> -    cpu_register_physical_memory(0, 0xa0000, ram_addr);
> -    cpu_register_physical_memory(0x100000,
> -                 below_4g_mem_size - 0x100000,
> -                 ram_addr + 0x100000);
> +    ram_register(0, below_4g_mem_size, ram_addr);
>

What's the impact of this?  Won't it conflict with BIOS memory 
registration?  What about VGA?

In terms of patch hygiene, it should be in a separate patch titled 
"register 0xa0000-0x100000 as RAM" or something.  It's a much more 
drastic change than making use of the new RAM API.

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

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

[Avi Kivity]

On 12/13/2010 11:24 PM, Alex Williamson wrote:
> Register the actual VM RAM using the new API
>
>
> @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
>       /* allocate RAM */
>       ram_addr = qemu_ram_alloc(NULL, "pc.ram",
>                                 below_4g_mem_size + above_4g_mem_size);
> -    cpu_register_physical_memory(0, 0xa0000, ram_addr);
> -    cpu_register_physical_memory(0x100000,
> -                 below_4g_mem_size - 0x100000,
> -                 ram_addr + 0x100000);
> +    ram_register(0, below_4g_mem_size, ram_addr);
>

What's the impact of this?  Won't it conflict with BIOS memory 
registration?  What about VGA?

In terms of patch hygiene, it should be in a separate patch titled 
"register 0xa0000-0x100000 as RAM" or something.  It's a much more 
drastic change than making use of the new RAM API.

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

Re: [PATCH v4 2/2] RAM API: Make use of it for x86 PC

[Alex Williamson]

On Tue, 2010-12-14 at 11:18 +0200, Avi Kivity wrote:
> On 12/13/2010 11:24 PM, Alex Williamson wrote:
> > Register the actual VM RAM using the new API
> >
> >
> > @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
> >       /* allocate RAM */
> >       ram_addr = qemu_ram_alloc(NULL, "pc.ram",
> >                                 below_4g_mem_size + above_4g_mem_size);
> > -    cpu_register_physical_memory(0, 0xa0000, ram_addr);
> > -    cpu_register_physical_memory(0x100000,
> > -                 below_4g_mem_size - 0x100000,
> > -                 ram_addr + 0x100000);
> > +    ram_register(0, below_4g_mem_size, ram_addr);
> >
> 
> What's the impact of this?  Won't it conflict with BIOS memory 
> registration?  What about VGA?
> 
> In terms of patch hygiene, it should be in a separate patch titled 
> "register 0xa0000-0x100000 as RAM" or something.  It's a much more 
> drastic change than making use of the new RAM API.

As we discussed in the v2 patch, the chipset can selectively switch
regions within this range to point at VGA, ROM, or RAM, but there's
always physical RAM backing the space, even when it's mapping isn't
active.  VGA and ROM will be overlay the RAM mapping.  I'm fine with
splitting this into two patches for debug-ability, but the change is
reflective of following the RAM API and registering all of "RAM".  Maybe
it would be sufficient to make such a note explicit in this commit log?
Thanks,

Alex

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

[Alex Williamson]

On Tue, 2010-12-14 at 11:18 +0200, Avi Kivity wrote:
> On 12/13/2010 11:24 PM, Alex Williamson wrote:
> > Register the actual VM RAM using the new API
> >
> >
> > @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
> >       /* allocate RAM */
> >       ram_addr = qemu_ram_alloc(NULL, "pc.ram",
> >                                 below_4g_mem_size + above_4g_mem_size);
> > -    cpu_register_physical_memory(0, 0xa0000, ram_addr);
> > -    cpu_register_physical_memory(0x100000,
> > -                 below_4g_mem_size - 0x100000,
> > -                 ram_addr + 0x100000);
> > +    ram_register(0, below_4g_mem_size, ram_addr);
> >
> 
> What's the impact of this?  Won't it conflict with BIOS memory 
> registration?  What about VGA?
> 
> In terms of patch hygiene, it should be in a separate patch titled 
> "register 0xa0000-0x100000 as RAM" or something.  It's a much more 
> drastic change than making use of the new RAM API.

As we discussed in the v2 patch, the chipset can selectively switch
regions within this range to point at VGA, ROM, or RAM, but there's
always physical RAM backing the space, even when it's mapping isn't
active.  VGA and ROM will be overlay the RAM mapping.  I'm fine with
splitting this into two patches for debug-ability, but the change is
reflective of following the RAM API and registering all of "RAM".  Maybe
it would be sufficient to make such a note explicit in this commit log?
Thanks,

Alex

Re: [PATCH v4 2/2] RAM API: Make use of it for x86 PC

[Anthony Liguori]

On 12/14/2010 09:16 AM, Alex Williamson wrote:
> On Tue, 2010-12-14 at 11:18 +0200, Avi Kivity wrote:
>    
>> On 12/13/2010 11:24 PM, Alex Williamson wrote:
>>      
>>> Register the actual VM RAM using the new API
>>>
>>>
>>> @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
>>>        /* allocate RAM */
>>>        ram_addr = qemu_ram_alloc(NULL, "pc.ram",
>>>                                  below_4g_mem_size + above_4g_mem_size);
>>> -    cpu_register_physical_memory(0, 0xa0000, ram_addr);
>>> -    cpu_register_physical_memory(0x100000,
>>> -                 below_4g_mem_size - 0x100000,
>>> -                 ram_addr + 0x100000);
>>> +    ram_register(0, below_4g_mem_size, ram_addr);
>>>
>>>        
>> What's the impact of this?  Won't it conflict with BIOS memory
>> registration?  What about VGA?
>>      

There is no "conflict".  Memory registration can punch through previous 
registrations.

And the QEMU SMM code switches the VGA area back and forth between 
memory mapped and normal ram depending on the mode.

This presents no functional change, just structures RAM allocation to 
closer reflect the way things actually work.

Regards,

Anthony Liguori

>> In terms of patch hygiene, it should be in a separate patch titled
>> "register 0xa0000-0x100000 as RAM" or something.  It's a much more
>> drastic change than making use of the new RAM API.
>>      
> As we discussed in the v2 patch, the chipset can selectively switch
> regions within this range to point at VGA, ROM, or RAM, but there's
> always physical RAM backing the space, even when it's mapping isn't
> active.  VGA and ROM will be overlay the RAM mapping.  I'm fine with
> splitting this into two patches for debug-ability, but the change is
> reflective of following the RAM API and registering all of "RAM".  Maybe
> it would be sufficient to make such a note explicit in this commit log?
> Thanks,
>
> Alex
>
>    

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

[Anthony Liguori]

On 12/14/2010 09:16 AM, Alex Williamson wrote:
> On Tue, 2010-12-14 at 11:18 +0200, Avi Kivity wrote:
>    
>> On 12/13/2010 11:24 PM, Alex Williamson wrote:
>>      
>>> Register the actual VM RAM using the new API
>>>
>>>
>>> @@ -913,14 +913,11 @@ void pc_memory_init(ram_addr_t ram_size,
>>>        /* allocate RAM */
>>>        ram_addr = qemu_ram_alloc(NULL, "pc.ram",
>>>                                  below_4g_mem_size + above_4g_mem_size);
>>> -    cpu_register_physical_memory(0, 0xa0000, ram_addr);
>>> -    cpu_register_physical_memory(0x100000,
>>> -                 below_4g_mem_size - 0x100000,
>>> -                 ram_addr + 0x100000);
>>> +    ram_register(0, below_4g_mem_size, ram_addr);
>>>
>>>        
>> What's the impact of this?  Won't it conflict with BIOS memory
>> registration?  What about VGA?
>>      

There is no "conflict".  Memory registration can punch through previous 
registrations.

And the QEMU SMM code switches the VGA area back and forth between 
memory mapped and normal ram depending on the mode.

This presents no functional change, just structures RAM allocation to 
closer reflect the way things actually work.

Regards,

Anthony Liguori

>> In terms of patch hygiene, it should be in a separate patch titled
>> "register 0xa0000-0x100000 as RAM" or something.  It's a much more
>> drastic change than making use of the new RAM API.
>>      
> As we discussed in the v2 patch, the chipset can selectively switch
> regions within this range to point at VGA, ROM, or RAM, but there's
> always physical RAM backing the space, even when it's mapping isn't
> active.  VGA and ROM will be overlay the RAM mapping.  I'm fine with
> splitting this into two patches for debug-ability, but the change is
> reflective of following the RAM API and registering all of "RAM".  Maybe
> it would be sufficient to make such a note explicit in this commit log?
> Thanks,
>
> Alex
>
>    

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

[Paul Brook]

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

Why do we care? How are you defining "actual VM RAM"?

Surely the whole point of qemu_ram_alloc is to allocate a chunk of memory that 
can be mapped into the guest physical address space, so all uses of 
qemu_ram_alloc should be using this API.

Paul

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

[Paul Brook]

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

Why do we care? How are you defining "actual VM RAM"?

Surely the whole point of qemu_ram_alloc is to allocate a chunk of memory that 
can be mapped into the guest physical address space, so all uses of 
qemu_ram_alloc should be using this API.

Paul

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

[Alex Williamson]

On Wed, 2010-12-15 at 17:23 +0000, Paul Brook 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.
> 
> Why do we care? How are you defining "actual VM RAM"?
> 
> Surely the whole point of qemu_ram_alloc is to allocate a chunk of memory that 
> can be mapped into the guest physical address space, so all uses of 
> qemu_ram_alloc should be using this API.

http://wiki.qemu.org/Features/RamAPI

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

[Alex Williamson]

On Wed, 2010-12-15 at 17:23 +0000, Paul Brook 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.
> 
> Why do we care? How are you defining "actual VM RAM"?
> 
> Surely the whole point of qemu_ram_alloc is to allocate a chunk of memory that 
> can be mapped into the guest physical address space, so all uses of 
> qemu_ram_alloc should be using this API.

http://wiki.qemu.org/Features/RamAPI

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

[Anthony Liguori]

On 12/15/2010 11:23 AM, Paul Brook 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.
>>      
> Why do we care? How are you defining "actual VM RAM"?
>
> Surely the whole point of qemu_ram_alloc is to allocate a chunk of memory that
> can be mapped into the guest physical address space, so all uses of
> qemu_ram_alloc should be using this API.
>    

"actual VM RAM" == the DIMM devices.  This address has exactly a 1-1 
mapping between memory content and an address.  It doesn't change during 
program execution.

It may be mapped in the CPU in weird ways, it may be visibly different 
to devices, but that's a different interface.

Why do we care about differentiating "actual VM RAM" from things that 
behave like RAM but are not actually RAM (like device ROM)?  Because the 
semantics are different.  ROM is non-volatile and RAM is volatile.  If 
we don't make that distinction in our interfaces, we loose the ability 
to model the behavioral differences.

For things like paravirtual devices, we can take short cuts (to optimize 
performance) by saying the device is directly connecting to RAM (and 
doesn't go through the normal translation hierarchy).

Regards,

Anthony Liguori

> Paul
> --
> 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
>    

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

[Anthony Liguori]

On 12/15/2010 11:23 AM, Paul Brook 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.
>>      
> Why do we care? How are you defining "actual VM RAM"?
>
> Surely the whole point of qemu_ram_alloc is to allocate a chunk of memory that
> can be mapped into the guest physical address space, so all uses of
> qemu_ram_alloc should be using this API.
>    

"actual VM RAM" == the DIMM devices.  This address has exactly a 1-1 
mapping between memory content and an address.  It doesn't change during 
program execution.

It may be mapped in the CPU in weird ways, it may be visibly different 
to devices, but that's a different interface.

Why do we care about differentiating "actual VM RAM" from things that 
behave like RAM but are not actually RAM (like device ROM)?  Because the 
semantics are different.  ROM is non-volatile and RAM is volatile.  If 
we don't make that distinction in our interfaces, we loose the ability 
to model the behavioral differences.

For things like paravirtual devices, we can take short cuts (to optimize 
performance) by saying the device is directly connecting to RAM (and 
doesn't go through the normal translation hierarchy).

Regards,

Anthony Liguori

> Paul
> --
> 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
>