From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [RFC][PATCH v1] xen-fbfront: replace deferred io with buffer queue Date: Mon, 19 Jan 2015 13:25:51 +0000 Message-ID: References: <1421668996-23935-1-git-send-email-sergiy.kibrik@globallogic.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YDCLG-0004iX-Cy for xen-devel@lists.xenproject.org; Mon, 19 Jan 2015 13:26:06 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: xen-devel@lists.xenproject.org, Sergiy Kibrik List-Id: xen-devel@lists.xenproject.org On Mon, 19 Jan 2015, Stefano Stabellini wrote: > 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 > > --- > > > > 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 > > #include > > #include > > @@ -32,20 +26,34 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > #include > > > > +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. Sorry, I realize now that fortunately it is the other way around. It would be nice to see a correspondent patch for xenfb (the backend). Who configures num_bufs initially in your system? > 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 > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >