[Qemu-devel] [PULL 0/4] xen: old bug fixes

[Qemu-devel] [PULL 0/4] xen: old bug fixes

[Stefano Stabellini]

Hi Anthony,
I have few not very exiting xen bug fixes laying around that haven't
been applied to master yet. None of them are new, some of them are more
than an year old.


Please pull (git branch based on 07ff2c4475df77e38a31d50ee7f3932631806c15):

git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-next


Anthony PERARD (1):
      xen-mapcache: Fix rlimit set size.

John Haxby (2):
      Introduce a new 'connected' xendev op called when Connected.
      Move the xenfb pointer handler to the connected method

Stefano Stabellini (1):
      xen: use uint64_t instead of target_ulong in cpu_ioreq_move

 hw/xen_backend.c |   40 ++++++++++++++++++++++++++++++++++------
 hw/xen_backend.h |    3 ++-
 hw/xen_console.c |    4 ++--
 hw/xen_disk.c    |    2 +-
 hw/xen_nic.c     |    2 +-
 hw/xenfb.c       |   29 ++++++++++++++++++++---------
 xen-all.c        |    2 +-
 xen-mapcache.c   |   29 ++++++++++++++++++++++++-----
 8 files changed, 85 insertions(+), 26 deletions(-)

Cheers,

Stefano

[Qemu-devel] [PATCH 1/4] Introduce a new 'connected' xendev op called when Connected.

[stefano.stabellini]

From: John Haxby <john.haxby@oracle.com>

Rename the existing xendev 'connect' op to 'initialised' and introduce
a new 'connected' op.  This new op, if defined, is called when the
backend is connected.  Note that since there is no state transition this
may be called more than once.

Signed-off-by: John Haxby <john.haxby@oracle.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/xen_backend.c |   40 ++++++++++++++++++++++++++++++++++------
 hw/xen_backend.h |    3 ++-
 hw/xen_console.c |    4 ++--
 hw/xen_disk.c    |    2 +-
 hw/xen_nic.c     |    2 +-
 hw/xenfb.c       |    8 ++++----
 6 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/hw/xen_backend.c b/hw/xen_backend.c
index d881fa2..366479b 100644
--- a/hw/xen_backend.c
+++ b/hw/xen_backend.c
@@ -421,13 +421,13 @@ static int xen_be_try_init(struct XenDevice *xendev)
 }
 
 /*
- * Try to connect xendev.  Depends on the frontend being ready
+ * Try to initialise xendev.  Depends on the frontend being ready
  * for it (shared ring and evtchn info in xenstore, state being
  * Initialised or Connected).
  *
  * Goes to Connected on success.
  */
-static int xen_be_try_connect(struct XenDevice *xendev)
+static int xen_be_try_initialise(struct XenDevice *xendev)
 {
     int rc = 0;
 
@@ -441,11 +441,11 @@ static int xen_be_try_connect(struct XenDevice *xendev)
         }
     }
 
-    if (xendev->ops->connect) {
-        rc = xendev->ops->connect(xendev);
+    if (xendev->ops->initialise) {
+        rc = xendev->ops->initialise(xendev);
     }
     if (rc != 0) {
-        xen_be_printf(xendev, 0, "connect() failed\n");
+        xen_be_printf(xendev, 0, "initialise() failed\n");
         return rc;
     }
 
@@ -454,6 +454,29 @@ static int xen_be_try_connect(struct XenDevice *xendev)
 }
 
 /*
+ * Try to let xendev know that it is connected.  Depends on the
+ * frontend being Connected.  Note that this may be called more
+ * than once since the backend state is not modified.
+ */
+static void xen_be_try_connected(struct XenDevice *xendev)
+{
+    if (!xendev->ops->connected) {
+        return;
+    }
+
+    if (xendev->fe_state != XenbusStateConnected) {
+        if (xendev->ops->flags & DEVOPS_FLAG_IGNORE_STATE) {
+            xen_be_printf(xendev, 2, "frontend not ready, ignoring\n");
+        } else {
+            xen_be_printf(xendev, 2, "frontend not ready (yet)\n");
+            return;
+        }
+    }
+
+    xendev->ops->connected(xendev);
+}
+
+/*
  * Teardown connection.
  *
  * Goes to Closed when done.
@@ -508,7 +531,12 @@ void xen_be_check_state(struct XenDevice *xendev)
             rc = xen_be_try_init(xendev);
             break;
         case XenbusStateInitWait:
-            rc = xen_be_try_connect(xendev);
+            rc = xen_be_try_initialise(xendev);
+            break;
+        case XenbusStateConnected:
+            /* xendev->be_state doesn't change */
+            xen_be_try_connected(xendev);
+            rc = -1;
             break;
         case XenbusStateClosed:
             rc = xen_be_try_reset(xendev);
diff --git a/hw/xen_backend.h b/hw/xen_backend.h
index 6401c85..3305630 100644
--- a/hw/xen_backend.h
+++ b/hw/xen_backend.h
@@ -21,7 +21,8 @@ struct XenDevOps {
     uint32_t  flags;
     void      (*alloc)(struct XenDevice *xendev);
     int       (*init)(struct XenDevice *xendev);
-    int       (*connect)(struct XenDevice *xendev);
+    int       (*initialise)(struct XenDevice *xendev);
+    void      (*connected)(struct XenDevice *xendev);
     void      (*event)(struct XenDevice *xendev);
     void      (*disconnect)(struct XenDevice *xendev);
     int       (*free)(struct XenDevice *xendev);
diff --git a/hw/xen_console.c b/hw/xen_console.c
index 8ef104c..c6bea81 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -212,7 +212,7 @@ out:
     return ret;
 }
 
-static int con_connect(struct XenDevice *xendev)
+static int con_initialise(struct XenDevice *xendev)
 {
     struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
     int limit;
@@ -273,7 +273,7 @@ struct XenDevOps xen_console_ops = {
     .size       = sizeof(struct XenConsole),
     .flags      = DEVOPS_FLAG_IGNORE_STATE,
     .init       = con_init,
-    .connect    = con_connect,
+    .initialise = con_initialise,
     .event      = con_event,
     .disconnect = con_disconnect,
 };
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index add815f..4abdaf9 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -846,7 +846,7 @@ struct XenDevOps xen_blkdev_ops = {
     .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .alloc      = blk_alloc,
     .init       = blk_init,
-    .connect    = blk_connect,
+    .initialise    = blk_connect,
     .disconnect = blk_disconnect,
     .event      = blk_event,
     .free       = blk_free,
diff --git a/hw/xen_nic.c b/hw/xen_nic.c
index ff86491..9a97d3e 100644
--- a/hw/xen_nic.c
+++ b/hw/xen_nic.c
@@ -433,7 +433,7 @@ struct XenDevOps xen_netdev_ops = {
     .size       = sizeof(struct XenNetDev),
     .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .init       = net_init,
-    .connect    = net_connect,
+    .initialise    = net_connect,
     .event      = net_event,
     .disconnect = net_disconnect,
     .free       = net_free,
diff --git a/hw/xenfb.c b/hw/xenfb.c
index 0a01ae3..c001ab9 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -351,7 +351,7 @@ static int input_init(struct XenDevice *xendev)
     return 0;
 }
 
-static int input_connect(struct XenDevice *xendev)
+static int input_initialise(struct XenDevice *xendev)
 {
     struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
     int rc;
@@ -865,7 +865,7 @@ static int fb_init(struct XenDevice *xendev)
     return 0;
 }
 
-static int fb_connect(struct XenDevice *xendev)
+static int fb_initialise(struct XenDevice *xendev)
 {
     struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
     struct xenfb_page *fb_page;
@@ -959,7 +959,7 @@ static void fb_event(struct XenDevice *xendev)
 struct XenDevOps xen_kbdmouse_ops = {
     .size       = sizeof(struct XenInput),
     .init       = input_init,
-    .connect    = input_connect,
+    .initialise = input_initialise,
     .disconnect = input_disconnect,
     .event      = input_event,
 };
@@ -967,7 +967,7 @@ struct XenDevOps xen_kbdmouse_ops = {
 struct XenDevOps xen_framebuffer_ops = {
     .size       = sizeof(struct XenFB),
     .init       = fb_init,
-    .connect    = fb_connect,
+    .initialise = fb_initialise,
     .disconnect = fb_disconnect,
     .event      = fb_event,
     .frontend_changed = fb_frontend_changed,
-- 
1.7.2.3

[Qemu-devel] [PATCH 2/4] Move the xenfb pointer handler to the connected method

[stefano.stabellini]

From: John Haxby <john.haxby@oracle.com>

Ensure that we read "request-abs-pointer" after the frontend has written
it.  This means that we will correctly set up an ansolute or relative
pointer handler correctly.

Signed-off-by: John Haxby <john.haxby@oracle.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/xenfb.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/xenfb.c b/hw/xenfb.c
index c001ab9..a644727 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -356,10 +356,6 @@ static int input_initialise(struct XenDevice *xendev)
     struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
     int rc;
 
-    if (xenstore_read_fe_int(xendev, "request-abs-pointer",
-                             &in->abs_pointer_wanted) == -1)
-	in->abs_pointer_wanted = 0;
-
     if (!in->c.ds) {
         char *vfb = xenstore_read_str(NULL, "device/vfb");
         if (vfb == NULL) {
@@ -377,10 +373,24 @@ static int input_initialise(struct XenDevice *xendev)
 	return rc;
 
     qemu_add_kbd_event_handler(xenfb_key_event, in);
+    return 0;
+}
+
+static void input_connected(struct XenDevice *xendev)
+{
+    struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
+
+    if (xenstore_read_fe_int(xendev, "request-abs-pointer",
+                             &in->abs_pointer_wanted) == -1) {
+        in->abs_pointer_wanted = 0;
+    }
+
+    if (in->qmouse) {
+        qemu_remove_mouse_event_handler(in->qmouse);
+    }
     in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
 					      in->abs_pointer_wanted,
 					      "Xen PVFB Mouse");
-    return 0;
 }
 
 static void input_disconnect(struct XenDevice *xendev)
@@ -960,6 +970,7 @@ struct XenDevOps xen_kbdmouse_ops = {
     .size       = sizeof(struct XenInput),
     .init       = input_init,
     .initialise = input_initialise,
+    .connected  = input_connected,
     .disconnect = input_disconnect,
     .event      = input_event,
 };
-- 
1.7.2.3

[Qemu-devel] [PATCH 3/4] xen: use uint64_t instead of target_ulong in cpu_ioreq_move

[stefano.stabellini]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

cpu_ioreq_move might move 8 bytes at a time so we must make sure that
the temporary variable can hold 8 bytes.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen-all.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 9eaeac1..3611e19 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -620,7 +620,7 @@ static void cpu_ioreq_move(ioreq_t *req)
             }
         }
     } else {
-        target_ulong tmp;
+        uint64_t tmp;
 
         if (req->dir == IOREQ_READ) {
             for (i = 0; i < req->count; i++) {
-- 
1.7.2.3

[Qemu-devel] [PATCH 4/4] xen-mapcache: Fix rlimit set size.

[stefano.stabellini]

From: Anthony PERARD <anthony.perard@citrix.com>

Previously, the address space soft limit was set mcache_max_size. So,
before the mcache_max_size was reached by the mapcache, QEMU was killed
for overuse of the virtual address space.

This patch fix that by setting the soft limit the maximum than can have
QEMU. So the soft and hard limit are always set to RLIM_INFINITY if QEMU
is privileged.

In case QEMU is not run as root and the limit is too low, the maximum
mapcache size will be set the rlim_max - 80MB because observed that QEMU
use 75MB more than the maximum mapcache size after several empirical
tests.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen-mapcache.c |   29 ++++++++++++++++++++++++-----
 1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index 007136a..686dd1a 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -40,6 +40,13 @@
 #endif
 #define MCACHE_BUCKET_SIZE (1UL << MCACHE_BUCKET_SHIFT)
 
+/* This is the size of the virtual address space reserve to QEMU that will not
+ * be use by MapCache.
+ * From empirical tests I observed that qemu use 75MB more than the
+ * max_mcache_size.
+ */
+#define NON_MCACHE_MEMORY_SIZE (80 * 1024 * 1024)
+
 #define mapcache_lock()   ((void)0)
 #define mapcache_unlock() ((void)0)
 
@@ -92,15 +99,27 @@ void xen_map_cache_init(void)
     QTAILQ_INIT(&mapcache->locked_entries);
     mapcache->last_address_index = -1;
 
-    getrlimit(RLIMIT_AS, &rlimit_as);
-    if (rlimit_as.rlim_max < MCACHE_MAX_SIZE) {
-        rlimit_as.rlim_cur = rlimit_as.rlim_max;
+    if (geteuid() == 0) {
+        rlimit_as.rlim_cur = RLIM_INFINITY;
+        rlimit_as.rlim_max = RLIM_INFINITY;
+        mapcache->max_mcache_size = MCACHE_MAX_SIZE;
     } else {
-        rlimit_as.rlim_cur = MCACHE_MAX_SIZE;
+        getrlimit(RLIMIT_AS, &rlimit_as);
+        rlimit_as.rlim_cur = rlimit_as.rlim_max;
+
+        if (rlimit_as.rlim_max != RLIM_INFINITY) {
+            fprintf(stderr, "Warning: QEMU's maximum size of virtual"
+                    " memory is not infinity.\n");
+        }
+        if (rlimit_as.rlim_max < MCACHE_MAX_SIZE + NON_MCACHE_MEMORY_SIZE) {
+            mapcache->max_mcache_size = rlimit_as.rlim_max -
+                NON_MCACHE_MEMORY_SIZE;
+        } else {
+            mapcache->max_mcache_size = MCACHE_MAX_SIZE;
+        }
     }
 
     setrlimit(RLIMIT_AS, &rlimit_as);
-    mapcache->max_mcache_size = rlimit_as.rlim_cur;
 
     mapcache->nr_buckets =
         (((mapcache->max_mcache_size >> XC_PAGE_SHIFT) +
-- 
1.7.2.3

Re: [Qemu-devel] [PULL 0/4] xen: old bug fixes

[Anthony Liguori]

On 09/09/2011 08:39 AM, Stefano Stabellini wrote:
> Hi Anthony,
> I have few not very exiting xen bug fixes laying around that haven't
> been applied to master yet. None of them are new, some of them are more
> than an year old.
>
>
> Please pull (git branch based on 07ff2c4475df77e38a31d50ee7f3932631806c15):
>
> git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-next

Pulled.  Thanks.

Regards,

Anthony Liguori

>
>
> Anthony PERARD (1):
>        xen-mapcache: Fix rlimit set size.
>
> John Haxby (2):
>        Introduce a new 'connected' xendev op called when Connected.
>        Move the xenfb pointer handler to the connected method
>
> Stefano Stabellini (1):
>        xen: use uint64_t instead of target_ulong in cpu_ioreq_move
>
>   hw/xen_backend.c |   40 ++++++++++++++++++++++++++++++++++------
>   hw/xen_backend.h |    3 ++-
>   hw/xen_console.c |    4 ++--
>   hw/xen_disk.c    |    2 +-
>   hw/xen_nic.c     |    2 +-
>   hw/xenfb.c       |   29 ++++++++++++++++++++---------
>   xen-all.c        |    2 +-
>   xen-mapcache.c   |   29 ++++++++++++++++++++++++-----
>   8 files changed, 85 insertions(+), 26 deletions(-)
>
> Cheers,
>
> Stefano
>
>