[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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 <sergiy.kibrik@xxxxxxxxxxxxxxx>
> > ---
> > 
> > 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.
 
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@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> > 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.