Re: [RFC][PATCH v1] xen-fbfront: replace deferred io with buffer queue

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Mon, 19 Jan 2015, Sergiy Kibrik wrote:
> Use N-buffering instead of old deferred I/O, which is not suitable for high
> frame rates. This includes new event type -- xenfb_in_released,
> to track buffers not being used by backend.
> Also use grants for fb pages, as they allow backend to map them
> to video devices.

Please make the grant change separately in another patch.


> Signed-off-by: Sergiy Kibrik <sergiy.kibrik@globallogic.com>
> ---
> 
> Hi,
> Here I'd like to present changes to xen-fbfront driver as example of
> how it can possibly be modified to support high frame rates.
> With this changes plus modified xenfb backend I was able to achieve 60 FPS on
> ARM based DRA7xx SoC, but this is rather display limitation, not driver itself.
> 
> The idea is to send full resolution shared buffers to backend -- modern UI and
> multimedia applications cause heavy screen redraw. For this purpose driver
> allocates N-times bigger buffer than actual framebuffer resolution, each chunk
> assigned ID (0-N), which is carried within event message to backend.
> As soon as buffers displayed & released on host side, backend sends back release
> event with ID of released chunk, which can be used as framebuffer again.

Isn't it going to use a lot of memory? Could you write down some example
configurations and relative memory consumption?


> For my setup I used modified Xen framebuffer backend [1], DRA7xx SoC and
> OMAP Display Subsystem for video output (omap_vout V4L2 driver).
> 
> [1] http://www-archive.xenproject.org/files/summit_3/Xenpvfb.pdf
> 
>  drivers/video/fbdev/xen-fbfront.c |  344 ++++++++++++++++++++++++++-----------
>  include/xen/interface/io/fbif.h   |    9 +-
>  2 files changed, 250 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
> index 09dc447..a3e40ac 100644
> --- a/drivers/video/fbdev/xen-fbfront.c
> +++ b/drivers/video/fbdev/xen-fbfront.c
> @@ -11,13 +11,7 @@
>   *  more details.
>   */
>  
> -/*
> - * TODO:
> - *
> - * Switch to grant tables when they become capable of dealing with the
> - * frame buffer.
> - */
> -
> +/*#define DEBUG*/

spurious change?


>  #include <linux/console.h>
>  #include <linux/kernel.h>
>  #include <linux/errno.h>
> @@ -32,20 +26,34 @@
>  #include <xen/xen.h>
>  #include <xen/events.h>
>  #include <xen/page.h>
> +#include <xen/grant_table.h>
>  #include <xen/interface/io/fbif.h>
>  #include <xen/interface/io/protocols.h>
>  #include <xen/xenbus.h>
>  #include <xen/platform_pci.h>
>  
> +enum xenfb_buf_state {
> +	XENFB_BUF_RELEASED = 0,
> +	XENFB_BUF_ACTIVE,
> +	XENFB_BUF_NEXT,
> +};

Could you please add a comment to explain what are these states for?
Maybe you could also explain exactly how the new protocol works.


>  struct xenfb_info {
>  	unsigned char		*fb;
> +	int			fb_size;
>  	struct fb_info		*fb_info;
>  	int			x1, y1, x2, y2;	/* dirty rectangle,
>  						   protected by dirty_lock */
> -	spinlock_t		dirty_lock;
> +
> +	spinlock_t		b_lock;

Why are you renaming the spinlock?  If it is really necessary, please do
it in a separate patch. Mixing all the changes together makes the patch
difficult to read.


> +	enum xenfb_buf_state	*buffers;
> +	int			nr_buffers;
>  	int			nr_pages;
> +	struct completion	completion;
> +
>  	int			irq;
>  	struct xenfb_page	*page;
> +	int			ring_ref;
>  	unsigned long 		*mfns;
>  	int			update_wanted; /* XENFB_TYPE_UPDATE wanted */
>  	int			feature_resize; /* XENFB_TYPE_RESIZE ok */
> @@ -70,6 +78,43 @@ static void xenfb_init_shared_page(struct xenfb_info *, struct fb_info *);
>  static int xenfb_connect_backend(struct xenbus_device *, struct xenfb_info *);
>  static void xenfb_disconnect_backend(struct xenfb_info *);
>  
> +static void *rvmalloc(unsigned long size)
> +{
> +	void *mem;
> +	unsigned long adr;
> +
> +	size = PAGE_ALIGN(size);
> +	mem = vmalloc_32(size);

Is vmalloc_32 really needed? Could vmalloc be used?


> +	if (!mem)
> +		return NULL;
> +
> +	memset(mem, 0, size); /* Clear the ram out, no junk to the user */
> +	adr = (unsigned long) mem;
> +	while (size > 0) {
> +		SetPageReserved(vmalloc_to_page((void *)adr));
> +		adr += PAGE_SIZE;
> +		size -= PAGE_SIZE;
> +	}
> +
> +	return mem;
> +}
> +
> +static void rvfree(void *mem, unsigned long size)
> +{
> +	unsigned long adr;
> +
> +	if (!mem)
> +		return;
> +
> +	adr = (unsigned long) mem;
> +	while ((long) size > 0) {
> +		ClearPageReserved(vmalloc_to_page((void *)adr));
> +		adr += PAGE_SIZE;
> +		size -= PAGE_SIZE;
> +	}
> +	vfree(mem);
> +}
> +
>  static void xenfb_send_event(struct xenfb_info *info,
>  			     union xenfb_out_event *event)
>  {
> @@ -147,7 +192,7 @@ static void xenfb_refresh(struct xenfb_info *info,
>  	if (!info->update_wanted)
>  		return;
>  
> -	spin_lock_irqsave(&info->dirty_lock, flags);
> +	spin_lock_irqsave(&info->b_lock, flags);
>  
>  	/* Combine with dirty rectangle: */
>  	if (info->y1 < y1)
> @@ -165,7 +210,7 @@ static void xenfb_refresh(struct xenfb_info *info,
>  		info->x2 = x2;
>  		info->y1 = y1;
>  		info->y2 = y2;
> -		spin_unlock_irqrestore(&info->dirty_lock, flags);
> +		spin_unlock_irqrestore(&info->b_lock, flags);
>  		return;
>  	}
>  
> @@ -173,42 +218,12 @@ static void xenfb_refresh(struct xenfb_info *info,
>  	info->x1 = info->y1 = INT_MAX;
>  	info->x2 = info->y2 = 0;
>  
> -	spin_unlock_irqrestore(&info->dirty_lock, flags);
> +	spin_unlock_irqrestore(&info->b_lock, flags);
>  
>  	if (x1 <= x2 && y1 <= y2)
>  		xenfb_do_update(info, x1, y1, x2 - x1 + 1, y2 - y1 + 1);
>  }
>  
> -static void xenfb_deferred_io(struct fb_info *fb_info,
> -			      struct list_head *pagelist)
> -{
> -	struct xenfb_info *info = fb_info->par;
> -	struct page *page;
> -	unsigned long beg, end;
> -	int y1, y2, miny, maxy;
> -
> -	miny = INT_MAX;
> -	maxy = 0;
> -	list_for_each_entry(page, pagelist, lru) {
> -		beg = page->index << PAGE_SHIFT;
> -		end = beg + PAGE_SIZE - 1;
> -		y1 = beg / fb_info->fix.line_length;
> -		y2 = end / fb_info->fix.line_length;
> -		if (y2 >= fb_info->var.yres)
> -			y2 = fb_info->var.yres - 1;
> -		if (miny > y1)
> -			miny = y1;
> -		if (maxy < y2)
> -			maxy = y2;
> -	}
> -	xenfb_refresh(info, 0, miny, fb_info->var.xres, maxy - miny + 1);
> -}
> -
> -static struct fb_deferred_io xenfb_defio = {
> -	.delay		= HZ / 20,
> -	.deferred_io	= xenfb_deferred_io,
> -};
> -
>  static int xenfb_setcolreg(unsigned regno, unsigned red, unsigned green,
>  			   unsigned blue, unsigned transp,
>  			   struct fb_info *info)
> @@ -275,26 +290,40 @@ static ssize_t xenfb_write(struct fb_info *p, const char __user *buf,
>  	return res;
>  }
>  
> +static int find_released(struct xenfb_info *info)
> +{
> +	int i;
> +	for (i = 0; i < info->nr_buffers; i++)
> +		if (info->buffers[i] == XENFB_BUF_RELEASED)
> +			return i;
> +	return -1;
> +}
> +
>  static int
>  xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  {
>  	struct xenfb_info *xenfb_info;
>  	int required_mem_len;
> +	int buffer;
> +	unsigned long flags;
>  
>  	xenfb_info = info->par;
>  
> -	if (!xenfb_info->feature_resize) {
> -		if (var->xres == video[KPARAM_WIDTH] &&
> -		    var->yres == video[KPARAM_HEIGHT] &&
> -		    var->bits_per_pixel == xenfb_info->page->depth) {
> -			return 0;
> -		}
> +	if (var->xres != video[KPARAM_WIDTH] ||
> +	    var->yres != video[KPARAM_HEIGHT] ||
> +	    var->bits_per_pixel != xenfb_info->page->depth)
>  		return -EINVAL;
> -	}
>  
> -	/* Can't resize past initial width and height */
> -	if (var->xres > video[KPARAM_WIDTH] || var->yres > video[KPARAM_HEIGHT])
> -		return -EINVAL;
> +	if (xenfb_queue_full(xenfb_info))
> +		return -EBUSY;
> +
> +	spin_lock_irqsave(&xenfb_info->b_lock, flags);
> +	buffer = var->yoffset / var->yres;
> +	if (find_released(xenfb_info) == -1) {
> +		spin_unlock_irqrestore(&xenfb_info->b_lock, flags);
> +		return -EBUSY;
> +	}
> +	spin_unlock_irqrestore(&xenfb_info->b_lock, flags);
>  
>  	required_mem_len = var->xres * var->yres * xenfb_info->page->depth / 8;
>  	if (var->bits_per_pixel == xenfb_info->page->depth &&
> @@ -307,25 +336,85 @@ xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  	return -EINVAL;
>  }
>  
> -static int xenfb_set_par(struct fb_info *info)
> +static int xenfb_set_par(struct fb_info *fbi)
>  {
> -	struct xenfb_info *xenfb_info;
> +	struct xenfb_info *info = fbi->par;
> +	struct xenbus_device *xbdev = info->xbdev;
> +	struct fb_var_screeninfo *var = &fbi->var;
> +	int buffer = var->yoffset / var->yres;
>  	unsigned long flags;
>  
> -	xenfb_info = info->par;
> +	BUG_ON(buffer < 0 || buffer >= info->nr_buffers);
> +
> +	xenfb_do_update(info, var->xoffset, var->yoffset,
> +				var->xres, var->yres);
> +	dev_dbg(&xbdev->dev, "sent buf %d\n", buffer);
> +
> +	spin_lock_irqsave(&info->b_lock, flags);
> +	info->buffers[buffer] = XENFB_BUF_NEXT;
> +	buffer = find_released(info);
> +	if (buffer != -1) {
> +		var->yoffset = var->yres * buffer;
> +		dev_dbg(&xbdev->dev, "giving out %d\n", buffer);
> +		spin_unlock_irqrestore(&info->b_lock, flags);
> +		return 0;
> +	}
>  
> -	spin_lock_irqsave(&xenfb_info->resize_lock, flags);
> -	xenfb_info->resize.type = XENFB_TYPE_RESIZE;
> -	xenfb_info->resize.width = info->var.xres;
> -	xenfb_info->resize.height = info->var.yres;
> -	xenfb_info->resize.stride = info->fix.line_length;
> -	xenfb_info->resize.depth = info->var.bits_per_pixel;
> -	xenfb_info->resize.offset = 0;
> -	xenfb_info->resize_dpy = 1;
> -	spin_unlock_irqrestore(&xenfb_info->resize_lock, flags);
> +	/* If no buffers available, e.g. in case of double-buffering,
> +	 * we have to wait for backend to release some. If backend is
> +	 * already dead, we return last buffer and hope
> +	 * client knows what to do
> +	*/
> +	INIT_COMPLETION(info->completion);
> +	spin_unlock_irqrestore(&info->b_lock, flags);
> +
> +	if (wait_for_completion_interruptible_timeout(&info->completion,
> +					msecs_to_jiffies(40)) <= 0) {
> +		dev_dbg(&xbdev->dev, "no buffer after timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	spin_lock_irqsave(&info->b_lock, flags);
> +	buffer = find_released(info);
> +	spin_unlock_irqrestore(&info->b_lock, flags);
> +	BUG_ON(buffer == -1); /* wtf is going on? */
> +	var->yoffset = var->yres * buffer;
> +
> +	dev_dbg(&xbdev->dev, "given %d after wait\n", buffer);
>  	return 0;
>  }
>  
> +static int xenfb_mmap(struct fb_info *fbi,
> +		    struct vm_area_struct *vma)
> +{
> +	struct xenfb_info *info = fbi->par;
> +	unsigned long start = vma->vm_start;
> +	unsigned long size = vma->vm_end - vma->vm_start;
> +	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
> +	unsigned long page, pos;
> +
> +	if (offset + size > fbi->fix.smem_len)
> +		return -EINVAL;
> +
> +	pos = (unsigned long)info->fb + offset;
> +
> +	while (size > 0) {
> +		page = vmalloc_to_pfn((void *)pos);
> +		if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED))
> +			return -EAGAIN;
> +
> +		start += PAGE_SIZE;
> +		pos += PAGE_SIZE;
> +		if (size > PAGE_SIZE)
> +			size -= PAGE_SIZE;
> +		else
> +			size = 0;
> +	}
> +
> +	return 0;
> +
> +}
> +
>  static struct fb_ops xenfb_fb_ops = {
>  	.owner		= THIS_MODULE,
>  	.fb_read	= fb_sys_read,
> @@ -336,26 +425,36 @@ static struct fb_ops xenfb_fb_ops = {
>  	.fb_imageblit	= xenfb_imageblit,
>  	.fb_check_var	= xenfb_check_var,
>  	.fb_set_par     = xenfb_set_par,
> +	.fb_mmap	= xenfb_mmap,
>  };
>  
>  static irqreturn_t xenfb_event_handler(int rq, void *dev_id)
>  {
> -	/*
> -	 * No in events recognized, simply ignore them all.
> -	 * If you need to recognize some, see xen-kbdfront's
> -	 * input_handler() for how to do that.
> -	 */
>  	struct xenfb_info *info = dev_id;
>  	struct xenfb_page *page = info->page;
> +	uint32_t cons, in_prod;
> +	unsigned long flags;
>  
> -	if (page->in_cons != page->in_prod) {
> -		info->page->in_cons = info->page->in_prod;
> -		notify_remote_via_irq(info->irq);
> +	rmb();
> +	in_prod = page->in_prod;
> +	dev_dbg(&info->xbdev->dev, "handle %d in events\n", in_prod);
> +
> +	spin_lock_irqsave(&info->b_lock, flags);
> +	for (cons = page->in_cons; cons != in_prod; cons++) {
> +		union xenfb_in_event *event = &XENFB_IN_RING_REF(page, cons);
> +		if (event->type == XENFB_TYPE_RELEASE) {
> +			int buffer = event->release.buffer;
> +			if (buffer < 0 || buffer >= info->nr_buffers)
> +				continue;
> +			info->buffers[buffer] = XENFB_BUF_RELEASED;
> +			dev_dbg(&info->xbdev->dev, "released %d\n", buffer);
> +			complete(&info->completion);
> +		}
>  	}
> +	page->in_cons = cons;
> +	spin_unlock_irqrestore(&info->b_lock, flags);
>  
> -	/* Flush dirty rectangle: */
> -	xenfb_refresh(info, INT_MAX, INT_MAX, -INT_MAX, -INT_MAX);
> -
> +	mb();
>  	return IRQ_HANDLED;
>  }
>  
> @@ -364,8 +463,6 @@ static int xenfb_probe(struct xenbus_device *dev,
>  {
>  	struct xenfb_info *info;
>  	struct fb_info *fb_info;
> -	int fb_size;
> -	int val;
>  	int ret = 0;
>  
>  	info = kzalloc(sizeof(*info), GFP_KERNEL);
> @@ -375,42 +472,50 @@ static int xenfb_probe(struct xenbus_device *dev,
>  	}
>  
>  	/* Limit kernel param videoram amount to what is in xenstore */
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "videoram", "%d", &val) == 1) {
> -		if (val < video[KPARAM_MEM])
> -			video[KPARAM_MEM] = val;
> -	}
> -
> -	/* If requested res does not fit in available memory, use default */
> -	fb_size = video[KPARAM_MEM] * 1024 * 1024;
> -	if (video[KPARAM_WIDTH] * video[KPARAM_HEIGHT] * XENFB_DEPTH / 8
> -	    > fb_size) {
> +	if (xenbus_scanf(XBT_NIL, dev->otherend,
> +			"num_bufs", "%d", &info->nr_buffers) != 1)
> +		info->nr_buffers = 2;

This doesn't sound like a secure interface: a potentially very
significant and unbound memory allocation in dom0 is caused by a
parameter configured by the guest.

It can be trivial for a malicious guest to take down the system.

> +	if (xenbus_scanf(XBT_NIL, dev->otherend, "size", "%dx%d",
> +			&video[KPARAM_WIDTH], &video[KPARAM_HEIGHT]) != 2) {
>  		video[KPARAM_WIDTH] = XENFB_WIDTH;
>  		video[KPARAM_HEIGHT] = XENFB_HEIGHT;
> -		fb_size = XENFB_DEFAULT_FB_LEN;
>  	}
>  
>  	dev_set_drvdata(&dev->dev, info);
>  	info->xbdev = dev;
>  	info->irq = -1;
>  	info->x1 = info->y1 = INT_MAX;
> -	spin_lock_init(&info->dirty_lock);
> +	spin_lock_init(&info->b_lock);
>  	spin_lock_init(&info->resize_lock);
> +	init_completion(&info->completion);
>  
> -	info->fb = vzalloc(fb_size);
> +	info->fb_size = video[KPARAM_WIDTH] * video[KPARAM_HEIGHT] *
> +			(XENFB_DEPTH / 8) * info->nr_buffers;
> +	info->fb = rvmalloc(info->fb_size);
>  	if (info->fb == NULL)
>  		goto error_nomem;
>  
> -	info->nr_pages = (fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> +	info->nr_pages = (info->fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>  
>  	info->mfns = vmalloc(sizeof(unsigned long) * info->nr_pages);
>  	if (!info->mfns)
>  		goto error_nomem;
> +	info->buffers = kzalloc(sizeof(info->buffers[0]) * info->nr_buffers,
> +								GFP_KERNEL);
> +	if (!info->buffers)
> +		goto error_nomem;
>  
>  	/* set up shared page */
>  	info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
>  	if (!info->page)
>  		goto error_nomem;
>  
> +	memset(info->page->pd, -1, sizeof(info->page->pd));
> +
> +	info->ring_ref = xenbus_grant_ring(dev, virt_to_mfn(info->page));
> +	if (info->ring_ref < 0)
> +		goto error;
> +
>  	/* abusing framebuffer_alloc() to allocate pseudo_palette */
>  	fb_info = framebuffer_alloc(sizeof(u32) * 256, NULL);
>  	if (fb_info == NULL)
> @@ -438,11 +543,13 @@ static int xenfb_probe(struct xenbus_device *dev,
>  
>  	fb_info->fix.visual = FB_VISUAL_TRUECOLOR;
>  	fb_info->fix.line_length = fb_info->var.xres * XENFB_DEPTH / 8;
> -	fb_info->fix.smem_start = 0;
> -	fb_info->fix.smem_len = fb_size;
> +	fb_info->fix.smem_start =
> +			(unsigned long)page_to_phys(vmalloc_to_page(info->fb));
> +	fb_info->fix.smem_len = info->fb_size;
>  	strcpy(fb_info->fix.id, "xen");
>  	fb_info->fix.type = FB_TYPE_PACKED_PIXELS;
>  	fb_info->fix.accel = FB_ACCEL_NONE;
> +	fb_info->fix.ypanstep = fb_info->var.yres;
>  
>  	fb_info->flags = FBINFO_FLAG_DEFAULT | FBINFO_VIRTFB;
>  
> @@ -453,9 +560,6 @@ static int xenfb_probe(struct xenbus_device *dev,
>  		goto error;
>  	}
>  
> -	fb_info->fbdefio = &xenfb_defio;
> -	fb_deferred_io_init(fb_info);
> -
>  	xenfb_init_shared_page(info, fb_info);
>  
>  	ret = xenfb_connect_backend(dev, info);
> @@ -521,6 +625,7 @@ static int xenfb_resume(struct xenbus_device *dev)
>  static int xenfb_remove(struct xenbus_device *dev)
>  {
>  	struct xenfb_info *info = dev_get_drvdata(&dev->dev);
> +	int i;
>  
>  	xenfb_disconnect_backend(info);
>  	if (info->fb_info) {
> @@ -529,9 +634,26 @@ static int xenfb_remove(struct xenbus_device *dev)
>  		fb_dealloc_cmap(&info->fb_info->cmap);
>  		framebuffer_release(info->fb_info);
>  	}
> +
> +	for (i = 0; i < sizeof(info->page->pd); i++)
> +		if (info->page->pd[i] != -1) {
> +			gnttab_end_foreign_access(
> +				info->page->pd[i], 0, 0UL);
> +			info->page->pd[i] = -1;
> +		}
> +
> +	if (info->ring_ref >= 0) {
> +		gnttab_end_foreign_access(info->ring_ref, 0, 0UL);
> +		info->ring_ref = -1;
> +	}
> +
>  	free_page((unsigned long)info->page);
> +
> +	for (i = 0; i < info->nr_pages; i++)
> +		gnttab_end_foreign_access(info->mfns[i], 0, 0UL);
>  	vfree(info->mfns);
> -	vfree(info->fb);
> +	rvfree(info->fb, info->fb_size);
> +	kfree(info->buffers);
>  	kfree(info);
>  
>  	return 0;
> @@ -545,14 +667,32 @@ static unsigned long vmalloc_to_mfn(void *address)
>  static void xenfb_init_shared_page(struct xenfb_info *info,
>  				   struct fb_info *fb_info)
>  {
> -	int i;
> +	int i, ref;
> +	grant_ref_t gref_head;
>  	int epd = PAGE_SIZE / sizeof(info->mfns[0]);
>  
> -	for (i = 0; i < info->nr_pages; i++)
> -		info->mfns[i] = vmalloc_to_mfn(info->fb + i * PAGE_SIZE);
> +	if (gnttab_alloc_grant_references(info->nr_pages +
> +				DIV_ROUND_UP(info->nr_pages, epd),
> +				&gref_head)) {
> +		WARN(1, "failed to allocate %u grants\n", info->nr_pages);
> +		return;
> +	}
>  
> -	for (i = 0; i * epd < info->nr_pages; i++)
> -		info->page->pd[i] = vmalloc_to_mfn(&info->mfns[i * epd]);
> +	for (i = 0; i < info->nr_pages; i++) {
> +		ref = gnttab_claim_grant_reference(&gref_head);
> +		BUG_ON(ref == -ENOSPC);
> +		gnttab_grant_foreign_access_ref(ref, info->xbdev->otherend_id,
> +				vmalloc_to_mfn(info->fb + i * PAGE_SIZE), 0);
> +		info->mfns[i] = ref;
> +	}
> +
> +	for (i = 0; i * epd < info->nr_pages; i++) {
> +		ref = gnttab_claim_grant_reference(&gref_head);
> +		BUG_ON(ref == -ENOSPC);
> +		gnttab_grant_foreign_access_ref(ref, info->xbdev->otherend_id,
> +				vmalloc_to_mfn(&info->mfns[i * epd]), 0);
> +		info->page->pd[i] = ref;
> +	}
>  
>  	info->page->width = fb_info->var.xres;
>  	info->page->height = fb_info->var.yres;
> @@ -585,8 +725,8 @@ static int xenfb_connect_backend(struct xenbus_device *dev,
>  		xenbus_dev_fatal(dev, ret, "starting transaction");
>  		goto unbind_irq;
>  	}
> -	ret = xenbus_printf(xbt, dev->nodename, "page-ref", "%lu",
> -			    virt_to_mfn(info->page));
> +	ret = xenbus_printf(xbt, dev->nodename, "ring-ref", "%u",
> +			    info->ring_ref);
>  	if (ret)
>  		goto error_xenbus;
>  	ret = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
> diff --git a/include/xen/interface/io/fbif.h b/include/xen/interface/io/fbif.h
> index 974a51e..471d867 100644
> --- a/include/xen/interface/io/fbif.h
> +++ b/include/xen/interface/io/fbif.h
> @@ -77,13 +77,20 @@ union xenfb_out_event {
>  
>  /*
>   * Frontends should ignore unknown in events.
> - * No in events currently defined.
>   */
>  
> +#define XENFB_TYPE_RELEASE 4
> +struct xenfb_release {
> +	uint8_t type;	/* XENFB_TYPE_RELEASE */

Isn't the type already stored in struct xenfb_in_event?
Why do you need it here too? Are there two different types?


> +	int32_t buffer;	/* buffer # */
> +};
> +
> +
>  #define XENFB_IN_EVENT_SIZE 40
>  
>  union xenfb_in_event {
>  	uint8_t type;
> +	struct xenfb_release release;
>  	char pad[XENFB_IN_EVENT_SIZE];
>  };
>  
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>