[Qemu-devel] [PATCH 0/3] Fix vhost after memory API breakage

[Qemu-devel] [PATCH 0/3] Fix vhost after memory API breakage

[Avi Kivity]

The memory API adjustments broke vhost, this series puts it back together
again.

There is still a lot of room for simplification (for example split/merge
can no longer occur with the memory API), this is left for later.

Avi Kivity (3):
  vhost: fix incorrect userspace address
  vhost: fix mem_sections memory corruption
  vhost: improve region filtering

 hw/vhost.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

-- 
1.7.7.1

[Qemu-devel] [PATCH 1/3] vhost: fix incorrect userspace address

[Avi Kivity]

MemoryListener::region_add() gives us a slice of a MemoryRegion, not a
region.  Adjust the userspace address to reflect that.

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

diff --git a/hw/vhost.c b/hw/vhost.c
index cd56e75..541c716 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -378,7 +378,7 @@ static void vhost_set_memory(MemoryListener *listener,
     assert(size);
 
     /* Optimize no-change case. At least cirrus_vga does this a lot at this time. */
-    ram = memory_region_get_ram_ptr(section->mr);
+    ram = memory_region_get_ram_ptr(section->mr) + section->offset_within_region;
     if (add) {
         if (!vhost_dev_cmp_memory(dev, start_addr, size, (uintptr_t)ram)) {
             /* Region exists with same address. Nothing to do. */
-- 
1.7.7.1

[Qemu-devel] [PATCH 2/3] vhost: fix mem_sections memory corruption

[Avi Kivity]

A memset() used to delete an entry in an array did not take into account
the array element's size.

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

diff --git a/hw/vhost.c b/hw/vhost.c
index 541c716..d924fb0 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -456,7 +456,7 @@ static void vhost_region_del(MemoryListener *listener,
             == section->offset_within_address_space) {
             --dev->n_mem_sections;
             memmove(&dev->mem_sections[i], &dev->mem_sections[i+1],
-                    dev->n_mem_sections - i);
+                    (dev->n_mem_sections - i) * sizeof(*dev->mem_sections));
             break;
         }
     }
-- 
1.7.7.1

[Qemu-devel] [PATCH 3/3] vhost: improve region filtering

[Avi Kivity]

vhost memory management doesn't care about non-memory (e.g. PIO) or non-RAM
regions.  Adjust the filtering to reflect that, and move it earlier so it
applies to mem_sections too.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/vhost.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index d924fb0..19a7b5c 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -15,6 +15,7 @@
 #include "hw/hw.h"
 #include "range.h"
 #include <linux/vhost.h>
+#include "exec-memory.h"
 
 static void vhost_dev_sync_region(struct vhost_dev *dev,
                                   MemoryRegionSection *section,
@@ -365,10 +366,6 @@ static void vhost_set_memory(MemoryListener *listener,
     int r;
     void *ram;
 
-    if (!memory_region_is_ram(section->mr)) {
-        return;
-    }
-
     dev->mem = g_realloc(dev->mem, s);
 
     if (log_dirty) {
@@ -430,12 +427,22 @@ static void vhost_set_memory(MemoryListener *listener,
     }
 }
 
+static bool vhost_section(MemoryRegionSection *section)
+{
+    return section->address_space == get_system_memory()
+        && memory_region_is_ram(section->mr);
+}
+
 static void vhost_region_add(MemoryListener *listener,
                              MemoryRegionSection *section)
 {
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
 
+    if (!vhost_section(section)) {
+        return;
+    }
+
     ++dev->n_mem_sections;
     dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
                                 dev->n_mem_sections);
@@ -450,6 +457,10 @@ static void vhost_region_del(MemoryListener *listener,
                                          memory_listener);
     int i;
 
+    if (!vhost_section(section)) {
+        return;
+    }
+
     vhost_set_memory(listener, section, false);
     for (i = 0; i < dev->n_mem_sections; ++i) {
         if (dev->mem_sections[i].offset_within_address_space
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH 3/3] vhost: improve region filtering

[Avi Kivity]

On 01/09/2012 03:28 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 09, 2012 at 02:04:54PM +0200, Avi Kivity wrote:
> > vhost memory management doesn't care about non-memory (e.g. PIO) or non-RAM
> > regions.  Adjust the filtering to reflect that, and move it earlier so it
> > applies to mem_sections too.
> > 
> > Signed-off-by: Avi Kivity <avi@redhat.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Do you want me to push those patches, or will you do that as maintainer?

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

Re: [Qemu-devel] [PATCH 1/3] vhost: fix incorrect userspace address

[Michael S. Tsirkin]

On Mon, Jan 09, 2012 at 02:04:52PM +0200, Avi Kivity wrote:
> MemoryListener::region_add() gives us a slice of a MemoryRegion, not a
> region.  Adjust the userspace address to reflect that.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/vhost.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/vhost.c b/hw/vhost.c
> index cd56e75..541c716 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -378,7 +378,7 @@ static void vhost_set_memory(MemoryListener *listener,
>      assert(size);
>  
>      /* Optimize no-change case. At least cirrus_vga does this a lot at this time. */
> -    ram = memory_region_get_ram_ptr(section->mr);
> +    ram = memory_region_get_ram_ptr(section->mr) + section->offset_within_region;
>      if (add) {
>          if (!vhost_dev_cmp_memory(dev, start_addr, size, (uintptr_t)ram)) {
>              /* Region exists with same address. Nothing to do. */
> -- 
> 1.7.7.1
> 

Re: [Qemu-devel] [PATCH 2/3] vhost: fix mem_sections memory corruption

[Michael S. Tsirkin]

On Mon, Jan 09, 2012 at 02:04:53PM +0200, Avi Kivity wrote:
> A memset() used to delete an entry in an array did not take into account
> the array element's size.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/vhost.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/vhost.c b/hw/vhost.c
> index 541c716..d924fb0 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -456,7 +456,7 @@ static void vhost_region_del(MemoryListener *listener,
>              == section->offset_within_address_space) {
>              --dev->n_mem_sections;
>              memmove(&dev->mem_sections[i], &dev->mem_sections[i+1],
> -                    dev->n_mem_sections - i);
> +                    (dev->n_mem_sections - i) * sizeof(*dev->mem_sections));
>              break;
>          }
>      }
> -- 
> 1.7.7.1
> 

Re: [Qemu-devel] [PATCH 3/3] vhost: improve region filtering

[Michael S. Tsirkin]

On Mon, Jan 09, 2012 at 02:04:54PM +0200, Avi Kivity wrote:
> vhost memory management doesn't care about non-memory (e.g. PIO) or non-RAM
> regions.  Adjust the filtering to reflect that, and move it earlier so it
> applies to mem_sections too.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/vhost.c |   19 +++++++++++++++----
>  1 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vhost.c b/hw/vhost.c
> index d924fb0..19a7b5c 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -15,6 +15,7 @@
>  #include "hw/hw.h"
>  #include "range.h"
>  #include <linux/vhost.h>
> +#include "exec-memory.h"
>  
>  static void vhost_dev_sync_region(struct vhost_dev *dev,
>                                    MemoryRegionSection *section,
> @@ -365,10 +366,6 @@ static void vhost_set_memory(MemoryListener *listener,
>      int r;
>      void *ram;
>  
> -    if (!memory_region_is_ram(section->mr)) {
> -        return;
> -    }
> -
>      dev->mem = g_realloc(dev->mem, s);
>  
>      if (log_dirty) {
> @@ -430,12 +427,22 @@ static void vhost_set_memory(MemoryListener *listener,
>      }
>  }
>  
> +static bool vhost_section(MemoryRegionSection *section)
> +{
> +    return section->address_space == get_system_memory()
> +        && memory_region_is_ram(section->mr);
> +}
> +
>  static void vhost_region_add(MemoryListener *listener,
>                               MemoryRegionSection *section)
>  {
>      struct vhost_dev *dev = container_of(listener, struct vhost_dev,
>                                           memory_listener);
>  
> +    if (!vhost_section(section)) {
> +        return;
> +    }
> +
>      ++dev->n_mem_sections;
>      dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
>                                  dev->n_mem_sections);
> @@ -450,6 +457,10 @@ static void vhost_region_del(MemoryListener *listener,
>                                           memory_listener);
>      int i;
>  
> +    if (!vhost_section(section)) {
> +        return;
> +    }
> +
>      vhost_set_memory(listener, section, false);
>      for (i = 0; i < dev->n_mem_sections; ++i) {
>          if (dev->mem_sections[i].offset_within_address_space
> -- 
> 1.7.7.1
> 

Re: [Qemu-devel] [PATCH 3/3] vhost: improve region filtering

[Michael S. Tsirkin]

On Mon, Jan 09, 2012 at 03:27:24PM +0200, Avi Kivity wrote:
> On 01/09/2012 03:28 PM, Michael S. Tsirkin wrote:
> > On Mon, Jan 09, 2012 at 02:04:54PM +0200, Avi Kivity wrote:
> > > vhost memory management doesn't care about non-memory (e.g. PIO) or non-RAM
> > > regions.  Adjust the filtering to reflect that, and move it earlier so it
> > > applies to mem_sections too.
> > > 
> > > Signed-off-by: Avi Kivity <avi@redhat.com>
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Do you want me to push those patches, or will you do that as maintainer?

Pls go ahead and push them.
Thanks,
MST

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