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