Re: PATCH: 6/10: Merge private & public xenfb structs

["Daniel P. Berrange" <berrange@redhat.com>]

This patch merges the public & private structs from the paravirt FB
into a single struct. Since QEMU is the only consumer of this code
there is no need  for the artifical pub/priv split. Merging the two
will make it possible to more tightly integrate with QEMU's event
loop and do asynchronous non-blocking negoiation with the frontend
devices (see next patch).

 xenfb.c |  273 ++++++++++++++++++++++++++++++++--------------------------------
 xenfb.h |   16 ---
 2 files changed, 139 insertions(+), 150 deletions(-)

   Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Dan.

diff -r 82c317f263c2 tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c	Tue Aug 14 15:26:57 2007 -0400
+++ b/tools/ioemu/hw/xenfb.c	Tue Aug 14 15:45:22 2007 -0400
@@ -22,6 +22,8 @@
 
 // FIXME defend against malicious frontend?
 
+struct xenfb;
+
 struct xenfb_device {
 	const char *devicetype;
 	char nodename[64];	/* backend xenstore dir */
@@ -30,20 +32,33 @@ struct xenfb_device {
 	enum xenbus_state state; /* backend state */
 	void *page;		/* shared page */
 	evtchn_port_t port;
-	struct xenfb_private *xenfb;
+	struct xenfb *xenfb;
 };
 
-struct xenfb_private {
-	struct xenfb pub;
+struct xenfb_data
+{
+	void *pixels;
+	size_t len;
+
+	int row_stride;
+	int depth;
+	int width;
+	int height;
+	int abs_pointer_wanted;
+};
+
+struct xenfb {
+	DisplayState *ds;
 	int evt_xch;		/* event channel driver handle */
 	int xc;			/* hypervisor interface handle */
 	struct xs_handle *xsh;	/* xs daemon handle */
 	struct xenfb_device fb, kbd;
-	size_t fb_len;		/* size of framebuffer */
+	struct xenfb_data data;
 	char protocol[64];	/* frontend protocol */
+	int button_state;
 };
 
-static void xenfb_detach_dom(struct xenfb_private *);
+static void xenfb_detach_dom(struct xenfb *);
 
 static char *xenfb_path_in_dom(struct xs_handle *xsh,
 			       char *buf, size_t size,
@@ -124,7 +139,7 @@ static int xenfb_xs_printf(struct xs_han
 
 static void xenfb_device_init(struct xenfb_device *dev,
 			      const char *type,
-			      struct xenfb_private *xenfb)
+			      struct xenfb *xenfb)
 {
 	dev->devicetype = type;
 	dev->otherend_id = -1;
@@ -132,19 +147,17 @@ static void xenfb_device_init(struct xen
 	dev->xenfb = xenfb;
 }
 
-int xenfb_device_set_domain(struct xenfb_device *dev, int domid)
-{
-	struct xenfb_private *xenfb = dev->xenfb;
-
+static int xenfb_device_set_domain(struct xenfb_device *dev, int domid)
+{
 	dev->otherend_id = domid;
 
-	if (!xenfb_path_in_dom(xenfb->xsh,
+	if (!xenfb_path_in_dom(dev->xenfb->xsh,
 			       dev->otherend, sizeof(dev->otherend),
 			       domid, "device/%s/0", dev->devicetype)) {
 		errno = ENOENT;
 		return -1;
 	}
-	if (!xenfb_path_in_dom(xenfb->xsh,
+	if (!xenfb_path_in_dom(dev->xenfb->xsh,
 			       dev->nodename, sizeof(dev->nodename),
 			       0, "backend/%s/%d/0", dev->devicetype, domid)) {
 		errno = ENOENT;
@@ -156,8 +169,8 @@ int xenfb_device_set_domain(struct xenfb
 
 struct xenfb *xenfb_new(void)
 {
-	struct xenfb_private *xenfb = malloc(sizeof(*xenfb));
 	int serrno;
+	struct xenfb *xenfb = qemu_malloc(sizeof(struct xenfb));
 
 	if (xenfb == NULL)
 		return NULL;
@@ -179,30 +192,26 @@ struct xenfb *xenfb_new(void)
 	if (!xenfb->xsh)
 		goto fail;
 
-	return &xenfb->pub;
+	return xenfb;
 
  fail:
 	serrno = errno;
-	xenfb_delete(&xenfb->pub);
+	xenfb_delete(xenfb);
 	errno = serrno;
 	return NULL;
 }
 
 /* Remove the backend area in xenbus since the framebuffer really is
    going away. */
-void xenfb_teardown(struct xenfb *xenfb_pub)
-{
-       struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-
+void xenfb_teardown(struct xenfb *xenfb)
+{
        xs_rm(xenfb->xsh, XBT_NULL, xenfb->fb.nodename);
        xs_rm(xenfb->xsh, XBT_NULL, xenfb->kbd.nodename);
 }
 
 
-void xenfb_delete(struct xenfb *xenfb_pub)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
-
+void xenfb_delete(struct xenfb *xenfb)
+{
 	xenfb_detach_dom(xenfb);
 	if (xenfb->xc >= 0)
 		xc_interface_close(xenfb->xc);
@@ -334,7 +343,7 @@ static void xenfb_copy_mfns(int mode, in
 		dst[i] = (mode == 32) ? src32[i] : src64[i];
 }
 
-static int xenfb_map_fb(struct xenfb_private *xenfb, int domid)
+static int xenfb_map_fb(struct xenfb *xenfb, int domid)
 {
 	struct xenfb_page *page = xenfb->fb.page;
 	int n_fbmfns;
@@ -389,7 +398,7 @@ static int xenfb_map_fb(struct xenfb_pri
 #endif
 	}
 
-	n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
+	n_fbmfns = (xenfb->data.len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
 	n_fbdirs = n_fbmfns * mode / 8;
 	n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
 
@@ -412,9 +421,9 @@ static int xenfb_map_fb(struct xenfb_pri
 	xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map);
 	munmap(map, n_fbdirs * XC_PAGE_SIZE);
 
-	xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid,
+	xenfb->data.pixels = xc_map_foreign_batch(xenfb->xc, domid,
 				PROT_READ | PROT_WRITE, fbmfns, n_fbmfns);
-	if (xenfb->pub.pixels == NULL)
+	if (xenfb->data.pixels == NULL)
 		goto out;
 
 	ret = 0; /* all is fine */
@@ -429,7 +438,7 @@ static int xenfb_map_fb(struct xenfb_pri
 
 static int xenfb_bind(struct xenfb_device *dev)
 {
-	struct xenfb_private *xenfb = dev->xenfb;
+	struct xenfb *xenfb = dev->xenfb;
 	unsigned long mfn;
 	evtchn_port_t evtchn;
 
@@ -512,17 +521,74 @@ static void xenfb_dev_fatal(struct xenfb
 }
 
 
-static void xenfb_detach_dom(struct xenfb_private *xenfb)
+static void xenfb_detach_dom(struct xenfb *xenfb)
 {
 	xenfb_unbind(&xenfb->fb);
 	xenfb_unbind(&xenfb->kbd);
-	if (xenfb->pub.pixels) {
-		munmap(xenfb->pub.pixels, xenfb->fb_len);
-		xenfb->pub.pixels = NULL;
-	}
-}
-
-static void xenfb_on_fb_event(struct xenfb_private *xenfb)
+	if (xenfb->data.pixels) {
+		munmap(xenfb->data.pixels, xenfb->data.len);
+		xenfb->data.pixels = NULL;
+	}
+}
+
+/* A convenient function for munging pixels between different depths */
+#define BLT(SRC_T,DST_T,RLS,GLS,BLS,RRS,GRS,BRS,RM,GM,BM)               \
+    for (line = y ; line < h ; line++) {                                \
+        SRC_T *src = (SRC_T *)(xenfb->data.pixels + (line*xenfb->data.row_stride) + (x*xenfb->data.depth/8)); \
+        DST_T *dst = (DST_T *)(xenfb->ds->data + (line*xenfb->ds->linesize) + (x*xenfb->ds->depth/8)); \
+        int col;                                                        \
+        for (col = x ; col < w ; col++) {                               \
+            *dst = (((*src >> RRS)&RM) << RLS) |                        \
+                (((*src >> GRS)&GM) << GLS) |                           \
+                (((*src >> GRS)&BM) << BLS);                            \
+            src++;                                                      \
+            dst++;                                                      \
+        }                                                               \
+    }
+
+
+/* This copies data from the guest framebuffer region, into QEMU's copy
+ * NB. QEMU's copy is stored in the pixel format of a) the local X server (SDL case)
+ * or b) the current VNC client pixel format.
+ */
+static void xenfb_guest_copy(struct xenfb *xenfb, int x, int y, int w, int h)
+{
+    int line;
+
+    if (xenfb->data.depth == xenfb->ds->depth) { /* Perfect match can use fast path */
+        for (line = y ; line < (y+h) ; line++) {
+            memcpy(xenfb->ds->data + (line * xenfb->ds->linesize) + (x*xenfb->ds->depth/8),
+                   xenfb->data.pixels + (line*xenfb->data.row_stride) + (x*xenfb->data.depth/8),
+                   w * xenfb->data.depth/8);
+        }
+    } else { /* Mismatch requires slow pixel munging */
+        if (xenfb->data.depth == 8) {
+            /* 8 bit source == r:3 g:3 b:2 */
+            if (xenfb->ds->depth == 16) {
+                BLT(uint8_t, uint16_t,   5, 2, 0,   11, 5, 0,   7, 7, 3);
+            } else if (xenfb->ds->depth == 32) {
+                BLT(uint8_t, uint32_t,   5, 2, 0,   16, 8, 0,   7, 7, 3);
+            }
+        } else if (xenfb->data.depth == 16) {
+            /* 16 bit source == r:5 g:6 b:5 */
+            if (xenfb->ds->depth == 8) {
+                BLT(uint16_t, uint8_t,    11, 5, 0,   5, 2, 0,    31, 63, 31);
+            } else if (xenfb->ds->depth == 32) {
+                BLT(uint16_t, uint32_t,   11, 5, 0,   16, 8, 0,   31, 63, 31);
+            }
+        } else if (xenfb->data.depth == 32) {
+            /* 32 bit source == r:8 g:8 b:8 (padding:8) */
+            if (xenfb->ds->depth == 8) {
+                BLT(uint32_t, uint8_t,    16, 8, 0,   5, 2, 0,    255, 255, 255);
+            } else if (xenfb->ds->depth == 16) {
+                BLT(uint32_t, uint16_t,   16, 8, 0,   11, 5, 0,   255, 255, 255);
+            }
+        }
+    }
+    dpy_update(xenfb->ds, x, y, w, h);
+}
+
+static void xenfb_on_fb_event(struct xenfb *xenfb)
 {
 	uint32_t prod, cons;
 	struct xenfb_page *page = xenfb->fb.page;
@@ -536,11 +602,10 @@ static void xenfb_on_fb_event(struct xen
 
 		switch (event->type) {
 		case XENFB_TYPE_UPDATE:
-                    if (xenfb->pub.update)
-			xenfb->pub.update(&xenfb->pub,
-					  event->update.x, event->update.y,
-					  event->update.width, event->update.height);
-                    break;
+			xenfb_guest_copy(xenfb,
+					 event->update.x, event->update.y,
+					 event->update.width, event->update.height);
+			break;
 		}
 	}
 	mb();			/* ensure we're done with ring contents */
@@ -548,7 +613,7 @@ static void xenfb_on_fb_event(struct xen
 	xc_evtchn_notify(xenfb->evt_xch, xenfb->fb.port);
 }
 
-static void xenfb_on_kbd_event(struct xenfb_private *xenfb)
+static void xenfb_on_kbd_event(struct xenfb *xenfb)
 {
 	struct xenkbd_page *page = xenfb->kbd.page;
 
@@ -586,7 +651,7 @@ static int xenfb_on_state_change(struct 
 	return 0;
 }
 
-static int xenfb_kbd_event(struct xenfb_private *xenfb,
+static int xenfb_kbd_event(struct xenfb *xenfb,
 			   union xenkbd_in_event *event)
 {
 	uint32_t prod;
@@ -608,9 +673,8 @@ static int xenfb_kbd_event(struct xenfb_
 	return xc_evtchn_notify(xenfb->evt_xch, xenfb->kbd.port);
 }
 
-static int xenfb_send_key(struct xenfb *xenfb_pub, bool down, int keycode)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
+static int xenfb_send_key(struct xenfb *xenfb, bool down, int keycode)
+{
 	union xenkbd_in_event event;
 
 	memset(&event, 0, XENKBD_IN_EVENT_SIZE);
@@ -621,9 +685,8 @@ static int xenfb_send_key(struct xenfb *
 	return xenfb_kbd_event(xenfb, &event);
 }
 
-static int xenfb_send_motion(struct xenfb *xenfb_pub, int rel_x, int rel_y)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
+static int xenfb_send_motion(struct xenfb *xenfb, int rel_x, int rel_y)
+{
 	union xenkbd_in_event event;
 
 	memset(&event, 0, XENKBD_IN_EVENT_SIZE);
@@ -634,9 +697,8 @@ static int xenfb_send_motion(struct xenf
 	return xenfb_kbd_event(xenfb, &event);
 }
 
-static int xenfb_send_position(struct xenfb *xenfb_pub, int abs_x, int abs_y)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
+static int xenfb_send_position(struct xenfb *xenfb, int abs_x, int abs_y)
+{
 	union xenkbd_in_event event;
 
 	memset(&event, 0, XENKBD_IN_EVENT_SIZE);
@@ -660,11 +722,11 @@ static void xenfb_mouse_event(void *opaq
 {
     int i;
     struct xenfb *xenfb = (struct xenfb*)opaque;
-    DisplayState *ds = (DisplayState *)xenfb->user_data;
-    if (xenfb->abs_pointer_wanted)
+
+    if (xenfb->data.abs_pointer_wanted)
 	    xenfb_send_position(xenfb,
-				dx*ds->width/0x7fff,
-				dy*ds->height/0x7fff);
+				dx*xenfb->ds->width/0x7fff,
+				dy*xenfb->ds->height/0x7fff);
     else
 	    xenfb_send_motion(xenfb, dx, dy);
 
@@ -680,76 +742,19 @@ static void xenfb_mouse_event(void *opaq
     xenfb->button_state = button_state;
 }
 
-/* A convenient function for munging pixels between different depths */
-#define BLT(SRC_T,DST_T,RLS,GLS,BLS,RRS,GRS,BRS,RM,GM,BM)               \
-    for (line = y ; line < h ; line++) {                                \
-        SRC_T *src = (SRC_T *)(xenfb->pixels + (line*xenfb->row_stride) + (x*xenfb->depth/8)); \
-        DST_T *dst = (DST_T *)(ds->data + (line*ds->linesize) + (x*ds->depth/8)); \
-        int col;                                                        \
-        for (col = x ; col < w ; col++) {                               \
-            *dst = (((*src >> RRS)&RM) << RLS) |                        \
-                (((*src >> GRS)&GM) << GLS) |                           \
-                (((*src >> GRS)&BM) << BLS);                            \
-            src++;                                                      \
-            dst++;                                                      \
-        }                                                               \
-    }
-
-
-/* This copies data from the guest framebuffer region, into QEMU's copy
- * NB. QEMU's copy is stored in the pixel format of a) the local X server (SDL case)
- * or b) the current VNC client pixel format.
- */
-static void xenfb_guest_copy(struct xenfb *xenfb, int x, int y, int w, int h)
-{
-    DisplayState *ds = (DisplayState *)xenfb->user_data;
-    int line;
-
-    if (xenfb->depth == ds->depth) { /* Perfect match can use fast path */
-        for (line = y ; line < (y+h) ; line++) {
-            memcpy(ds->data + (line * ds->linesize) + (x*ds->depth/8),
-                   xenfb->pixels + (line*xenfb->row_stride) + (x*xenfb->depth/8),
-                   w * xenfb->depth/8);
-        }
-    } else { /* Mismatch requires slow pixel munging */
-        if (xenfb->depth == 8) {
-            /* 8 bit source == r:3 g:3 b:2 */
-            if (ds->depth == 16) {
-                BLT(uint8_t, uint16_t,   5, 2, 0,   11, 5, 0,   7, 7, 3);
-            } else if (ds->depth == 32) {
-                BLT(uint8_t, uint32_t,   5, 2, 0,   16, 8, 0,   7, 7, 3);
-            }
-        } else if (xenfb->depth == 16) {
-            /* 16 bit source == r:5 g:6 b:5 */
-            if (ds->depth == 8) {
-                BLT(uint16_t, uint8_t,    11, 5, 0,   5, 2, 0,    31, 63, 31);
-            } else if (ds->depth == 32) {
-                BLT(uint16_t, uint32_t,   11, 5, 0,   16, 8, 0,   31, 63, 31);
-            }
-        } else if (xenfb->depth == 32) {
-            /* 32 bit source == r:8 g:8 b:8 (padding:8) */
-            if (ds->depth == 8) {
-                BLT(uint32_t, uint8_t,    16, 8, 0,   5, 2, 0,    255, 255, 255);
-            } else if (ds->depth == 16) {
-                BLT(uint32_t, uint16_t,   16, 8, 0,   11, 5, 0,   255, 255, 255);
-            }
-        }
-    }
-    dpy_update(ds, x, y, w, h);
-}
 
 /* QEMU display state changed, so refresh the framebuffer copy */
 static void xenfb_update(void *opaque)
 {
     struct xenfb *xenfb = (struct xenfb *)opaque;
-    xenfb_guest_copy(xenfb, 0, 0, xenfb->width, xenfb->height);
+    xenfb_guest_copy(xenfb, 0, 0, xenfb->data.width, xenfb->data.height);
 }
 
 /* QEMU display state changed, so refresh the framebuffer copy */
 static void xenfb_invalidate(void *opaque)
 {
     struct xenfb *xenfb = (struct xenfb *)opaque;
-    xenfb_guest_copy(xenfb, 0, 0, xenfb->width, xenfb->height);
+    xenfb_guest_copy(xenfb, 0, 0, xenfb->data.width, xenfb->data.height);
 }
 
 /* Screen dump is not used in Xen, so no need to impl this ? */
@@ -757,9 +762,9 @@ static void xenfb_screen_dump(void *opaq
 
 
 
-static void xenfb_dispatch_channel(void *xenfb_pub)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
+static void xenfb_dispatch_channel(void *opaque)
+{
+	struct xenfb *xenfb = (struct xenfb *)opaque;
 	evtchn_port_t port;
 	port = xc_evtchn_pending(xenfb->evt_xch);
 	if (port == -1)
@@ -774,9 +779,9 @@ static void xenfb_dispatch_channel(void 
 		exit(1);
 }
 
-static void xenfb_dispatch_store(void *xenfb_pub)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
+static void xenfb_dispatch_store(void *opaque)
+{
+	struct xenfb *xenfb = (struct xenfb *)opaque;
 	unsigned dummy;
 	char **vec;
 	int r;
@@ -791,9 +796,8 @@ static void xenfb_dispatch_store(void *x
 }
 
 
-int xenfb_attach_dom(struct xenfb *xenfb_pub, int domid, DisplayState *ds)
-{
-	struct xenfb_private *xenfb = (struct xenfb_private *)xenfb_pub;
+int xenfb_attach_dom(struct xenfb *xenfb, int domid, DisplayState *ds)
+{
 	struct xs_handle *xsh = xenfb->xsh;
 	int val, serrno;
 	struct xenfb_page *fb_page;
@@ -849,12 +853,12 @@ int xenfb_attach_dom(struct xenfb *xenfb
 
 	/* TODO check for permitted ranges */
 	fb_page = xenfb->fb.page;
-	xenfb->pub.depth = fb_page->depth;
-	xenfb->pub.width = fb_page->width;
-	xenfb->pub.height = fb_page->height;
+	xenfb->data.depth = fb_page->depth;
+	xenfb->data.width = fb_page->width;
+	xenfb->data.height = fb_page->height;
 	/* TODO check for consistency with the above */
-	xenfb->fb_len = fb_page->mem_length;
-	xenfb->pub.row_stride = fb_page->line_length;
+	xenfb->data.len = fb_page->mem_length;
+	xenfb->data.row_stride = fb_page->line_length;
 
 	if (xenfb_map_fb(xenfb, domid) < 0)
 		goto error;
@@ -869,7 +873,7 @@ int xenfb_attach_dom(struct xenfb *xenfb
 	if (xenfb_xs_scanf1(xsh, xenfb->kbd.otherend, "request-abs-pointer",
 			    "%d", &val) < 0)
 		val = 0;
-	xenfb->pub.abs_pointer_wanted = val;
+	xenfb->data.abs_pointer_wanted = val;
 
 	/* Listen for events from xenstore */
 	if (qemu_set_fd_handler2(xs_fileno(xenfb->xsh), NULL, xenfb_dispatch_store, NULL, xenfb) < 0)
@@ -882,19 +886,18 @@ int xenfb_attach_dom(struct xenfb *xenfb
 	/* Register our keyboard & mouse handlers */
 	qemu_add_kbd_event_handler(xenfb_put_keycode, xenfb);
 	qemu_add_mouse_event_handler(xenfb_mouse_event, xenfb,
-				     xenfb_pub->abs_pointer_wanted,
+				     xenfb->data.abs_pointer_wanted,
 				     "Xen PVFB Mouse");
 
-	xenfb_pub->update = xenfb_guest_copy;
-	xenfb_pub->user_data = ds;
+	xenfb->ds = ds;
 
 	/* Tell QEMU to allocate a graphical console */
 	graphic_console_init(ds,
 			     xenfb_update,
 			     xenfb_invalidate,
 			     xenfb_screen_dump,
-			     xenfb_pub);
-	dpy_resize(ds, xenfb_pub->width, xenfb_pub->height);
+			     xenfb);
+	dpy_resize(ds, xenfb->data.width, xenfb->data.height);
 
 	return 0;
 
diff -r 82c317f263c2 tools/ioemu/hw/xenfb.h
--- a/tools/ioemu/hw/xenfb.h	Tue Aug 14 15:26:57 2007 -0400
+++ b/tools/ioemu/hw/xenfb.h	Tue Aug 14 15:32:58 2007 -0400
@@ -5,21 +5,7 @@
 #include <stdbool.h>
 #include <sys/types.h>
 
-struct xenfb
-{
-	void *pixels;
-
-	int row_stride;
-	int depth;
-	int width;
-	int height;
-	int abs_pointer_wanted;
-	int button_state;
-
-	void *user_data;
-
-	void (*update)(struct xenfb *xenfb, int x, int y, int width, int height);
-};
+struct xenfb;
 
 struct xenfb *xenfb_new(void);
 void xenfb_delete(struct xenfb *xenfb);

-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|