Patchwork [v3,5/6] xen: record physmap changes to xenstore

login
register
mail settings
Submitter Stefano Stabellini
Date Jan. 19, 2012, 11:56 a.m.
Message ID <1326974181-32511-5-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/136890/
State New
Headers show

Comments

Stefano Stabellini - Jan. 19, 2012, 11:56 a.m.
Write to xenstore any physmap changes so that the hypervisor can be
aware of them.
Read physmap changes from xenstore on boot.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen-all.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)
Avi Kivity - Jan. 25, 2012, 11:45 a.m.
On 01/19/2012 04:16 PM, Stefano Stabellini wrote:
> > 
> > If you are migrating to a newer qemu then the <original-addr> could in
> > principal change, I think.
>
> Not unless the implementation of qemu_ram_alloc_from_ptr or
> find_ram_offset change, but these are core qemu functions.

Both of these functions will be removed.  There will no longer be a
qemu-internal address space for physical memory; instead memory will be
addressed using a (MemoryRegion, offset) pair.

We can/will hook memory_region_init_ram() to call xen_ram_alloc() which
can then generate those old addresses, but those (like qemu_ram_alloc())
are dependent on allocation order and you shouldn't depend on them
returning stable values.

> Or the device starts allocating more memory of course, but it wouldn't
> be the same device anymore.
> In any case, if we also match on the MemoryRegion name we cannot go
> wrong.

Match on just the MemoryRegion (and match on the object itself, not the
name; see xen_register_framebuffer()).

> > > We could add some additional info to better identify the physmap entry,
> > > probably the best we could use is the memory_region name.
> > 
> > And/Or the PCI BDF + BAR?
>
> I don't think we can get the PCI BDF from the Xen MemoryListener, but we
> can get the MemoryRegion. The most identifying info in it I think is the
> name, for example for vga would be "vga.vram".
Stefano Stabellini - Jan. 25, 2012, 11:55 a.m.
On Wed, 25 Jan 2012, Avi Kivity wrote:
> On 01/19/2012 04:16 PM, Stefano Stabellini wrote:
> > > 
> > > If you are migrating to a newer qemu then the <original-addr> could in
> > > principal change, I think.
> >
> > Not unless the implementation of qemu_ram_alloc_from_ptr or
> > find_ram_offset change, but these are core qemu functions.
> 
> Both of these functions will be removed.  There will no longer be a
> qemu-internal address space for physical memory; instead memory will be
> addressed using a (MemoryRegion, offset) pair.
> 
> We can/will hook memory_region_init_ram() to call xen_ram_alloc() which
> can then generate those old addresses, but those (like qemu_ram_alloc())
> are dependent on allocation order and you shouldn't depend on them
> returning stable values.
> 
> > Or the device starts allocating more memory of course, but it wouldn't
> > be the same device anymore.
> > In any case, if we also match on the MemoryRegion name we cannot go
> > wrong.
> 
> Match on just the MemoryRegion (and match on the object itself, not the
> name; see xen_register_framebuffer()).

I agree that would be ideal, but how can that work across save/restore?
Unless we introduce some other kind of identifier for MemoryRegion, the
best we have is the name right now, correct?
Avi Kivity - Jan. 25, 2012, noon
On 01/25/2012 01:55 PM, Stefano Stabellini wrote:
> > 
> > Match on just the MemoryRegion (and match on the object itself, not the
> > name; see xen_register_framebuffer()).
>
> I agree that would be ideal, but how can that work across save/restore?
> Unless we introduce some other kind of identifier for MemoryRegion, the
> best we have is the name right now, correct?

Right, you can't put the pointer in save/restore, the name is the only
option (and that's what qemu uses in its native save format).  I thought
you were talking about internal matching, like in a previous version of
the patch/discussion.

Patch

diff --git a/xen-all.c b/xen-all.c
index 507d93d..c830cb1 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -253,6 +253,7 @@  static int xen_add_to_physmap(XenIOState *state,
     XenPhysmap *physmap = NULL;
     target_phys_addr_t pfn, start_gpfn;
     target_phys_addr_t phys_offset = memory_region_get_ram_addr(mr);
+    char path[80], value[17];
 
     if (get_physmapping(state, start_addr, size)) {
         return 0;
@@ -299,6 +300,22 @@  go_physmap:
                                    start_addr >> TARGET_PAGE_BITS,
                                    (start_addr + size) >> TARGET_PAGE_BITS,
                                    XEN_DOMCTL_MEM_CACHEATTR_WB);
+
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
+            xen_domid, (uint64_t)phys_offset);
+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)start_addr);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -1;
+    }
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
+            xen_domid, (uint64_t)phys_offset);
+    snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)size);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -1;
+    }
+
     return 0;
 }
 
@@ -926,6 +943,50 @@  int xen_init(void)
     return 0;
 }
 
+static void xen_read_physmap(XenIOState *state)
+{
+    XenPhysmap *physmap = NULL;
+    unsigned int len, num, i;
+    char path[80], *value = NULL;
+    char **entries = NULL;
+
+    snprintf(path, sizeof(path),
+            "/local/domain/0/device-model/%d/physmap", xen_domid);
+    entries = xs_directory(state->xenstore, 0, path, &num);
+    if (entries == NULL)
+        return;
+
+    for (i = 0; i < num; i++) {
+        physmap = g_malloc(sizeof (XenPhysmap));
+        physmap->phys_offset = strtoull(entries[i], NULL, 16);
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/start_addr",
+                xen_domid, entries[i]);
+        value = xs_read(state->xenstore, 0, path, &len);
+        if (value == NULL) {
+            free(physmap);
+            continue;
+        }
+        physmap->start_addr = strtoull(value, NULL, 16);
+        free(value);
+
+        snprintf(path, sizeof(path),
+                "/local/domain/0/device-model/%d/physmap/%s/size",
+                xen_domid, entries[i]);
+        value = xs_read(state->xenstore, 0, path, &len);
+        if (value == NULL) {
+            free(physmap);
+            continue;
+        }
+        physmap->size = strtoull(value, NULL, 16);
+        free(value);
+
+        QLIST_INSERT_HEAD(&state->physmap, physmap, list);
+    }
+    free(entries);
+    return;
+}
+
 int xen_hvm_init(void)
 {
     int i, rc;
@@ -998,6 +1059,7 @@  int xen_hvm_init(void)
     xen_be_register("console", &xen_console_ops);
     xen_be_register("vkbd", &xen_kbdmouse_ops);
     xen_be_register("qdisk", &xen_blkdev_ops);
+    xen_read_physmap(state);
 
     return 0;
 }