Konrad Rzeszutek Wilk
2011-Mar-15 14:11 UTC
[Xen-devel] [PATCH] Add grant references for fbfront/kbdfront
This patchset is proposed for 2.6.39.
The patchset description from:
http://lists.colo.xensource.com/archives/html/xen-devel/2011-03/msg00381.html
This series fixes the interface for the fbfront and kbdfront devices,
which were storing MFNs in xenstore rather than creating grant table
entries. To maintain backwards compatibility, a different xenstore key
is used (page-gref instead of page-ref) and the use of grants must be
requested for fbfront (because two levels of page references are
embedded within the shared page). This makes it possible to move a
display server out of dom0 without giving the display domain full access
to other domain''s memory.
Changes since v1:
Updated xenstore key to "feature-grants"
Updated frontend patch has been sent to qemu-devel.
drivers/input/xen-kbdfront.c | 39 +++++++++----
drivers/video/xen-fbfront.c | 126 ++++++++++++++++++++++++++++++++++-------
2 files changed, 132 insertions(+), 33 deletions(-)
Daniel De Graaf (3):
xen-fbfront: Read width/height from backend
xen-kbdfront: Add grant reference for shared page
xen-fbfront: Use grant references when requested
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-15 14:11 UTC
[Xen-devel] [PATCH 1/4] xen-fbfront: Read width/height from backend
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
This allows the backend driver to specify the size of the framebuffer
instead of requiring it to be on the kernel command line.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
---
drivers/video/xen-fbfront.c | 30 ++++++++++++++++++++++++------
1 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index a20218c..56d6061 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -58,7 +58,7 @@ struct xenfb_info {
#define XENFB_DEFAULT_FB_LEN (XENFB_WIDTH * XENFB_HEIGHT * XENFB_DEPTH / 8)
enum { KPARAM_MEM, KPARAM_WIDTH, KPARAM_HEIGHT, KPARAM_CNT };
-static int video[KPARAM_CNT] = { 2, XENFB_WIDTH, XENFB_HEIGHT };
+static int video[KPARAM_CNT] = { 0, 0, 0 };
module_param_array(video, int, NULL, 0);
MODULE_PARM_DESC(video,
"Video memory size in MB, width, height in pixels (default
2,800,600)");
@@ -363,7 +363,7 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
{
struct xenfb_info *info;
struct fb_info *fb_info;
- int fb_size;
+ int fb_size, fb_need;
int val;
int ret;
@@ -375,14 +375,32 @@ static int __devinit 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])
+ if (!video[KPARAM_MEM] || val < video[KPARAM_MEM])
video[KPARAM_MEM] = val;
}
+ /* Take width/height from xenstore if not set on command line */
+ if (!video[KPARAM_WIDTH]) {
+ if (xenbus_scanf(XBT_NIL, dev->otherend, "width",
"%d", &val) == 1)
+ video[KPARAM_WIDTH] = val;
+ else
+ video[KPARAM_WIDTH] = XENFB_WIDTH;
+ }
+ if (!video[KPARAM_HEIGHT]) {
+ if (xenbus_scanf(XBT_NIL, dev->otherend, "height",
"%d", &val) == 1)
+ video[KPARAM_HEIGHT] = val;
+ else
+ video[KPARAM_HEIGHT] = XENFB_HEIGHT;
+ }
+
+ fb_need = video[KPARAM_WIDTH] * video[KPARAM_HEIGHT] * XENFB_DEPTH / 8;
+ if (video[KPARAM_MEM])
+ fb_size = video[KPARAM_MEM] * 1024 * 1024;
+ else
+ fb_size = video[KPARAM_MEM] = fb_need;
+
/* 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 (fb_need > fb_size) {
video[KPARAM_WIDTH] = XENFB_WIDTH;
video[KPARAM_HEIGHT] = XENFB_HEIGHT;
fb_size = XENFB_DEFAULT_FB_LEN;
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-15 14:11 UTC
[Xen-devel] [PATCH 2/4] xen-kbdfront: Add grant reference for shared page
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Without a grant reference, full access to the domain''s memory is
required to use the shared page. Add an additional parameter in
xenstore to allow grant mapping to be used.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
---
drivers/input/xen-kbdfront.c | 39 ++++++++++++++++++++++++++++-----------
1 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
index 7f85a86..a5c064e 100644
--- a/drivers/input/xen-kbdfront.c
+++ b/drivers/input/xen-kbdfront.c
@@ -11,12 +11,6 @@
* more details.
*/
-/*
- * TODO:
- *
- * Switch to grant tables together with xen-fbfront.c.
- */
-
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/kernel.h>
@@ -30,6 +24,8 @@
#include <xen/xen.h>
#include <xen/events.h>
#include <xen/page.h>
+#include <xen/grant_table.h>
+#include <xen/interface/grant_table.h>
#include <xen/interface/io/fbif.h>
#include <xen/interface/io/kbdif.h>
#include <xen/xenbus.h>
@@ -38,6 +34,7 @@ struct xenkbd_info {
struct input_dev *kbd;
struct input_dev *ptr;
struct xenkbd_page *page;
+ int gref;
int irq;
struct xenbus_device *xbdev;
char phys[32];
@@ -122,6 +119,7 @@ static int __devinit xenkbd_probe(struct xenbus_device *dev,
dev_set_drvdata(&dev->dev, info);
info->xbdev = dev;
info->irq = -1;
+ info->gref = -1;
snprintf(info->phys, sizeof(info->phys), "xenbus/%s",
dev->nodename);
info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
@@ -218,15 +216,20 @@ static int xenkbd_connect_backend(struct xenbus_device
*dev,
int ret, evtchn;
struct xenbus_transaction xbt;
+ ret = gnttab_grant_foreign_access(dev->otherend_id,
+ virt_to_mfn(info->page), 0);
+ if (ret < 0)
+ return ret;
+ info->gref = ret;
+
ret = xenbus_alloc_evtchn(dev, &evtchn);
if (ret)
- return ret;
+ goto error_grant;
ret = bind_evtchn_to_irqhandler(evtchn, input_handler,
0, dev->devicetype, info);
if (ret < 0) {
- xenbus_free_evtchn(dev, evtchn);
xenbus_dev_fatal(dev, ret, "bind_evtchn_to_irqhandler");
- return ret;
+ goto error_evtchan;
}
info->irq = ret;
@@ -234,12 +237,15 @@ static int xenkbd_connect_backend(struct xenbus_device
*dev,
ret = xenbus_transaction_start(&xbt);
if (ret) {
xenbus_dev_fatal(dev, ret, "starting transaction");
- return ret;
+ goto error_irqh;
}
ret = xenbus_printf(xbt, dev->nodename, "page-ref",
"%lu",
virt_to_mfn(info->page));
if (ret)
goto error_xenbus;
+ ret = xenbus_printf(xbt, dev->nodename, "page-gref",
"%u", info->gref);
+ if (ret)
+ goto error_xenbus;
ret = xenbus_printf(xbt, dev->nodename, "event-channel",
"%u",
evtchn);
if (ret)
@@ -249,7 +255,7 @@ static int xenkbd_connect_backend(struct xenbus_device *dev,
if (ret == -EAGAIN)
goto again;
xenbus_dev_fatal(dev, ret, "completing transaction");
- return ret;
+ goto error_irqh;
}
xenbus_switch_state(dev, XenbusStateInitialised);
@@ -258,6 +264,14 @@ static int xenkbd_connect_backend(struct xenbus_device
*dev,
error_xenbus:
xenbus_transaction_end(xbt, 1);
xenbus_dev_fatal(dev, ret, "writing xenstore");
+ error_irqh:
+ unbind_from_irqhandler(info->irq, info);
+ info->irq = -1;
+ error_evtchan:
+ xenbus_free_evtchn(dev, evtchn);
+ error_grant:
+ gnttab_end_foreign_access_ref(info->gref, 0);
+ info->gref = -1;
return ret;
}
@@ -266,6 +280,9 @@ static void xenkbd_disconnect_backend(struct xenkbd_info
*info)
if (info->irq >= 0)
unbind_from_irqhandler(info->irq, info);
info->irq = -1;
+ if (info->gref >= 0)
+ gnttab_end_foreign_access_ref(info->gref, 0);
+ info->gref = -1;
}
static void xenkbd_backend_changed(struct xenbus_device *dev,
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-15 14:11 UTC
[Xen-devel] [PATCH 3/4] xen-fbfront: Use grant references when requested
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
The current version of the Xen framebuffer API passes MFNs directly
to the backend driver, which requires the backend to have full access
to this domain''s memory. Add a parameter in xenbus to request the use
of
grant entries instead, which are slightly slower to map but provide
inter-domain isolation.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
---
drivers/video/xen-fbfront.c | 96 +++++++++++++++++++++++++++++++++++-------
1 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index 56d6061..b43c5c9 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -11,13 +11,6 @@
* more details.
*/
-/*
- * TODO:
- *
- * Switch to grant tables when they become capable of dealing with the
- * frame buffer.
- */
-
#include <linux/console.h>
#include <linux/kernel.h>
#include <linux/errno.h>
@@ -32,6 +25,8 @@
#include <xen/xen.h>
#include <xen/events.h>
#include <xen/page.h>
+#include <xen/grant_table.h>
+#include <xen/interface/grant_table.h>
#include <xen/interface/io/fbif.h>
#include <xen/interface/io/protocols.h>
#include <xen/xenbus.h>
@@ -46,6 +41,8 @@ struct xenfb_info {
int irq;
struct xenfb_page *page;
unsigned long *mfns;
+ grant_ref_t *mfn_grefs;
+ int page_gref;
int update_wanted; /* XENFB_TYPE_UPDATE wanted */
int feature_resize; /* XENFB_TYPE_RESIZE ok */
struct xenfb_resize resize; /* protected by resize_lock */
@@ -65,7 +62,7 @@ MODULE_PARM_DESC(video,
static void xenfb_make_preferred_console(void);
static int xenfb_remove(struct xenbus_device *);
-static void xenfb_init_shared_page(struct xenfb_info *, struct fb_info *);
+static void xenfb_init_shared_page(struct xenbus_device *, 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 *);
@@ -412,6 +409,7 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
info->x1 = info->y1 = INT_MAX;
spin_lock_init(&info->dirty_lock);
spin_lock_init(&info->resize_lock);
+ info->page_gref = -1;
info->fb = vmalloc(fb_size);
if (info->fb == NULL)
@@ -474,7 +472,7 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
fb_info->fbdefio = &xenfb_defio;
fb_deferred_io_init(fb_info);
- xenfb_init_shared_page(info, fb_info);
+ xenfb_init_shared_page(dev, info, fb_info);
ret = xenfb_connect_backend(dev, info);
if (ret < 0)
@@ -528,7 +526,7 @@ static int xenfb_resume(struct xenbus_device *dev)
struct xenfb_info *info = dev_get_drvdata(&dev->dev);
xenfb_disconnect_backend(info);
- xenfb_init_shared_page(info, info->fb_info);
+ xenfb_init_shared_page(dev, info, info->fb_info);
return xenfb_connect_backend(dev, info);
}
@@ -543,9 +541,25 @@ static int xenfb_remove(struct xenbus_device *dev)
fb_dealloc_cmap(&info->fb_info->cmap);
framebuffer_release(info->fb_info);
}
+ if (info->page_gref >= 0) {
+ int epd = PAGE_SIZE / sizeof(info->mfns[0]);
+ int pdpages = (info->nr_pages + epd - 1) / epd;
+ int i;
+ gnttab_end_foreign_access_ref(info->page_gref, 0);
+ gnttab_free_grant_reference(info->page_gref);
+ for (i = 0; i < pdpages; i++) {
+ gnttab_end_foreign_access_ref(info->mfn_grefs[i], 1);
+ gnttab_free_grant_reference(info->mfn_grefs[i]);
+ }
+ for (i = 0; i < info->nr_pages; i++) {
+ gnttab_end_foreign_access_ref(info->mfns[i], 1);
+ gnttab_free_grant_reference(info->mfns[i]);
+ }
+ }
free_page((unsigned long)info->page);
vfree(info->mfns);
vfree(info->fb);
+ kfree(info->mfn_grefs);
kfree(info);
return 0;
@@ -556,17 +570,63 @@ static unsigned long vmalloc_to_mfn(void *address)
return pfn_to_mfn(vmalloc_to_pfn(address));
}
-static void xenfb_init_shared_page(struct xenfb_info *info,
+static void xenfb_init_shared_page(struct xenbus_device *dev,
+ struct xenfb_info *info,
struct fb_info *fb_info)
{
- int i;
int epd = PAGE_SIZE / sizeof(info->mfns[0]);
+ int be_id = dev->otherend_id;
+ int i, ref;
+ unsigned long mfn;
+ grant_ref_t gref_head;
+ int pdpages = (info->nr_pages + epd - 1) / epd;
+ int allpages = info->nr_pages + pdpages + 1;
+
+ int grants = 0;
+ xenbus_scanf(XBT_NIL, dev->otherend, "feature-grants",
"%d", &grants);
+
+ if (grants) {
+ int err = gnttab_alloc_grant_references(allpages, &gref_head);
+ info->mfn_grefs = kzalloc(pdpages*sizeof(grant_ref_t), GFP_KERNEL);
+ if (!info->mfn_grefs || err < 0) {
+ info->page_gref = -ENOSPC;
+ if (err >= 0)
+ gnttab_free_grant_references(gref_head);
+ grants = 0;
+ } else {
+ ref = gnttab_claim_grant_reference(&gref_head);
+ mfn = virt_to_mfn(info->page);
+ BUG_ON(ref == -ENOSPC);
+ gnttab_grant_foreign_access_ref(ref, be_id, mfn, 0);
+ info->page_gref = ref;
+ }
+ } else
+ info->page_gref = -ENOENT;
for (i = 0; i < info->nr_pages; i++)
- info->mfns[i] = vmalloc_to_mfn(info->fb + i * PAGE_SIZE);
+ {
+ mfn = vmalloc_to_mfn(info->fb + i * PAGE_SIZE);
+ if (grants) {
+ ref = gnttab_claim_grant_reference(&gref_head);
+ BUG_ON(ref == -ENOSPC);
+ gnttab_grant_foreign_access_ref(ref, be_id, mfn, 1);
+ info->mfns[i] = ref;
+ } else
+ info->mfns[i] = mfn;
+ }
for (i = 0; i * epd < info->nr_pages; i++)
- info->page->pd[i] = vmalloc_to_mfn(&info->mfns[i * epd]);
+ {
+ mfn = vmalloc_to_mfn(&info->mfns[i * epd]);
+ if (grants) {
+ ref = gnttab_claim_grant_reference(&gref_head);
+ BUG_ON(ref == -ENOSPC);
+ gnttab_grant_foreign_access_ref(ref, be_id, mfn, 1);
+ info->mfn_grefs[i] = ref;
+ info->page->pd[i] = ref;
+ } else
+ info->page->pd[i] = mfn;
+ }
info->page->width = fb_info->var.xres;
info->page->height = fb_info->var.yres;
@@ -599,8 +659,12 @@ 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));
+ if (info->page_gref < 0)
+ ret = xenbus_printf(xbt, dev->nodename, "page-ref",
"%lu",
+ virt_to_mfn(info->page));
+ else
+ ret = xenbus_printf(xbt, dev->nodename, "page-gref",
"%u",
+ info->page_gref);
if (ret)
goto error_xenbus;
ret = xenbus_printf(xbt, dev->nodename, "event-channel",
"%u",
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Dmitry Torokhov
2011-Mar-16 05:34 UTC
[Xen-devel] Re: [PATCH 2/4] xen-kbdfront: Add grant reference for shared page
On Tue, Mar 15, 2011 at 10:11:13AM -0400, Konrad Rzeszutek Wilk wrote:> From: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Without a grant reference, full access to the domain''s memory is > required to use the shared page. Add an additional parameter in > xenstore to allow grant mapping to be used. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>You are passing the patch on so it should be Signed-off-by... It is independent of the other 2 so I can pick it up and merge (so that we do not have conflicts with move), right? -- Dmitry _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-16 13:45 UTC
[Xen-devel] Re: [PATCH 2/4] xen-kbdfront: Add grant reference for shared page
On Tue, Mar 15, 2011 at 10:34:26PM -0700, Dmitry Torokhov wrote:> On Tue, Mar 15, 2011 at 10:11:13AM -0400, Konrad Rzeszutek Wilk wrote: > > From: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > > > Without a grant reference, full access to the domain''s memory is > > required to use the shared page. Add an additional parameter in > > xenstore to allow grant mapping to be used. > > > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>> > You are passing the patch on so it should be Signed-off-by...Oh, somehow I thought that I was suppose to change my SOB to ACK since it would now go through your tree.> > It is independent of the other 2 so I can pick it up and merge (so that > we do not have conflicts with move), right?Yes.> > -- > Dmitry_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-16 13:47 UTC
Re: [Xen-devel] [PATCH 2/4] xen-kbdfront: Add grant reference for shared page
On Tue, Mar 15, 2011 at 10:11:13AM -0400, Konrad Rzeszutek Wilk wrote:> From: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Without a grant reference, full access to the domain''s memory is > required to use the shared page. Add an additional parameter in > xenstore to allow grant mapping to be used. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Replace Acked-by by Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> please. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-16 13:47 UTC
Re: [Xen-devel] [PATCH 3/4] xen-fbfront: Use grant references when requested
On Tue, Mar 15, 2011 at 10:11:14AM -0400, Konrad Rzeszutek Wilk wrote:> From: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > The current version of the Xen framebuffer API passes MFNs directly > to the backend driver, which requires the backend to have full access > to this domain''s memory. Add a parameter in xenbus to request the use of > grant entries instead, which are slightly slower to map but provide > inter-domain isolation. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Replace Acked-by by Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> please. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Mar-16 13:47 UTC
Re: [Xen-devel] [PATCH 1/4] xen-fbfront: Read width/height from backend
On Tue, Mar 15, 2011 at 10:11:12AM -0400, Konrad Rzeszutek Wilk wrote:> From: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > This allows the backend driver to specify the size of the framebuffer > instead of requiring it to be on the kernel command line. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Replace Acked-by by Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> please. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel