diff mbox

[14/23] vhost: convert to MemoryListener API

Message ID 1324304024-11220-15-git-send-email-avi@redhat.com
State New
Headers show

Commit Message

Avi Kivity Dec. 19, 2011, 2:13 p.m. UTC 
Drop the use of cpu_register_phys_memory_client() in favour of the new
MemoryListener API.  The new API simplifies the caller, since there is no
need to deal with splitting and merging slots; however this is not exploited
in this patch.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/vhost.c |  126 ++++++++++++++++++++++++++++++++++++++++++++---------------
 hw/vhost.h |    3 +-
 2 files changed, 96 insertions(+), 33 deletions(-)

Comments

Michael S. Tsirkin Dec. 22, 2011, 12:50 p.m. UTC | #1 
On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote:
> +static void vhost_log_start(MemoryListener *listener,
> +                            MemoryRegionSection *section)
> +{
> +    /* FIXME: implement */
> +}
> +
> +static void vhost_log_stop(MemoryListener *listener,
> +                           MemoryRegionSection *section)
> +{
> +    /* FIXME: implement */
> +}
> +

What exactly do we need to fix here?
Avi Kivity Dec. 22, 2011, 12:50 p.m. UTC | #2 
On 12/22/2011 02:50 PM, Michael S. Tsirkin wrote:
> On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote:
> > +static void vhost_log_start(MemoryListener *listener,
> > +                            MemoryRegionSection *section)
> > +{
> > +    /* FIXME: implement */
> > +}
> > +
> > +static void vhost_log_stop(MemoryListener *listener,
> > +                           MemoryRegionSection *section)
> > +{
> > +    /* FIXME: implement */
> > +}
> > +
>
> What exactly do we need to fix here?

Tell vhost to start tracking those regions?

I guess you don't often read packets into the framebuffer, or we'd have
a lot of bug reports.
Michael S. Tsirkin Dec. 22, 2011, 12:57 p.m. UTC | #3 
On Thu, Dec 22, 2011 at 02:50:27PM +0200, Avi Kivity wrote:
> On 12/22/2011 02:50 PM, Michael S. Tsirkin wrote:
> > On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote:
> > > +static void vhost_log_start(MemoryListener *listener,
> > > +                            MemoryRegionSection *section)
> > > +{
> > > +    /* FIXME: implement */
> > > +}
> > > +
> > > +static void vhost_log_stop(MemoryListener *listener,
> > > +                           MemoryRegionSection *section)
> > > +{
> > > +    /* FIXME: implement */
> > > +}
> > > +
> >
> > What exactly do we need to fix here?
> 
> Tell vhost to start tracking those regions?
> 
> I guess you don't often read packets into the framebuffer, or we'd have
> a lot of bug reports.

Yes, we currently simply don't pass these regions to vhost.
It currently signals an error if such is
observed, so we could handle framebuffer in userspace
if we wanted to.

> -- 
> error compiling committee.c: too many arguments to function
Avi Kivity Dec. 22, 2011, 12:57 p.m. UTC | #4 
On 12/22/2011 02:57 PM, Michael S. Tsirkin wrote:
> On Thu, Dec 22, 2011 at 02:50:27PM +0200, Avi Kivity wrote:
> > On 12/22/2011 02:50 PM, Michael S. Tsirkin wrote:
> > > On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote:
> > > > +static void vhost_log_start(MemoryListener *listener,
> > > > +                            MemoryRegionSection *section)
> > > > +{
> > > > +    /* FIXME: implement */
> > > > +}
> > > > +
> > > > +static void vhost_log_stop(MemoryListener *listener,
> > > > +                           MemoryRegionSection *section)
> > > > +{
> > > > +    /* FIXME: implement */
> > > > +}
> > > > +
> > >
> > > What exactly do we need to fix here?
> > 
> > Tell vhost to start tracking those regions?
> > 
> > I guess you don't often read packets into the framebuffer, or we'd have
> > a lot of bug reports.
>
> Yes, we currently simply don't pass these regions to vhost.
> It currently signals an error if such is
> observed, so we could handle framebuffer in userspace
> if we wanted to.

I see, thanks.
diff mbox

Patch

diff --git a/hw/vhost.c b/hw/vhost.c
index 0870cb7..a1c5e4c 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -57,12 +57,12 @@  static void vhost_dev_sync_region(struct vhost_dev *dev,
     }
 }
 
-static int vhost_client_sync_dirty_bitmap(CPUPhysMemoryClient *client,
-                                          target_phys_addr_t start_addr,
-                                          target_phys_addr_t end_addr)
+static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
+                                   target_phys_addr_t start_addr,
+                                   target_phys_addr_t end_addr)
 {
-    struct vhost_dev *dev = container_of(client, struct vhost_dev, client);
     int i;
+
     if (!dev->log_enabled || !dev->started) {
         return 0;
     }
@@ -81,6 +81,17 @@  static int vhost_client_sync_dirty_bitmap(CPUPhysMemoryClient *client,
     return 0;
 }
 
+static void vhost_log_sync(MemoryListener *listener,
+                          MemoryRegionSection *section)
+{
+    struct vhost_dev *dev = container_of(listener, struct vhost_dev,
+                                         memory_listener);
+    target_phys_addr_t start_addr = section->offset_within_address_space;
+    target_phys_addr_t end_addr = start_addr + section->size;
+
+    vhost_sync_dirty_bitmap(dev, start_addr, end_addr);
+}
+
 /* Assign/unassign. Keep an unsorted array of non-overlapping
  * memory regions in dev->mem. */
 static void vhost_dev_unassign_memory(struct vhost_dev *dev,
@@ -259,8 +270,7 @@  static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size)
     log_base = (uint64_t)(unsigned long)log;
     r = ioctl(dev->control, VHOST_SET_LOG_BASE, &log_base);
     assert(r >= 0);
-    vhost_client_sync_dirty_bitmap(&dev->client, 0,
-                                   (target_phys_addr_t)~0x0ull);
+    vhost_sync_dirty_bitmap(dev, 0, (target_phys_addr_t)~0x0ull);
     if (dev->log) {
         g_free(dev->log);
     }
@@ -335,31 +345,37 @@  static bool vhost_dev_cmp_memory(struct vhost_dev *dev,
     return uaddr != reg->userspace_addr + start_addr - reg->guest_phys_addr;
 }
 
-static void vhost_client_set_memory(CPUPhysMemoryClient *client,
-                                    target_phys_addr_t start_addr,
-                                    ram_addr_t size,
-                                    ram_addr_t phys_offset,
-                                    bool log_dirty)
+static void vhost_set_memory(MemoryListener *listener,
+                             MemoryRegionSection *section,
+                             bool add)
 {
-    struct vhost_dev *dev = container_of(client, struct vhost_dev, client);
-    ram_addr_t flags = phys_offset & ~TARGET_PAGE_MASK;
+    struct vhost_dev *dev = container_of(listener, struct vhost_dev,
+                                         memory_listener);
+    target_phys_addr_t start_addr = section->offset_within_address_space;
+    ram_addr_t size = section->size;
+    bool log_dirty = memory_region_is_logging(section->mr);
     int s = offsetof(struct vhost_memory, regions) +
         (dev->mem->nregions + 1) * sizeof dev->mem->regions[0];
     uint64_t log_size;
     int r;
+    void *ram;
+
+    if (!memory_region_is_ram(section->mr)) {
+        return;
+    }
 
     dev->mem = g_realloc(dev->mem, s);
 
     if (log_dirty) {
-        flags = IO_MEM_UNASSIGNED;
+        add = false;
     }
 
     assert(size);
 
     /* Optimize no-change case. At least cirrus_vga does this a lot at this time. */
-    if (flags == IO_MEM_RAM) {
-        if (!vhost_dev_cmp_memory(dev, start_addr, size,
-                                  (uintptr_t)qemu_get_ram_ptr(phys_offset))) {
+    ram = memory_region_get_ram_ptr(section->mr);
+    if (add) {
+        if (!vhost_dev_cmp_memory(dev, start_addr, size, (uintptr_t)ram)) {
             /* Region exists with same address. Nothing to do. */
             return;
         }
@@ -371,10 +387,9 @@  static void vhost_client_set_memory(CPUPhysMemoryClient *client,
     }
 
     vhost_dev_unassign_memory(dev, start_addr, size);
-    if (flags == IO_MEM_RAM) {
+    if (add) {
         /* Add given mapping, merging adjacent regions if any */
-        vhost_dev_assign_memory(dev, start_addr, size,
-                                (uintptr_t)qemu_get_ram_ptr(phys_offset));
+        vhost_dev_assign_memory(dev, start_addr, size, (uintptr_t)ram);
     } else {
         /* Remove old mapping for this memory, if any. */
         vhost_dev_unassign_memory(dev, start_addr, size);
@@ -410,6 +425,18 @@  static void vhost_client_set_memory(CPUPhysMemoryClient *client,
     }
 }
 
+static void vhost_region_add(MemoryListener *listener,
+                             MemoryRegionSection *section)
+{
+    vhost_set_memory(listener, section, true);
+}
+
+static void vhost_region_del(MemoryListener *listener,
+                             MemoryRegionSection *section)
+{
+    vhost_set_memory(listener, section, false);
+}
+
 static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
                                     struct vhost_virtqueue *vq,
                                     unsigned idx, bool enable_log)
@@ -467,10 +494,10 @@  static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
     return r;
 }
 
-static int vhost_client_migration_log(CPUPhysMemoryClient *client,
-                                      int enable)
+static int vhost_migration_log(MemoryListener *listener, int enable)
 {
-    struct vhost_dev *dev = container_of(client, struct vhost_dev, client);
+    struct vhost_dev *dev = container_of(listener, struct vhost_dev,
+                                         memory_listener);
     int r;
     if (!!enable == dev->log_enabled) {
         return 0;
@@ -500,6 +527,38 @@  static int vhost_client_migration_log(CPUPhysMemoryClient *client,
     return 0;
 }
 
+static void vhost_log_global_start(MemoryListener *listener)
+{
+    int r;
+
+    r = vhost_migration_log(listener, true);
+    if (r < 0) {
+        abort();
+    }
+}
+
+static void vhost_log_global_stop(MemoryListener *listener)
+{
+    int r;
+
+    r = vhost_migration_log(listener, false);
+    if (r < 0) {
+        abort();
+    }
+}
+
+static void vhost_log_start(MemoryListener *listener,
+                            MemoryRegionSection *section)
+{
+    /* FIXME: implement */
+}
+
+static void vhost_log_stop(MemoryListener *listener,
+                           MemoryRegionSection *section)
+{
+    /* FIXME: implement */
+}
+
 static int vhost_virtqueue_init(struct vhost_dev *dev,
                                 struct VirtIODevice *vdev,
                                 struct vhost_virtqueue *vq,
@@ -645,17 +704,21 @@  int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
     }
     hdev->features = features;
 
-    hdev->client.set_memory = vhost_client_set_memory;
-    hdev->client.sync_dirty_bitmap = vhost_client_sync_dirty_bitmap;
-    hdev->client.migration_log = vhost_client_migration_log;
-    hdev->client.log_start = NULL;
-    hdev->client.log_stop = NULL;
+    hdev->memory_listener = (MemoryListener) {
+        .region_add = vhost_region_add,
+        .region_del = vhost_region_del,
+        .log_start = vhost_log_start,
+        .log_stop = vhost_log_stop,
+        .log_sync = vhost_log_sync,
+        .log_global_start = vhost_log_global_start,
+        .log_global_stop = vhost_log_global_stop,
+    };
     hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
     hdev->log = NULL;
     hdev->log_size = 0;
     hdev->log_enabled = false;
     hdev->started = false;
-    cpu_register_phys_memory_client(&hdev->client);
+    memory_listener_register(&hdev->memory_listener);
     hdev->force = force;
     return 0;
 fail:
@@ -666,7 +729,7 @@  int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
 
 void vhost_dev_cleanup(struct vhost_dev *hdev)
 {
-    cpu_unregister_phys_memory_client(&hdev->client);
+    memory_listener_unregister(&hdev->memory_listener);
     g_free(hdev->mem);
     close(hdev->control);
 }
@@ -808,8 +871,7 @@  void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
                                 hdev->vqs + i,
                                 i);
     }
-    vhost_client_sync_dirty_bitmap(&hdev->client, 0,
-                                   (target_phys_addr_t)~0x0ull);
+    vhost_sync_dirty_bitmap(hdev, 0, (target_phys_addr_t)~0x0ull);
     r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, false);
     if (r < 0) {
         fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
diff --git a/hw/vhost.h b/hw/vhost.h
index c9452f0..d1824ec 100644
--- a/hw/vhost.h
+++ b/hw/vhost.h
@@ -3,6 +3,7 @@ 
 
 #include "hw/hw.h"
 #include "hw/virtio.h"
+#include "memory.h"
 
 /* Generic structures common for any vhost based device. */
 struct vhost_virtqueue {
@@ -26,7 +27,7 @@  typedef unsigned long vhost_log_chunk_t;
 
 struct vhost_memory;
 struct vhost_dev {
-    CPUPhysMemoryClient client;
+    MemoryListener memory_listener;
     int control;
     struct vhost_memory *mem;
     struct vhost_virtqueue *vqs;