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