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

[Sergiy Kibrik <sergiy.kibrik@xxxxxxxxxxxxxxx>]

hi Stefano, thank you for comments,

On 1/19/2015 3:21 PM, 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.
> 
ok, will do it

> 
>> 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?

yes, it is. E.g. on DRA7xx evaluation board with 800x480 display we
adopted quad-buffering, this means we allocate around 6.8MB overall.
However for traditional double-buffered case it's gonna be around 3.4Mb,
affordable size for graphics use.

> 
> 
>> 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?

no, it was left intentionally as a hint for turning debug messages on.
But if it's redundant I can remove one.

> 
> 
>>  #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.
> 
will do that
> 
>>  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.

lock renamed because it protects the whole xenfb_info object, not just
rectangle attributes. I'll make up separate patch.

> 
> 
>> +    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?
> 

no specific need for vmalloc_32, will correct that

> 
>> +    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?
> 
> 

xenfb_in_event is a union.
I just did it in the way union xenfb_out_event already implemented.

-- 
regards,
Sergey

>> +    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