[Qemu-devel] Re: [PATCH v2] xen: Add grant reference support to framebuffer/input

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 03/08/2011 04:04 PM, Jan Kiszka wrote:
> On 2011-03-08 21:29, Daniel De Graaf wrote:
>> Request and support using grant references in backends for
>> the Xen kbdmouse and framebuffer devices.
> 
> You should pass your patch through scripts/checkpatch.pl and fix the
> complaints (it will only remark violations in modified code).
> 
> Jan
> 

I formatted the code using the existing code as a template. In particular,
the indentation throughout the file is already formatted using tabs.

As requested, below is a patch that passes checkpatch. It may make sense
to fix the whitespace in the file after this patch is applied.

---------------------------------------------------------------->8

Request and support using grant references in backends for
the Xen kbdmouse and framebuffer devices.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

---
 hw/xenfb.c |  112 +++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 84 insertions(+), 28 deletions(-)

diff --git a/hw/xenfb.c b/hw/xenfb.c
index 05c51cc..d4f5114 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -61,6 +61,7 @@ struct common {
     struct XenDevice  xendev;  /* must be first */
     void              *page;
     DisplayState      *ds;
+    int               uses_gref;
 };
 
 struct XenInput {
@@ -100,22 +101,28 @@ struct XenFB {
 
 static int common_bind(struct common *c)
 {
-    int mfn;
+    int ref;
 
-    if (xenstore_read_fe_int(&c->xendev, "page-ref", &mfn) == -1)
-	return -1;
     if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
 	return -1;
+    if (xenstore_read_fe_int(&c->xendev, "page-gref", &ref) == 0) {
+        c->page = xc_gnttab_map_grant_ref(c->xendev.gnttabdev, c->xendev.dom,
+                                          ref, PROT_READ | PROT_WRITE);
+        c->uses_gref = 1;
+    } else if (xenstore_read_fe_int(&c->xendev, "page-ref", &ref) == 0) {
+        xen_pfn_t pfn = ref;
+        c->page = xc_map_foreign_pages(xen_xc, c->xendev.dom,
+                                       PROT_READ | PROT_WRITE, &pfn, 1);
+        c->uses_gref = 0;
+    } else
+        return -1;
 
-    c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
-				   XC_PAGE_SIZE,
-				   PROT_READ | PROT_WRITE, mfn);
     if (c->page == NULL)
 	return -1;
 
     xen_be_bind_evtchn(&c->xendev);
-    xen_be_printf(&c->xendev, 1, "ring mfn %d, remote-port %d, local-port %d\n",
-		  mfn, c->xendev.remote_port, c->xendev.local_port);
+    xen_be_printf(&c->xendev, 1, "ring ref %d, remote/local port %d/%d\n",
+                  ref, c->xendev.remote_port, c->xendev.local_port);
 
     return 0;
 }
@@ -123,10 +130,12 @@ static int common_bind(struct common *c)
 static void common_unbind(struct common *c)
 {
     xen_be_unbind_evtchn(&c->xendev);
-    if (c->page) {
-	munmap(c->page, XC_PAGE_SIZE);
-	c->page = NULL;
+    if (c->page && c->uses_gref) {
+        xc_gnttab_munmap(c->xendev.gnttabdev, c->page, 1);
+    } else if (c->page) {
+        munmap(c->page, XC_PAGE_SIZE);
     }
+    c->page = NULL;
 }
 
 /* -------------------------------------------------------------------- */
@@ -430,8 +439,6 @@ static int xenfb_map_fb(struct XenFB *xenfb)
     struct xenfb_page *page = xenfb->c.page;
     char *protocol = xenfb->c.xendev.protocol;
     int n_fbdirs;
-    unsigned long *pgmfns = NULL;
-    unsigned long *fbmfns = NULL;
     void *map, *pd;
     int mode, ret = -1;
 
@@ -480,36 +487,75 @@ static int xenfb_map_fb(struct XenFB *xenfb)
 #endif
     }
 
-    if (xenfb->pixels) {
+    if (xenfb->pixels && xenfb->c.uses_gref) {
+        xc_gnttab_munmap(xenfb->c.xendev.gnttabdev, xenfb->pixels,
+                         xenfb->fbpages);
+    } else if (xenfb->pixels) {
         munmap(xenfb->pixels, xenfb->fbpages * XC_PAGE_SIZE);
-        xenfb->pixels = NULL;
     }
+    xenfb->pixels = NULL;
 
     xenfb->fbpages = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
     n_fbdirs = xenfb->fbpages * mode / 8;
     n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
 
-    pgmfns = qemu_mallocz(sizeof(unsigned long) * n_fbdirs);
-    fbmfns = qemu_mallocz(sizeof(unsigned long) * xenfb->fbpages);
+    if (xenfb->c.uses_gref) {
+        uint32_t *domids = qemu_mallocz(sizeof(uint32_t)*n_fbdirs);
+        uint32_t *refs = qemu_mallocz(sizeof(uint32_t)*n_fbdirs);
+        int i;
+        for (i = 0; i < n_fbdirs; i++) {
+            domids[i] = xenfb->c.xendev.dom;
+            refs[i] = (mode == 32) ? ((uint32_t *)pd)[i] : ((uint64_t *)pd)[i];
+        }
+
+        map = xc_gnttab_map_grant_refs(xenfb->c.xendev.gnttabdev,
+                                       n_fbdirs, domids, refs, PROT_READ);
+        qemu_free(domids);
+        qemu_free(refs);
 
-    xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
-    map = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
-			       PROT_READ, pgmfns, n_fbdirs);
-    if (map == NULL)
-	goto out;
-    xenfb_copy_mfns(mode, xenfb->fbpages, fbmfns, map);
-    munmap(map, n_fbdirs * XC_PAGE_SIZE);
+        if (map == NULL) {
+            goto out;
+        }
+
+        domids = qemu_mallocz(sizeof(uint32_t)*xenfb->fbpages);
+        refs = qemu_mallocz(sizeof(uint32_t)*xenfb->fbpages);
+        for (i = 0; i < xenfb->fbpages; i++) {
+            domids[i] = xenfb->c.xendev.dom;
+            refs[i] = (mode == 32) ? ((uint32_t *)map)[i] :
+                                     ((uint64_t *)map)[i];
+        }
+
+        xc_gnttab_munmap(xenfb->c.xendev.gnttabdev, map, n_fbdirs);
+        xenfb->pixels = xc_gnttab_map_grant_refs(xenfb->c.xendev.gnttabdev,
+                                xenfb->fbpages, domids, refs, PROT_READ);
+        qemu_free(domids);
+        qemu_free(refs);
+    } else {
+        unsigned long *pgmfns = qemu_mallocz(sizeof(unsigned long)*n_fbdirs);
+        xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
+        map = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
+                                   PROT_READ, pgmfns, n_fbdirs);
+        qemu_free(pgmfns);
+
+        if (map == NULL) {
+            goto out;
+        }
+
+        unsigned long *fbmfns = qemu_mallocz(sizeof(unsigned long) *
+                                             xenfb->fbpages);
+        xenfb_copy_mfns(mode, xenfb->fbpages, fbmfns, map);
+        munmap(map, n_fbdirs * XC_PAGE_SIZE);
+        xenfb->pixels = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
+                               PROT_READ | PROT_WRITE, fbmfns, xenfb->fbpages);
+        qemu_free(fbmfns);
+    }
 
-    xenfb->pixels = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
-					 PROT_READ | PROT_WRITE, fbmfns, xenfb->fbpages);
     if (xenfb->pixels == NULL)
 	goto out;
 
     ret = 0; /* all is fine */
 
 out:
-    qemu_free(pgmfns);
-    qemu_free(fbmfns);
     return ret;
 }
 
@@ -893,6 +939,7 @@ static int fb_init(struct XenDevice *xendev)
 #ifdef XENFB_TYPE_RESIZE
     xenstore_write_be_int(xendev, "feature-resize", 1);
 #endif
+    xenstore_write_be_int(xendev, "feature-grants", 1);
     return 0;
 }
 
@@ -946,6 +993,13 @@ static void fb_disconnect(struct XenDevice *xendev)
 {
     struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev);
 
+    if (fb->pixels && fb->c.uses_gref) {
+        xc_gnttab_munmap(fb->c.xendev.gnttabdev, fb->pixels, fb->fbpages);
+    } else if (fb->pixels) {
+        /* Note: This is not needed if we are doing the mmap() below */
+        /* munmap(fb->pixels, fb->fbpages * XC_PAGE_SIZE); */
+    }
+
     /*
      * FIXME: qemu can't un-init gfx display (yet?).
      *   Replacing the framebuffer with anonymous shared memory
@@ -989,6 +1043,7 @@ static void fb_event(struct XenDevice *xendev)
 
 struct XenDevOps xen_kbdmouse_ops = {
     .size       = sizeof(struct XenInput),
+    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .init       = input_init,
     .initialise = input_initialise,
     .connected  = input_connected,
@@ -998,6 +1053,7 @@ struct XenDevOps xen_kbdmouse_ops = {
 
 struct XenDevOps xen_framebuffer_ops = {
     .size       = sizeof(struct XenFB),
+    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .init       = fb_init,
     .initialise = fb_initialise,
     .disconnect = fb_disconnect,