thr3ads.net - Xen devel - [Xen-devel] [PATCH v2] Add grant references for fbfront/kbdfront [Mar 2011]

If this information is useful, please help other people find it:
Share via:

Daniel De Graaf

2011-Mar-07 20:11 UTC

[Xen-devel] [PATCH v2] Add grant references for fbfront/kbdfront

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.

[PATCH 1/3] xen-fbfront: Read width/height from backend
[PATCH 2/3] xen-fbfront: Use grant references when requested
[PATCH 3/3] xen-kbdfront: Add grant reference for shared page

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2011-Mar-07 20:11 UTC

head link

[Xen-devel] [PATCH 1/3] xen-fbfront: Read width/height from backend

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>
---
 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.3.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2011-Mar-07 20:11 UTC

head link

[Xen-devel] [PATCH 2/3] xen-fbfront: Use grant references when requested

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>
---
 drivers/video/xen-fbfront.c |   73 +++++++++++++++++++++++++++++++++---------
 1 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index 56d6061..b3107e4 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,7 @@ struct xenfb_info {
 	int			irq;
 	struct xenfb_page	*page;
 	unsigned long 		*mfns;
+	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 +61,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 *);
 
@@ -474,7 +470,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 +524,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);
 }
 
@@ -556,17 +552,58 @@ 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 allpages = info->nr_pages + ((info->nr_pages + epd - 1) / epd) + 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);
+		if (err < 0) {
+			xenbus_dev_fatal(dev, err, "fbdev grant refs");
+			info->page_gref = -ENOSPC;
+		} 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->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 +636,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.3.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2011-Mar-07 20:11 UTC

head link

[Xen-devel] [PATCH 3/3] xen-kbdfront: Add grant reference for shared page

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>
---
 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.3.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Campbell

2011-Mar-08 10:44 UTC

head link

[Xen-devel] Re: [PATCH v2] Add grant references for fbfront/kbdfront

On Mon, 2011-03-07 at 20:11 +0000, Daniel De Graaf
wrote:> 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.
Looks good to me. I presume this was all tested with the existing
backends as well as the separate display server backend?

Under that assumption all 3:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I think it would be good to get the backend patches into the tree as
well, to be used even when running in domain 0. Not just for good form
but because it should help avoid this stuff from bit-rotting etc which
seems like a danger if the only user is your display server.

Ian.
> 
> Changes since v1:
> 	Updated xenstore key to "feature-grants"
> 
> Updated frontend patch has been sent to qemu-devel.
> 
> [PATCH 1/3] xen-fbfront: Read width/height from backend
> [PATCH 2/3] xen-fbfront: Use grant references when requested
> [PATCH 3/3] xen-kbdfront: Add grant reference for shared page


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2011-Mar-08 21:03 UTC

head link

[Xen-devel] Re: [PATCH v2] Add grant references for fbfront/kbdfront

On 03/08/2011 05:44 AM, Ian Campbell wrote:> On Mon, 2011-03-07 at 20:11 +0000, Daniel De Graaf wrote:
>> 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.
> 
> Looks good to me. I presume this was all tested with the existing
> backends as well as the separate display server backend?
> 
> Under that assumption all 3:
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> I think it would be good to get the backend patches into the tree as
> well, to be used even when running in domain 0. Not just for good form
> but because it should help avoid this stuff from bit-rotting etc which
> seems like a danger if the only user is your display server.
> 
> Ian.
> 
The kernel changes have been tested using both qemu-dm (for PV guest; the
emulated device is still used for HVM) and a custom display server (for
both PV and HVM). I have not tested using a qemu-dm on HVM guests (dom0 or
stub-domain), since the default on HVM is to use qemu''s own emulated
graphics card rather than the Xen framebuffer.

Testing on HVM guests requires another patch (below) that I did not submit
because it will cause HVM domains to delay boot for the 300-second device
timeout when vfb entries are placed in xenstore without actually providing
a vfb backend. Unfortunately, this is how XM stores the VNC parameters for
HVM guests so that they are in the same location as for PV guests
(tools/python/xen/xend/XendConfig.py line 942), so providing the xenstore
entries without a backend is a common configuration.

---

diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
index a5c064e..54b48df 100644
--- a/drivers/input/xen-kbdfront.c
+++ b/drivers/input/xen-kbdfront.c
@@ -358,7 +358,7 @@ static struct xenbus_driver xenkbd_driver = {
 
 static int __init xenkbd_init(void)
 {
-	if (!xen_pv_domain())
+	if (!xen_domain())
 		return -ENODEV;
 
 	/* Nothing to do if running in dom0. */
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index b3107e4..03d7ef3 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -748,7 +748,7 @@ static struct xenbus_driver xenfb_driver = {
 
 static int __init xenfb_init(void)
 {
-	if (!xen_pv_domain())
+	if (!xen_domain())
 		return -ENODEV;
 
 	/* Nothing to do if running in dom0. */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Campbell

2011-Mar-09 10:31 UTC

head link

[Xen-devel] Re: [PATCH v2] Add grant references for fbfront/kbdfront

On Tue, 2011-03-08 at 21:03 +0000, Daniel De Graaf
wrote:> On 03/08/2011 05:44 AM, Ian Campbell wrote:
> > On Mon, 2011-03-07 at 20:11 +0000, Daniel De Graaf wrote:
> >> 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.
> > 
> > Looks good to me. I presume this was all tested with the existing
> > backends as well as the separate display server backend?
> > 
> > Under that assumption all 3:
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > I think it would be good to get the backend patches into the tree as
> > well, to be used even when running in domain 0. Not just for good form
> > but because it should help avoid this stuff from bit-rotting etc which
> > seems like a danger if the only user is your display server.
> > 
> > Ian.
> > 
> 
> The kernel changes have been tested using both qemu-dm (for PV guest; the
> emulated device is still used for HVM) and a custom display server (for
> both PV and HVM). I have not tested using a qemu-dm on HVM guests (dom0 or
> stub-domain), since the default on HVM is to use qemu''s own
emulated
> graphics card rather than the Xen framebuffer.
OK, thanks.
> Testing on HVM guests requires another patch (below) that I did not submit
> because it will cause HVM domains to delay boot for the 300-second device
> timeout when vfb entries are placed in xenstore without actually providing
> a vfb backend. Unfortunately, this is how XM stores the VNC parameters for
> HVM guests so that they are in the same location as for PV guests
> (tools/python/xen/xend/XendConfig.py line 942), so providing the xenstore
> entries without a backend is a common configuration.
Hrm, I guess that something we''ll have to figure out some other time.

Ian.
> ---
> 
> diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
> index a5c064e..54b48df 100644
> --- a/drivers/input/xen-kbdfront.c
> +++ b/drivers/input/xen-kbdfront.c
> @@ -358,7 +358,7 @@ static struct xenbus_driver xenkbd_driver = {
>  
>  static int __init xenkbd_init(void)
>  {
> -	if (!xen_pv_domain())
> +	if (!xen_domain())
>  		return -ENODEV;
>  
>  	/* Nothing to do if running in dom0. */
> diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
> index b3107e4..03d7ef3 100644
> --- a/drivers/video/xen-fbfront.c
> +++ b/drivers/video/xen-fbfront.c
> @@ -748,7 +748,7 @@ static struct xenbus_driver xenfb_driver = {
>  
>  static int __init xenfb_init(void)
>  {
> -	if (!xen_pv_domain())
> +	if (!xen_domain())
>  		return -ENODEV;
>  
>  	/* Nothing to do if running in dom0. */


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Konrad Rzeszutek Wilk

2011-Mar-09 21:53 UTC

head link

[Xen-devel] Re: [PATCH 2/3] xen-fbfront: Use grant references when requested

> +	int be_id = dev->otherend_id;
> +	int i, ref;
> +	unsigned long mfn;
> +	grant_ref_t gref_head;
> +	int allpages = info->nr_pages + ((info->nr_pages + epd - 1) / epd)
+ 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);
> +		if (err < 0) {
> +			xenbus_dev_fatal(dev, err, "fbdev grant refs");
> +			info->page_gref = -ENOSPC;
> +		} 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->page->pd[i] = ref;
> +		} else
> +			info->page->pd[i] = mfn;
> +	}
Shouldn''t we in xenfb_remove also cleanup (unclaim and wholesale free
the
grant reference lot?)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2011-Mar-09 22:36 UTC

head link

[Xen-devel] [PATCH 2/3 v2] xen-fbfront: Use grant references when requested

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>
---
 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.3.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Mar 2011 - [PATCH v2] Add grant references for fbfront/kbdfront

[Xen-devel] [PATCH v2] Add grant references for fbfront/kbdfront

[Xen-devel] [PATCH 1/3] xen-fbfront: Read width/height from backend

[Xen-devel] [PATCH 2/3] xen-fbfront: Use grant references when requested

[Xen-devel] [PATCH 3/3] xen-kbdfront: Add grant reference for shared page

[Xen-devel] Re: [PATCH v2] Add grant references for fbfront/kbdfront

[Xen-devel] Re: [PATCH v2] Add grant references for fbfront/kbdfront

[Xen-devel] Re: [PATCH v2] Add grant references for fbfront/kbdfront

[Xen-devel] Re: [PATCH 2/3] xen-fbfront: Use grant references when requested

[Xen-devel] [PATCH 2/3 v2] xen-fbfront: Use grant references when requested