thr3ads.net - Xen devel - [Xen-devel] [PATCH v4] Userspace grant communication [Jan 2011]

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

Daniel De Graaf

2011-Jan-14 15:46 UTC

[Xen-devel] [PATCH v4] Userspace grant communication

Rebased on top of linus''s tree, and fixed a few issues identified in
v3.

[PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open
[PATCH 2/6] xen-gntdev: Remove unneeded structures from grant_map tracking data
[PATCH 3/6] xen-gntdev: Use find_vma rather than iterating our vma list manually
[PATCH 4/6] xen-gntdev: Add reference counting to maps
[PATCH 5/6] xen-gntdev: Support mapping in HVM domains
[PATCH 6/6] xen-gntalloc: Userspace grant allocation driver

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

Daniel De Graaf

2011-Jan-14 15:46 UTC

head link

[Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

Because there is no limitation on how many times a user can open a
given device file, an per-file-description limit on the number of
pages granted offers little to no benefit. Change to a global limit
and remove the ioctl() as the parameter can now be changed via sysfs.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |   52 +++++++++++++++----------------------------------
 1 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 1e31cdc..59e6a51 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -45,15 +45,15 @@ MODULE_AUTHOR("Derek G. Murray
<Derek.Murray@cl.cam.ac.uk>, "
 	      "Gerd Hoffmann <kraxel@redhat.com>");
 MODULE_DESCRIPTION("User-space granted page access driver");
 
-static int limit = 1024;
+static int limit = 1024*1024;
 module_param(limit, int, 0644);
-MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped at
"
-		"once by a gntdev instance");
+MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped by
"
+		"the gntdev device");
+
+static atomic_t pages_mapped = ATOMIC_INIT(0);
 
 struct gntdev_priv {
 	struct list_head maps;
-	uint32_t used;
-	uint32_t limit;
 	/* lock protects maps from concurrent changes */
 	spinlock_t lock;
 	struct mm_struct *mm;
@@ -82,9 +82,8 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
 #ifdef DEBUG
 	struct grant_map *map;
 
-	pr_debug("maps list (priv %p, usage %d/%d)\n",
-	       priv, priv->used, priv->limit);
-
+	pr_debug("%s: maps list (priv %p)\n",
+	       __FUNCTION__, priv);
 	list_for_each_entry(map, &priv->maps, next)
 		pr_debug("  index %2d, count %2d %s\n",
 		       map->index, map->count,
@@ -121,9 +120,6 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv
*priv, int count)
 	add->count = count;
 	add->priv  = priv;
 
-	if (add->count + priv->used > priv->limit)
-		goto err;
-
 	return add;
 
 err:
@@ -154,7 +150,6 @@ static void gntdev_add_map(struct gntdev_priv *priv, struct
grant_map *add)
 	list_add_tail(&add->next, &priv->maps);
 
 done:
-	priv->used += add->count;
 	gntdev_print_maps(priv, "[new]", add->index);
 }
 
@@ -200,7 +195,7 @@ static int gntdev_del_map(struct grant_map *map)
 		if (map->unmap_ops[i].handle)
 			return -EBUSY;
 
-	map->priv->used -= map->count;
+	atomic_sub(map->count, &pages_mapped);
 	list_del(&map->next);
 	return 0;
 }
@@ -386,7 +381,6 @@ static int gntdev_open(struct inode *inode, struct file
*flip)
 
 	INIT_LIST_HEAD(&priv->maps);
 	spin_lock_init(&priv->lock);
-	priv->limit = limit;
 
 	priv->mm = get_task_mm(current);
 	if (!priv->mm) {
@@ -443,19 +437,25 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 	pr_debug("priv %p, add %d\n", priv, op.count);
 	if (unlikely(op.count <= 0))
 		return -EINVAL;
-	if (unlikely(op.count > priv->limit))
-		return -EINVAL;
 
 	err = -ENOMEM;
 	map = gntdev_alloc_map(priv, op.count);
 	if (!map)
 		return err;
+
 	if (copy_from_user(map->grants, &u->refs,
 			   sizeof(map->grants[0]) * op.count) != 0) {
 		gntdev_free_map(map);
 		return err;
 	}
 
+	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
+	{
+		pr_debug("%s: can''t map: over limit\n", __FUNCTION__);
+		gntdev_free_map(map);
+		return err;
+	}
+
 	spin_lock(&priv->lock);
 	gntdev_add_map(priv, map);
 	op.index = map->index << PAGE_SHIFT;
@@ -518,23 +518,6 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
gntdev_priv *priv,
 	return 0;
 }
 
-static long gntdev_ioctl_set_max_grants(struct gntdev_priv *priv,
-					struct ioctl_gntdev_set_max_grants __user *u)
-{
-	struct ioctl_gntdev_set_max_grants op;
-
-	if (copy_from_user(&op, u, sizeof(op)) != 0)
-		return -EFAULT;
-	pr_debug("priv %p, limit %d\n", priv, op.count);
-	if (op.count > limit)
-		return -E2BIG;
-
-	spin_lock(&priv->lock);
-	priv->limit = op.count;
-	spin_unlock(&priv->lock);
-	return 0;
-}
-
 static long gntdev_ioctl(struct file *flip,
 			 unsigned int cmd, unsigned long arg)
 {
@@ -551,9 +534,6 @@ static long gntdev_ioctl(struct file *flip,
 	case IOCTL_GNTDEV_GET_OFFSET_FOR_VADDR:
 		return gntdev_ioctl_get_offset_for_vaddr(priv, ptr);
 
-	case IOCTL_GNTDEV_SET_MAX_GRANTS:
-		return gntdev_ioctl_set_max_grants(priv, ptr);
-
 	default:
 		pr_debug("priv %p, unknown cmd %x\n", priv, cmd);
 		return -ENOIOCTLCMD;
-- 
1.7.3.4


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

Daniel De Graaf

2011-Jan-14 15:46 UTC

head link

[Xen-devel] [PATCH 2/6] xen-gntdev: Remove unneeded structures from grant_map tracking data

The entire hypercall argument list isn''t required; only selected
fields from the hypercall need to be tracked between the ioctl, map,
and unmap operations.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |  225 ++++++++++++++++++++++++++++++--------------------
 1 files changed, 136 insertions(+), 89 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 59e6a51..d0802b5 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -60,18 +60,24 @@ struct gntdev_priv {
 	struct mmu_notifier mn;
 };
 
+struct granted_page {
+	u64 pte_maddr;
+	union {
+		struct ioctl_gntdev_grant_ref target;
+		grant_handle_t handle;
+	};
+};
+
 struct grant_map {
 	struct list_head next;
 	struct gntdev_priv *priv;
 	struct vm_area_struct *vma;
 	int index;
 	int count;
-	int flags;
-	int is_mapped;
-	struct ioctl_gntdev_grant_ref *grants;
-	struct gnttab_map_grant_ref   *map_ops;
-	struct gnttab_unmap_grant_ref *unmap_ops;
+	int is_mapped:1;
+	int is_ro:1;
 	struct page **pages;
+	struct granted_page pginfo[0];
 };
 
 /* ------------------------------------------------------------------ */
@@ -91,24 +97,19 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
 #endif
 }
 
-static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count)
+static struct grant_map *gntdev_alloc_map(int count,
+		struct ioctl_gntdev_grant_ref* grants)
 {
 	struct grant_map *add;
 	int i;
 
-	add = kzalloc(sizeof(struct grant_map), GFP_KERNEL);
-	if (NULL == add)
+	add = kzalloc(sizeof(*add) + sizeof(add->pginfo[0])*count, GFP_KERNEL);
+	if (!add)
 		return NULL;
 
-	add->grants    = kzalloc(sizeof(add->grants[0])    * count, GFP_KERNEL);
-	add->map_ops   = kzalloc(sizeof(add->map_ops[0])   * count, GFP_KERNEL);
-	add->unmap_ops = kzalloc(sizeof(add->unmap_ops[0]) * count, GFP_KERNEL);
-	add->pages     = kzalloc(sizeof(add->pages[0])     * count, GFP_KERNEL);
-	if (NULL == add->grants    ||
-	    NULL == add->map_ops   ||
-	    NULL == add->unmap_ops ||
-	    NULL == add->pages)
-		goto err;
+	add->pages = kzalloc(sizeof(add->pages[0])*count, GFP_KERNEL);
+	if (!add->pages)
+		goto err_nopages;
 
 	for (i = 0; i < count; i++) {
 		add->pages[i] = alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
@@ -118,20 +119,18 @@ static struct grant_map *gntdev_alloc_map(struct
gntdev_priv *priv, int count)
 
 	add->index = 0;
 	add->count = count;
-	add->priv  = priv;
+	for(i = 0; i < count; i++)
+		add->pginfo[i].target = grants[i];
 
 	return add;
 
 err:
-	if (add->pages)
-		for (i = 0; i < count; i++) {
-			if (add->pages[i])
-				__free_page(add->pages[i]);
-		}
+	for (i = 0; i < count; i++) {
+		if (add->pages[i])
+			__free_page(add->pages[i]);
+	}
 	kfree(add->pages);
-	kfree(add->grants);
-	kfree(add->map_ops);
-	kfree(add->unmap_ops);
+err_nopages:
 	kfree(add);
 	return NULL;
 }
@@ -140,6 +139,7 @@ static void gntdev_add_map(struct gntdev_priv *priv, struct
grant_map *add)
 {
 	struct grant_map *map;
 
+	spin_lock(&priv->lock);
 	list_for_each_entry(map, &priv->maps, next) {
 		if (add->index + add->count < map->index) {
 			list_add_tail(&add->next, &map->next);
@@ -150,7 +150,9 @@ static void gntdev_add_map(struct gntdev_priv *priv, struct
grant_map *add)
 	list_add_tail(&add->next, &priv->maps);
 
 done:
+	add->priv = priv;
 	gntdev_print_maps(priv, "[new]", add->index);
+	spin_unlock(&priv->lock);
 }
 
 static struct grant_map *gntdev_find_map_index(struct gntdev_priv *priv,
@@ -191,9 +193,10 @@ static int gntdev_del_map(struct grant_map *map)
 
 	if (map->vma)
 		return -EBUSY;
-	for (i = 0; i < map->count; i++)
-		if (map->unmap_ops[i].handle)
-			return -EBUSY;
+	if (map->is_mapped)
+		for (i = 0; i < map->count; i++)
+			if (map->pginfo[i].handle)
+				return -EBUSY;
 
 	atomic_sub(map->count, &pages_mapped);
 	list_del(&map->next);
@@ -207,15 +210,11 @@ static void gntdev_free_map(struct grant_map *map)
 	if (!map)
 		return;
 
-	if (map->pages)
-		for (i = 0; i < map->count; i++) {
-			if (map->pages[i])
-				__free_page(map->pages[i]);
-		}
+	for (i = 0; i < map->count; i++) {
+		if (map->pages[i])
+			__free_page(map->pages[i]);
+	}
 	kfree(map->pages);
-	kfree(map->grants);
-	kfree(map->map_ops);
-	kfree(map->unmap_ops);
 	kfree(map);
 }
 
@@ -229,50 +228,96 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
 	u64 pte_maddr;
 
 	BUG_ON(pgnr >= map->count);
+
 	pte_maddr = arbitrary_virt_to_machine(pte).maddr;
+	map->pginfo[pgnr].pte_maddr = pte_maddr;
 
-	gnttab_set_map_op(&map->map_ops[pgnr], pte_maddr,
-			  GNTMAP_contains_pte | map->flags,
-			  map->grants[pgnr].ref,
-			  map->grants[pgnr].domid);
-	gnttab_set_unmap_op(&map->unmap_ops[pgnr], pte_maddr,
-			    GNTMAP_contains_pte | map->flags,
-			    0 /* handle */);
 	return 0;
 }
 
 static int map_grant_pages(struct grant_map *map)
 {
-	int i, err = 0;
+	int i, flags, err = 0;
+	struct gnttab_map_grant_ref* map_ops = NULL;
 
-	pr_debug("map %d+%d\n", map->index, map->count);
-	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
-	if (err)
-		return err;
+	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
+	if (map->is_ro)
+		flags |= GNTMAP_readonly;
+
+	err = -ENOMEM;
+	map_ops = kzalloc(sizeof(map_ops[0]) * map->count, GFP_TEMPORARY);
+	if (!map_ops)
+		goto out;
+
+	for(i=0; i < map->count; i++) {
+		gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr, flags,
+				  map->pginfo[i].target.ref,
+				  map->pginfo[i].target.domid);
+	}
+	pr_debug("%s: map %d+%d\n", __FUNCTION__, map->index,
map->count);
+
+	err = gnttab_map_refs(map_ops, map->pages, map->count);
+
+	if (WARN_ON(err))
+		goto out;
 
 	for (i = 0; i < map->count; i++) {
-		if (map->map_ops[i].status)
+		if (map_ops[i].status) {
+			__free_page(map->pages[i]);
+			map->pages[i] = NULL;
 			err = -EINVAL;
-		map->unmap_ops[i].handle = map->map_ops[i].handle;
+		} else {
+			map->pginfo[i].handle = map_ops[i].handle;
+		}
 	}
+
+out:
+	kfree(map_ops);
 	return err;
 }
 
-static int unmap_grant_pages(struct grant_map *map, int offset, int pages)
+static void unmap_grant_pages(struct grant_map *map, int offset, int pages)
 {
-	int i, err = 0;
+	int i, flags, err = 0;
+	struct gnttab_unmap_grant_ref *unmap_ops;
+	struct gnttab_unmap_grant_ref unmap_single;
+
+	if (pages > 1) {
+		unmap_ops = kzalloc(sizeof(unmap_ops[0]) * pages,
+		                    GFP_TEMPORARY);
+		if (unlikely(!unmap_ops)) {
+			for(i=0; i < pages; i++)
+				unmap_grant_pages(map, offset + i, 1);
+			return;
+		}
+	} else {
+		unmap_ops = &unmap_single;
+	}
+
+	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
+	if (map->is_ro)
+		flags |= GNTMAP_readonly;
 
+	for(i=0; i < pages; i++)
+		gnttab_set_unmap_op(&unmap_ops[i],
+		                    map->pginfo[offset+i].pte_maddr, flags,
+		                    map->pginfo[offset+i].handle);
 	pr_debug("map %d+%d [%d+%d]\n", map->index, map->count,
offset, pages);
-	err = gnttab_unmap_refs(map->unmap_ops + offset, map->pages, pages);
+
+	err = gnttab_unmap_refs(unmap_ops, map->pages + offset, pages);
+
 	if (err)
-		return err;
+		goto out;
 
 	for (i = 0; i < pages; i++) {
-		if (map->unmap_ops[offset+i].status)
-			err = -EINVAL;
-		map->unmap_ops[offset+i].handle = 0;
+		WARN_ON(unmap_ops[i].status);
+		__free_page(map->pages[offset+i]);
+		map->pages[offset+i] = NULL;
+		map->pginfo[offset+i].handle = 0;
 	}
-	return err;
+out:
+	if (unmap_ops != &unmap_single)
+		kfree(unmap_ops);
 }
 
 /* ------------------------------------------------------------------ */
@@ -309,7 +354,6 @@ static void mn_invl_range_start(struct mmu_notifier *mn,
 	struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn);
 	struct grant_map *map;
 	unsigned long mstart, mend;
-	int err;
 
 	spin_lock(&priv->lock);
 	list_for_each_entry(map, &priv->maps, next) {
@@ -327,10 +371,9 @@ static void mn_invl_range_start(struct mmu_notifier *mn,
 				map->index, map->count,
 				map->vma->vm_start, map->vma->vm_end,
 				start, end, mstart, mend);
-		err = unmap_grant_pages(map,
-					(mstart - map->vma->vm_start) >> PAGE_SHIFT,
-					(mend - mstart) >> PAGE_SHIFT);
-		WARN_ON(err);
+		unmap_grant_pages(map,
+				  (mstart - map->vma->vm_start) >> PAGE_SHIFT,
+				  (mend - mstart) >> PAGE_SHIFT);
 	}
 	spin_unlock(&priv->lock);
 }
@@ -347,7 +390,6 @@ static void mn_release(struct mmu_notifier *mn,
 {
 	struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn);
 	struct grant_map *map;
-	int err;
 
 	spin_lock(&priv->lock);
 	list_for_each_entry(map, &priv->maps, next) {
@@ -356,8 +398,7 @@ static void mn_release(struct mmu_notifier *mn,
 		pr_debug("map %d+%d (%lx %lx)\n",
 				map->index, map->count,
 				map->vma->vm_start, map->vma->vm_end);
-		err = unmap_grant_pages(map, /* offset */ 0, map->count);
-		WARN_ON(err);
+		unmap_grant_pages(map, /* offset */ 0, map->count);
 	}
 	spin_unlock(&priv->lock);
 }
@@ -430,6 +471,7 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 {
 	struct ioctl_gntdev_map_grant_ref op;
 	struct grant_map *map;
+	struct ioctl_gntdev_grant_ref* grants;
 	int err;
 
 	if (copy_from_user(&op, u, sizeof(op)) != 0)
@@ -438,37 +480,44 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 	if (unlikely(op.count <= 0))
 		return -EINVAL;
 
-	err = -ENOMEM;
-	map = gntdev_alloc_map(priv, op.count);
-	if (!map)
-		return err;
+	grants = kmalloc(sizeof(grants[0]) * op.count, GFP_TEMPORARY);
+	if (!grants)
+		return -ENOMEM;
 
-	if (copy_from_user(map->grants, &u->refs,
-			   sizeof(map->grants[0]) * op.count) != 0) {
-		gntdev_free_map(map);
-		return err;
+	if (copy_from_user(grants, u->refs, sizeof(grants[0]) * op.count)) {
+		err = -EFAULT;
+		goto out_free;
 	}
 
+	err = -ENOMEM;
+	map = gntdev_alloc_map(op.count, grants);
+	if (!map)
+		goto out_free;
+
 	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
 	{
 		pr_debug("%s: can''t map: over limit\n", __FUNCTION__);
-		gntdev_free_map(map);
-		return err;
+		goto out_free_map;
 	}
 
-	spin_lock(&priv->lock);
 	gntdev_add_map(priv, map);
 	op.index = map->index << PAGE_SHIFT;
-	spin_unlock(&priv->lock);
 
-	if (copy_to_user(u, &op, sizeof(op)) != 0) {
-		spin_lock(&priv->lock);
-		gntdev_del_map(map);
-		spin_unlock(&priv->lock);
-		gntdev_free_map(map);
-		return err;
+	if (copy_to_user(u, &op, sizeof(op))) {
+		err = -EFAULT;
+		goto out_remove;
 	}
-	return 0;
+	err = 0;
+out_free:
+	kfree(grants);
+	return err;
+out_remove:
+	spin_lock(&priv->lock);
+	gntdev_del_map(map);
+	spin_unlock(&priv->lock);
+out_free_map:
+	gntdev_free_map(map);
+	goto out_free;
 }
 
 static long gntdev_ioctl_unmap_grant_ref(struct gntdev_priv *priv,
@@ -574,9 +623,7 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	vma->vm_private_data = map;
 	map->vma = vma;
 
-	map->flags = GNTMAP_host_map | GNTMAP_application_map;
-	if (!(vma->vm_flags & VM_WRITE))
-		map->flags |= GNTMAP_readonly;
+	map->is_ro = !(vma->vm_flags & VM_WRITE);
 
 	spin_unlock(&priv->lock);
 
-- 
1.7.3.4


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

Daniel De Graaf

2011-Jan-14 15:46 UTC

head link

[Xen-devel] [PATCH 3/6] xen-gntdev: Use find_vma rather than iterating our vma list manually

This should be faster if many mappings exist, and also removes
the only user of map->vma not related to PTE modification.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |   32 ++++++++------------------------
 1 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index d0802b5..4f7eb3a 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -170,23 +170,6 @@ static struct grant_map *gntdev_find_map_index(struct
gntdev_priv *priv,
 	return NULL;
 }
 
-static struct grant_map *gntdev_find_map_vaddr(struct gntdev_priv *priv,
-					       unsigned long vaddr)
-{
-	struct grant_map *map;
-
-	list_for_each_entry(map, &priv->maps, next) {
-		if (!map->vma)
-			continue;
-		if (vaddr < map->vma->vm_start)
-			continue;
-		if (vaddr >= map->vma->vm_end)
-			continue;
-		return map;
-	}
-	return NULL;
-}
-
 static int gntdev_del_map(struct grant_map *map)
 {
 	int i;
@@ -545,22 +528,23 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
gntdev_priv *priv,
 					      struct ioctl_gntdev_get_offset_for_vaddr __user *u)
 {
 	struct ioctl_gntdev_get_offset_for_vaddr op;
+	struct vm_area_struct *vma;
 	struct grant_map *map;
 
 	if (copy_from_user(&op, u, sizeof(op)) != 0)
 		return -EFAULT;
 	pr_debug("priv %p, offset for vaddr %lx\n", priv, (unsigned
long)op.vaddr);
 
-	spin_lock(&priv->lock);
-	map = gntdev_find_map_vaddr(priv, op.vaddr);
-	if (map == NULL ||
-	    map->vma->vm_start != op.vaddr) {
-		spin_unlock(&priv->lock);
+	vma = find_vma(current->mm, op.vaddr);
+	if (!vma || vma->vm_ops != &gntdev_vmops)
 		return -EINVAL;
-	}
+
+	map = vma->vm_private_data;
+	if (!map)
+		return -EINVAL;
+
 	op.offset = map->index << PAGE_SHIFT;
 	op.count = map->count;
-	spin_unlock(&priv->lock);
 
 	if (copy_to_user(u, &op, sizeof(op)) != 0)
 		return -EFAULT;
-- 
1.7.3.4


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

Daniel De Graaf

2011-Jan-14 15:46 UTC

head link

[Xen-devel] [PATCH 4/6] xen-gntdev: Add reference counting to maps

This allows userspace to perform mmap() on the gntdev device and then
immediately close the filehandle or remove the mapping using the
remove ioctl, with the mapped area remaining valid until unmapped.
This also fixes an infinite loop when a gntdev device is closed
without first unmapping all areas.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |   56 +++++++++++++++++++------------------------------
 1 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 4f7eb3a..8a12857 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -70,16 +70,18 @@ struct granted_page {
 
 struct grant_map {
 	struct list_head next;
-	struct gntdev_priv *priv;
 	struct vm_area_struct *vma;
 	int index;
 	int count;
+	atomic_t users;
 	int is_mapped:1;
 	int is_ro:1;
 	struct page **pages;
 	struct granted_page pginfo[0];
 };
 
+static void unmap_grant_pages(struct grant_map *map, int offset, int pages);
+
 /* ------------------------------------------------------------------ */
 
 static void gntdev_print_maps(struct gntdev_priv *priv,
@@ -118,6 +120,7 @@ static struct grant_map *gntdev_alloc_map(int count,
 	}
 
 	add->index = 0;
+	atomic_set(&add->users, 1);
 	add->count = count;
 	for(i = 0; i < count; i++)
 		add->pginfo[i].target = grants[i];
@@ -150,7 +153,6 @@ static void gntdev_add_map(struct gntdev_priv *priv, struct
grant_map *add)
 	list_add_tail(&add->next, &priv->maps);
 
 done:
-	add->priv = priv;
 	gntdev_print_maps(priv, "[new]", add->index);
 	spin_unlock(&priv->lock);
 }
@@ -170,29 +172,18 @@ static struct grant_map *gntdev_find_map_index(struct
gntdev_priv *priv,
 	return NULL;
 }
 
-static int gntdev_del_map(struct grant_map *map)
-{
-	int i;
-
-	if (map->vma)
-		return -EBUSY;
-	if (map->is_mapped)
-		for (i = 0; i < map->count; i++)
-			if (map->pginfo[i].handle)
-				return -EBUSY;
-
-	atomic_sub(map->count, &pages_mapped);
-	list_del(&map->next);
-	return 0;
-}
-
-static void gntdev_free_map(struct grant_map *map)
+static void gntdev_put_map(struct grant_map *map)
 {
 	int i;
 
 	if (!map)
 		return;
 
+	if (!atomic_dec_and_test(&map->users))
+		return;
+
+	atomic_sub(map->count, &pages_mapped);
+
 	for (i = 0; i < map->count; i++) {
 		if (map->pages[i])
 			__free_page(map->pages[i]);
@@ -313,6 +304,7 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
 	map->is_mapped = 0;
 	map->vma = NULL;
 	vma->vm_private_data = NULL;
+	gntdev_put_map(map);
 }
 
 static int gntdev_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
@@ -430,17 +422,14 @@ static int gntdev_release(struct inode *inode, struct file
*flip)
 {
 	struct gntdev_priv *priv = flip->private_data;
 	struct grant_map *map;
-	int err;
 
 	pr_debug("priv %p\n", priv);
 
 	spin_lock(&priv->lock);
 	while (!list_empty(&priv->maps)) {
 		map = list_entry(priv->maps.next, struct grant_map, next);
-		err = gntdev_del_map(map);
-		if (WARN_ON(err))
-			gntdev_free_map(map);
-
+		list_del(&map->next);
+		gntdev_put_map(map);
 	}
 	spin_unlock(&priv->lock);
 
@@ -488,18 +477,14 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 
 	if (copy_to_user(u, &op, sizeof(op))) {
 		err = -EFAULT;
-		goto out_remove;
+		goto out_free;
 	}
 	err = 0;
 out_free:
 	kfree(grants);
 	return err;
-out_remove:
-	spin_lock(&priv->lock);
-	gntdev_del_map(map);
-	spin_unlock(&priv->lock);
 out_free_map:
-	gntdev_free_map(map);
+	gntdev_put_map(map);
 	goto out_free;
 }
 
@@ -516,11 +501,12 @@ static long gntdev_ioctl_unmap_grant_ref(struct
gntdev_priv *priv,
 
 	spin_lock(&priv->lock);
 	map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count);
-	if (map)
-		err = gntdev_del_map(map);
+	if (map) {
+		list_del(&map->next);
+		gntdev_put_map(map);
+		err = 0;
+	}
 	spin_unlock(&priv->lock);
-	if (!err)
-		gntdev_free_map(map);
 	return err;
 }
 
@@ -600,6 +586,8 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 		goto unlock_out;
 	}
 
+	atomic_inc(&map->users);
+
 	vma->vm_ops = &gntdev_vmops;
 
 	vma->vm_flags |= VM_RESERVED|VM_DONTCOPY|VM_DONTEXPAND|VM_PFNMAP;
-- 
1.7.3.4


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

Daniel De Graaf

2011-Jan-14 15:46 UTC

head link

[Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

HVM does not allow direct PTE modification, so instead we request
that Xen change its internal p2m mappings on the allocated pages and
map the memory into userspace normally.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c      |  117 +++++++++++++++++++++++++++++++--------------
 drivers/xen/grant-table.c |    6 ++
 2 files changed, 87 insertions(+), 36 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 8a12857..1931657 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -32,6 +32,7 @@
 #include <linux/sched.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
+#include <linux/highmem.h>
 
 #include <xen/xen.h>
 #include <xen/grant_table.h>
@@ -52,6 +53,8 @@ MODULE_PARM_DESC(limit, "Maximum number of grants that
may be mapped by "
 
 static atomic_t pages_mapped = ATOMIC_INIT(0);
 
+static int use_ptemod = 0;
+
 struct gntdev_priv {
 	struct list_head maps;
 	/* lock protects maps from concurrent changes */
@@ -184,6 +187,9 @@ static void gntdev_put_map(struct grant_map *map)
 
 	atomic_sub(map->count, &pages_mapped);
 
+	if (!use_ptemod)
+		unmap_grant_pages(map, 0, map->count);
+
 	for (i = 0; i < map->count; i++) {
 		if (map->pages[i])
 			__free_page(map->pages[i]);
@@ -212,9 +218,12 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
 static int map_grant_pages(struct grant_map *map)
 {
 	int i, flags, err = 0;
+	phys_addr_t addr;
 	struct gnttab_map_grant_ref* map_ops = NULL;
 
-	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
+	flags = GNTMAP_host_map;
+	if (use_ptemod)
+		flags |= GNTMAP_application_map | GNTMAP_contains_pte;
 	if (map->is_ro)
 		flags |= GNTMAP_readonly;
 
@@ -224,7 +233,11 @@ static int map_grant_pages(struct grant_map *map)
 		goto out;
 
 	for(i=0; i < map->count; i++) {
-		gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr, flags,
+		if (use_ptemod)
+			addr = map->pginfo[i].pte_maddr;
+		else
+			addr = (phys_addr_t)pfn_to_kaddr(page_to_pfn(map->pages[i]));
+		gnttab_set_map_op(&map_ops[i], addr, flags,
 				  map->pginfo[i].target.ref,
 				  map->pginfo[i].target.domid);
 	}
@@ -255,6 +268,7 @@ static void unmap_grant_pages(struct grant_map *map, int
offset, int pages)
 	int i, flags, err = 0;
 	struct gnttab_unmap_grant_ref *unmap_ops;
 	struct gnttab_unmap_grant_ref unmap_single;
+	phys_addr_t addr;
 
 	if (pages > 1) {
 		unmap_ops = kzalloc(sizeof(unmap_ops[0]) * pages,
@@ -268,14 +282,23 @@ static void unmap_grant_pages(struct grant_map *map, int
offset, int pages)
 		unmap_ops = &unmap_single;
 	}
 
-	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
+	flags = GNTMAP_host_map;
+	if (use_ptemod)
+		flags |= GNTMAP_application_map | GNTMAP_contains_pte;
 	if (map->is_ro)
 		flags |= GNTMAP_readonly;
 
-	for(i=0; i < pages; i++)
-		gnttab_set_unmap_op(&unmap_ops[i],
-		                    map->pginfo[offset+i].pte_maddr, flags,
+	for(i=0; i < pages; i++) {
+		if (WARN_ON(!map->pages[i]))
+			continue;
+		if (use_ptemod)
+			addr = map->pginfo[i].pte_maddr;
+		else
+			addr = (phys_addr_t)pfn_to_kaddr(page_to_pfn(map->pages[i]));
+		gnttab_set_unmap_op(&unmap_ops[i], addr, flags,
 		                    map->pginfo[offset+i].handle);
+	}
+
 	pr_debug("map %d+%d [%d+%d]\n", map->index, map->count,
offset, pages);
 
 	err = gnttab_unmap_refs(unmap_ops, map->pages + offset, pages);
@@ -284,8 +307,25 @@ static void unmap_grant_pages(struct grant_map *map, int
offset, int pages)
 		goto out;
 
 	for (i = 0; i < pages; i++) {
+		uint32_t check, *tmp;
 		WARN_ON(unmap_ops[i].status);
-		__free_page(map->pages[offset+i]);
+		if (!map->pages[i])
+			continue;
+		/* XXX When unmapping, Xen will sometimes end up mapping the GFN
+		 * to an invalid MFN. In this case, writes will be discarded and
+		 * reads will return all 0xFF bytes. Leak these unusable GFNs
+		 * until a way to restore them is found.
+		 */
+		tmp = kmap(map->pages[i]);
+		tmp[0] = 0xdeaddead;
+		mb();
+		check = tmp[0];
+		kunmap(map->pages[i]);
+		if (check == 0xdeaddead)
+			__free_page(map->pages[i]);
+		else
+			pr_debug("Discard page %d=%ld\n", i,
+				 page_to_pfn(map->pages[i]));
 		map->pages[offset+i] = NULL;
 		map->pginfo[offset+i].handle = 0;
 	}
@@ -307,17 +347,8 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
 	gntdev_put_map(map);
 }
 
-static int gntdev_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
-{
-	pr_debug("vaddr %p, pgoff %ld (shouldn''t happen)\n",
-			vmf->virtual_address, vmf->pgoff);
-	vmf->flags = VM_FAULT_ERROR;
-	return 0;
-}
-
 static struct vm_operations_struct gntdev_vmops = {
 	.close = gntdev_vma_close,
-	.fault = gntdev_vma_fault,
 };
 
 /* ------------------------------------------------------------------ */
@@ -398,14 +429,16 @@ static int gntdev_open(struct inode *inode, struct file
*flip)
 	INIT_LIST_HEAD(&priv->maps);
 	spin_lock_init(&priv->lock);
 
-	priv->mm = get_task_mm(current);
-	if (!priv->mm) {
-		kfree(priv);
-		return -ENOMEM;
+	if (use_ptemod) {
+		priv->mm = get_task_mm(current);
+		if (!priv->mm) {
+			kfree(priv);
+			return -ENOMEM;
+		}
+		priv->mn.ops = &gntdev_mmu_ops;
+		ret = mmu_notifier_register(&priv->mn, priv->mm);
+		mmput(priv->mm);
 	}
-	priv->mn.ops = &gntdev_mmu_ops;
-	ret = mmu_notifier_register(&priv->mn, priv->mm);
-	mmput(priv->mm);
 
 	if (ret) {
 		kfree(priv);
@@ -433,7 +466,8 @@ static int gntdev_release(struct inode *inode, struct file
*flip)
 	}
 	spin_unlock(&priv->lock);
 
-	mmu_notifier_unregister(&priv->mn, priv->mm);
+	if (use_ptemod)
+		mmu_notifier_unregister(&priv->mn, priv->mm);
 	kfree(priv);
 	return 0;
 }
@@ -567,7 +601,7 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	int index = vma->vm_pgoff;
 	int count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
 	struct grant_map *map;
-	int err = -EINVAL;
+	int i, err = -EINVAL;
 
 	if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags &
VM_SHARED))
 		return -EINVAL;
@@ -579,12 +613,10 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	map = gntdev_find_map_index(priv, index, count);
 	if (!map)
 		goto unlock_out;
-	if (map->vma)
+	if (use_ptemod && map->vma)
 		goto unlock_out;
-	if (priv->mm != vma->vm_mm) {
+	if (use_ptemod && priv->mm != vma->vm_mm)
 		printk(KERN_WARNING "Huh? Other mm?\n");
-		goto unlock_out;
-	}
 
 	atomic_inc(&map->users);
 
@@ -593,18 +625,20 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	vma->vm_flags |= VM_RESERVED|VM_DONTCOPY|VM_DONTEXPAND|VM_PFNMAP;
 
 	vma->vm_private_data = map;
-	map->vma = vma;
+
+	if (use_ptemod)
+		map->vma = vma;
 
 	map->is_ro = !(vma->vm_flags & VM_WRITE);
 
 	spin_unlock(&priv->lock);
 
-	err = apply_to_page_range(vma->vm_mm, vma->vm_start,
-				  vma->vm_end - vma->vm_start,
-				  find_grant_ptes, map);
-	if (err) {
-		printk(KERN_WARNING "find_grant_ptes() failure.\n");
-		return err;
+	if (use_ptemod) {
+		err = apply_to_page_range(vma->vm_mm, vma->vm_start,
+					  vma->vm_end - vma->vm_start,
+					  find_grant_ptes, map);
+		if (err)
+			return err;
 	}
 
 	err = map_grant_pages(map);
@@ -615,6 +649,15 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 
 	map->is_mapped = 1;
 
+	if (!use_ptemod) {
+		for(i = 0; i < count; i++) {
+			err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE,
+			                     map->pages[i]);
+			if (err)
+				return err;
+		}
+	}
+
 	return 0;
 
 unlock_out:
@@ -645,6 +688,8 @@ static int __init gntdev_init(void)
 	if (!xen_domain())
 		return -ENODEV;
 
+	use_ptemod = xen_pv_domain();
+
 	err = misc_register(&gntdev_miscdev);
 	if (err != 0) {
 		printk(KERN_ERR "Could not register gntdev device\n");
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 9ef54eb..9428ced 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -458,6 +458,9 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 	if (ret)
 		return ret;
 
+	if (xen_feature(XENFEAT_auto_translated_physmap))
+		return ret;
+
 	for (i = 0; i < count; i++) {
 		/* m2p override only supported for GNTMAP_contains_pte mappings */
 		if (!(map_ops[i].flags & GNTMAP_contains_pte))
@@ -483,6 +486,9 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref
*unmap_ops,
 	if (ret)
 		return ret;
 
+	if (xen_feature(XENFEAT_auto_translated_physmap))
+		return ret;
+
 	for (i = 0; i < count; i++) {
 		ret = m2p_remove_override(pages[i]);
 		if (ret)
-- 
1.7.3.4


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

Daniel De Graaf

2011-Jan-14 15:46 UTC

head link

[Xen-devel] [PATCH 6/6] xen-gntalloc: Userspace grant allocation driver

This allows a userspace application to allocate a shared page for
implementing inter-domain communication or device drivers. These
shared pages can be mapped using the gntdev device or by the kernel
in another domain.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/Kconfig    |    7 +
 drivers/xen/Makefile   |    2 +
 drivers/xen/gntalloc.c |  488 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/xen/gntalloc.h |   70 +++++++
 4 files changed, 567 insertions(+), 0 deletions(-)
 create mode 100644 drivers/xen/gntalloc.c
 create mode 100644 include/xen/gntalloc.h

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 07bec09..1fc06d3 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -79,6 +79,13 @@ config XEN_GNTDEV
 	select MMU_NOTIFIER
 	help
 	  Allows userspace processes to use grants.
+	  
+config XEN_GRANT_DEV_ALLOC
+	tristate "User-space grant reference allocator driver"
+	depends on XEN
+	help
+	  Allows userspace processes to create pages with access granted
+	  to other domains.
 
 config XEN_PLATFORM_PCI
 	tristate "xen platform pci device driver"
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 5088cc2..9585a1d 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_XEN_XENCOMM)	+= xencomm.o
 obj-$(CONFIG_XEN_BALLOON)	+= balloon.o
 obj-$(CONFIG_XEN_DEV_EVTCHN)	+= xen-evtchn.o
 obj-$(CONFIG_XEN_GNTDEV)	+= xen-gntdev.o
+obj-$(CONFIG_XEN_GRANT_DEV_ALLOC)	+= xen-gntalloc.o
 obj-$(CONFIG_XENFS)		+= xenfs/
 obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
 obj-$(CONFIG_XEN_PLATFORM_PCI)	+= xen-platform-pci.o
@@ -18,5 +19,6 @@ obj-$(CONFIG_XEN_DOM0)		+= pci.o
 
 xen-evtchn-y			:= evtchn.o
 xen-gntdev-y				:= gntdev.o
+xen-gntalloc-y				:= gntalloc.o
 
 xen-platform-pci-y		:= platform-pci.o
diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
new file mode 100644
index 0000000..4bee149
--- /dev/null
+++ b/drivers/xen/gntalloc.c
@@ -0,0 +1,488 @@
+/******************************************************************************
+ * gntalloc.c
+ *
+ * Device for creating grant references (in user-space) that may be shared
+ * with other domains.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * This driver exists to allow userspace programs in Linux to allocate kernel
+ * memory that will later be shared with another domain.  Without this device,
+ * Linux userspace programs cannot create grant references.
+ *
+ * How this stuff works:
+ *   X -> granting a page to Y
+ *   Y -> mapping the grant from X
+ *
+ *   1. X uses the gntalloc device to allocate a page of kernel memory, P.
+ *   2. X creates an entry in the grant table that says domid(Y) can access P.
+ *      This is done without a hypercall unless the grant table needs
expansion.
+ *   3. X gives the grant reference identifier, GREF, to Y.
+ *   4. Y maps the page, either directly into kernel memory for use in a
backend
+ *      driver, or via a the gntdev device to map into the address space of an
+ *      application running in Y. This is the first point at which Xen does any
+ *      tracking of the page.
+ *   5. A program in X mmap()s a segment of the gntalloc device that
corresponds
+ *      to the shared page, and can now communicate with Y over the shared
page.
+ *
+ *
+ * NOTE TO USERSPACE LIBRARIES:
+ *   The grant allocation and mmap()ing are, naturally, two separate
operations.
+ *   You set up the sharing by calling the create ioctl() and then the mmap().
+ *   Teardown requires munmap() and either close() or ioctl().
+ *
+ * WARNING: Since Xen does not allow a guest to forcibly end the use of a grant
+ * reference, this device can be used to consume kernel memory by leaving grant
+ * references mapped by another domain when an application exits. Therefore,
+ * there is a global limit on the number of pages that can be allocated. When
+ * all references to the page are unmapped, it will be freed during the next
+ * grant operation.
+ */
+
+#include <asm/atomic.h>
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/device.h>
+#include <linux/mm.h>
+#include <asm/uaccess.h>
+#include <linux/types.h>
+#include <linux/list.h>
+
+#include <xen/xen.h>
+#include <xen/page.h>
+#include <xen/grant_table.h>
+#include <xen/gntalloc.h>
+
+static int debug = 0;
+module_param(debug, int, 0644);
+
+static int limit = 1024;
+module_param(limit, int, 0644);
+
+static LIST_HEAD(gref_list);
+static DEFINE_SPINLOCK(gref_lock);
+static int gref_size = 0;
+
+/* Metadata on a grant reference. */
+struct gntalloc_gref {
+	struct list_head next_all;   /* list entry gref_list */
+	struct list_head next_file;  /* list entry file->list, if open */
+	struct page* page;	     /* The shared page */
+	uint64_t file_index;         /* File offset for mmap() */
+	unsigned int users;          /* Use count - when zero, waiting on Xen */
+	grant_ref_t gref_id;         /* The grant reference number */
+};
+
+struct gntalloc_file_private_data {
+	struct list_head list;
+	uint64_t index;
+};
+
+static void __del_gref(struct gntalloc_gref *gref);
+
+static void do_cleanup(void)
+{
+	struct gntalloc_gref *gref, *n;
+	list_for_each_entry_safe(gref, n, &gref_list, next_all) {
+		if (!gref->users)
+			__del_gref(gref);
+	}
+}
+
+static int add_grefs(struct ioctl_gntalloc_alloc_gref* op,
+	uint32_t* gref_ids, struct gntalloc_file_private_data *priv)
+{
+	int i, rc, readonly;
+	LIST_HEAD(queue_all);
+	LIST_HEAD(queue_file);
+	struct gntalloc_gref *gref;
+
+	readonly = !(op->flags & GNTALLOC_FLAG_WRITABLE);
+	rc = -ENOMEM;
+	for(i=0; i < op->count; i++) {
+		gref = kzalloc(sizeof(*gref), GFP_KERNEL);
+		if (!gref)
+			goto undo;
+		list_add_tail(&gref->next_all, &queue_all);
+		list_add_tail(&gref->next_file, &queue_file);
+		gref->users = 1;
+		gref->file_index = op->index + i * PAGE_SIZE;
+		gref->page = alloc_page(GFP_KERNEL|__GFP_ZERO);
+		if (!gref->page)
+			goto undo;
+
+		/* Grant foreign access to the page. */
+		gref->gref_id = gnttab_grant_foreign_access(op->domid,
+			pfn_to_mfn(page_to_pfn(gref->page)), readonly);
+		if (gref->gref_id < 0) {
+			rc = gref->gref_id;
+			goto undo;
+		}
+		gref_ids[i] = gref->gref_id;
+	}
+
+	/* Add to gref lists. */
+	spin_lock(&gref_lock);
+	list_splice_tail(&queue_all, &gref_list);
+	list_splice_tail(&queue_file, &priv->list);
+	spin_unlock(&gref_lock);
+
+	return 0;
+
+undo:
+	list_for_each_entry(gref, &queue_file, next_file) {
+		if (gref->page && gref->gref_id >= 0) {
+			__del_gref(gref);
+		} else {
+			if (gref->page)
+				__free_page(gref->page);
+			list_del(&gref->next_all);
+			kfree(gref);
+		}
+	}
+
+	/* It''s possible for the target domain to map the just-allocated
grant
+	 * references by blindly guessing their IDs; if this is done, then
+	 * __del_gref will leave them in the queue_all list. They need to be
+	 * added to the global list so that we can free them when they are no
+	 * longer referenced.
+	 */
+	if (unlikely(!list_empty(&queue_all))) {
+		spin_lock(&gref_lock);
+		list_splice_tail(&queue_all, &gref_list);
+		spin_unlock(&gref_lock);
+	}
+	return rc;
+}
+
+static void __del_gref(struct gntalloc_gref *gref)
+{
+	if (gnttab_query_foreign_access(gref->gref_id))
+		return;
+
+	if (!gnttab_end_foreign_access_ref(gref->gref_id, 0))
+		return;
+
+	gref_size--;
+	list_del(&gref->next_all);
+
+	__free_page(gref->page);
+	kfree(gref);
+}
+
+// finds contiguous grant references in a file, returns the first
+static struct gntalloc_gref* find_grefs(struct gntalloc_file_private_data
*priv,
+                                       uint64_t index, uint32_t count)
+{
+	struct gntalloc_gref *rv = NULL, *gref;
+	list_for_each_entry(gref, &priv->list, next_file) {
+		if (gref->file_index == index && !rv)
+			rv = gref;
+		if (rv) {
+			if (gref->file_index != index)
+				return NULL;
+			index += PAGE_SIZE;
+			count--;
+			if (count == 0)
+				return rv;
+		}
+	}
+	return NULL;
+}
+
+/*
+ * -------------------------------------
+ *  File operations.
+ * -------------------------------------
+ */
+static int gntalloc_open(struct inode *inode, struct file *filp)
+{
+	struct gntalloc_file_private_data *priv;
+
+	try_module_get(THIS_MODULE);
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		goto out_nomem;
+	INIT_LIST_HEAD(&priv->list);
+
+	filp->private_data = priv;
+
+	if (debug)
+		printk("%s: priv %p\n", __FUNCTION__, priv);
+
+	return 0;
+
+out_nomem:
+	return -ENOMEM;
+}
+
+static int gntalloc_release(struct inode *inode, struct file *filp)
+{
+	struct gntalloc_file_private_data *priv = filp->private_data;
+	struct gntalloc_gref *gref;
+
+	if (debug)
+		printk("%s: priv %p\n", __FUNCTION__, priv);
+
+	spin_lock(&gref_lock);
+	while (!list_empty(&priv->list)) {
+		gref = list_entry(priv->list.next,
+			struct gntalloc_gref, next_file);
+		list_del(&gref->next_file);
+		gref->users--;
+		if (gref->users == 0)
+			__del_gref(gref);
+	}
+	kfree(priv);
+	spin_unlock(&gref_lock);
+
+	module_put(THIS_MODULE);
+
+	return 0;
+}
+
+static long gntalloc_ioctl_alloc(struct gntalloc_file_private_data *priv,
+                                 struct ioctl_gntalloc_alloc_gref __user *arg)
+{
+	int rc = 0;
+	struct ioctl_gntalloc_alloc_gref op;
+	uint32_t* gref_ids;
+
+	if (debug)
+		printk("%s: priv %p\n", __FUNCTION__, priv);
+
+	if (copy_from_user(&op, arg, sizeof(op))) {
+		rc = -EFAULT;
+		goto out;
+	}
+
+	gref_ids = kzalloc(sizeof(gref_ids[0]) * op.count, GFP_TEMPORARY);
+	if (!gref_ids) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	spin_lock(&gref_lock);
+	do_cleanup();
+	if (gref_size + op.count > limit) {
+		spin_unlock(&gref_lock);
+		rc = -ENOSPC;
+		goto out_free;
+	}
+	gref_size += op.count;
+	op.index = priv->index;
+	priv->index += op.count * PAGE_SIZE;
+	spin_unlock(&gref_lock);
+
+	rc = add_grefs(&op, gref_ids, priv);
+	if (rc < 0)
+		goto out_free;
+
+	if (copy_to_user(arg, &op, sizeof(op))) {
+		rc = -EFAULT;
+		goto out_free;
+	}
+	if (copy_to_user(arg->gref_ids, gref_ids,
+	                   sizeof(gref_ids[0]) * op.count)) {
+		rc = -EFAULT;
+		goto out_free;
+	}
+
+out_free:
+	kfree(gref_ids);
+out:
+	return rc;
+}
+
+static long gntalloc_ioctl_dealloc(struct gntalloc_file_private_data *priv,
+                                   void __user *arg)
+{
+	int i, rc = 0;
+	struct ioctl_gntalloc_dealloc_gref op;
+	struct gntalloc_gref *gref, *n;
+
+	if (debug)
+		printk("%s: priv %p\n", __FUNCTION__, priv);
+
+	if (copy_from_user(&op, arg, sizeof(op))) {
+		rc = -EFAULT;
+		goto dealloc_grant_out;
+	}
+
+	spin_lock(&gref_lock);
+	gref = find_grefs(priv, op.index, op.count);
+	if (gref) {
+		for(i=0; i < op.count; i++) {
+			n = list_entry(gref->next_file.next,
+			               struct gntalloc_gref, next_file);
+			list_del(&gref->next_file);
+			gref->users--;
+			gref = n;
+		}
+	} else {
+		rc = -EINVAL;
+	}
+
+	do_cleanup();
+
+	spin_unlock(&gref_lock);
+dealloc_grant_out:
+	return rc;
+}
+
+static long gntalloc_ioctl(struct file *filp, unsigned int cmd,
+		unsigned long arg)
+{
+	struct gntalloc_file_private_data *priv = filp->private_data;
+
+	switch (cmd) {
+	case IOCTL_GNTALLOC_ALLOC_GREF:
+		return gntalloc_ioctl_alloc(priv, (void __user*)arg);
+
+	case IOCTL_GNTALLOC_DEALLOC_GREF:
+		return gntalloc_ioctl_dealloc(priv, (void __user*)arg);
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+
+	return 0;
+}
+
+static void gntalloc_vma_close(struct vm_area_struct *vma)
+{
+	struct gntalloc_gref *gref = vma->vm_private_data;
+	if (!gref)
+		return;
+
+	spin_lock(&gref_lock);
+	gref->users--;
+	if (gref->users == 0)
+		__del_gref(gref);
+	spin_unlock(&gref_lock);
+}
+
+static struct vm_operations_struct gntalloc_vmops = {
+	.close = gntalloc_vma_close,
+};
+
+static int gntalloc_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct gntalloc_file_private_data *priv = filp->private_data;
+	struct gntalloc_gref *gref;
+	int count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+	int rv, i;
+
+	if (debug)
+		printk("%s: priv %p, page %lu+%d\n", __func__,
+		       priv, vma->vm_pgoff, count);
+
+	if (!(vma->vm_flags & VM_SHARED)) {
+		printk(KERN_ERR "%s: Mapping must be shared.\n", __func__);
+		return -EINVAL;
+	}
+
+	spin_lock(&gref_lock);
+	gref = find_grefs(priv, vma->vm_pgoff << PAGE_SHIFT, count);
+	if (gref == NULL) {
+		rv = -ENOENT;
+		if (debug)
+			printk("%s: Could not find grant reference",
+				__func__);
+		goto out_unlock;
+	}
+
+	vma->vm_private_data = gref;
+
+	vma->vm_flags |= VM_RESERVED;
+	vma->vm_flags |= VM_DONTCOPY;
+	vma->vm_flags |= VM_IO;
+	vma->vm_flags |= VM_PFNMAP | VM_PFN_AT_MMAP;
+
+	vma->vm_ops = &gntalloc_vmops;
+
+	for(i=0; i < count; i++) {
+		gref->users++;
+		rv = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
+		                    gref->page);
+		if (rv)
+			goto out_unlock;
+
+		gref = list_entry(gref->next_file.next,
+		                  struct gntalloc_gref, next_file);
+	}
+	rv = 0;
+
+out_unlock:
+	spin_unlock(&gref_lock);
+	return rv;
+}
+
+static const struct file_operations gntalloc_fops = {
+	.owner = THIS_MODULE,
+	.open = gntalloc_open,
+	.release = gntalloc_release,
+	.unlocked_ioctl = gntalloc_ioctl,
+	.mmap = gntalloc_mmap
+};
+
+/*
+ * -------------------------------------
+ * Module creation/destruction.
+ * -------------------------------------
+ */
+static struct miscdevice gntalloc_miscdev = {
+	.minor	= MISC_DYNAMIC_MINOR,
+	.name	= "xen/gntalloc",
+	.fops	= &gntalloc_fops,
+};
+
+static int __init gntalloc_init(void)
+{
+	int err;
+
+	if (!xen_domain()) {
+		if (debug)
+			printk(KERN_ERR "gntalloc: You must be running Xen\n");
+		return -ENODEV;
+	}
+
+	err = misc_register(&gntalloc_miscdev);
+	if (err != 0) {
+		printk(KERN_ERR "Could not register misc gntalloc device\n");
+		return err;
+	}
+
+	if (debug)
+		printk(KERN_INFO "Created grant allocation device at %d,%d\n",
+			MISC_MAJOR, gntalloc_miscdev.minor);
+
+	return 0;
+}
+
+static void __exit gntalloc_exit(void)
+{
+	misc_deregister(&gntalloc_miscdev);
+}
+
+module_init(gntalloc_init);
+module_exit(gntalloc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Carter Weatherly <carter.weatherly@jhuapl.edu>,
"
+              "Daniel De Graaf <dgdegra@tycho.nsa.gov>");
+MODULE_DESCRIPTION("User-space grant reference allocator driver");
diff --git a/include/xen/gntalloc.h b/include/xen/gntalloc.h
new file mode 100644
index 0000000..e1d6d0f
--- /dev/null
+++ b/include/xen/gntalloc.h
@@ -0,0 +1,70 @@
+/******************************************************************************
+ * gntalloc.h
+ *
+ * Interface to /dev/xen/gntalloc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software
without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef __LINUX_PUBLIC_GNTALLOC_H__
+#define __LINUX_PUBLIC_GNTALLOC_H__
+
+/*
+ * Allocates a new page and creates a new grant reference.
+ */
+#define IOCTL_GNTALLOC_ALLOC_GREF \
+_IOC(_IOC_NONE, ''G'', 5, sizeof(struct
ioctl_gntalloc_alloc_gref))
+struct ioctl_gntalloc_alloc_gref {
+	/* IN parameters */
+	/* The ID of the domain to be given access to the grants. */
+	uint16_t domid;
+	/* Flags for this mapping */
+	uint16_t flags;
+	/* Number of pages to map */
+	uint32_t count;
+	/* OUT parameters */
+	/* The offset to be used on a subsequent call to mmap(). */
+	uint64_t index;
+	/* The grant references of the newly created grant, one per page */
+	/* Variable size, depending on count */
+	uint32_t gref_ids[1];
+};
+
+#define GNTALLOC_FLAG_WRITABLE 1
+
+/*
+ * Deallocates the grant reference, allowing the associated page to be freed if
+ * no other domains are using it.
+ */
+#define IOCTL_GNTALLOC_DEALLOC_GREF \
+_IOC(_IOC_NONE, ''G'', 6, sizeof(struct
ioctl_gntalloc_dealloc_gref))
+struct ioctl_gntalloc_dealloc_gref {
+	/* IN parameters */
+	/* The offset returned in the map operation */
+	uint64_t index;
+	/* Number of references to unmap */
+	uint32_t count;
+};
+#endif /* __LINUX_PUBLIC_GNTALLOC_H__ */
-- 
1.7.3.4


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

Konrad Rzeszutek Wilk

2011-Jan-14 16:20 UTC

head link

Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

On Fri, Jan 14, 2011 at 10:46:11AM -0500, Daniel De Graaf
wrote:> Because there is no limitation on how many times a user can open a
> given device file, an per-file-description limit on the number of
> pages granted offers little to no benefit. Change to a global limit
> and remove the ioctl() as the parameter can now be changed via sysfs.
Can you add here the comment which c/s in the Xen hypervisor
changed the tools so this won''t cause them to fail.

Just respond to this e-mail with the improved patch.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/gntdev.c |   52
+++++++++++++++----------------------------------
>  1 files changed, 16 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 1e31cdc..59e6a51 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -45,15 +45,15 @@ MODULE_AUTHOR("Derek G. Murray
<Derek.Murray@cl.cam.ac.uk>, "
>  	      "Gerd Hoffmann <kraxel@redhat.com>");
>  MODULE_DESCRIPTION("User-space granted page access driver");
>  
> -static int limit = 1024;
> +static int limit = 1024*1024;
>  module_param(limit, int, 0644);
> -MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped
at "
> -		"once by a gntdev instance");
> +MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped
by "
> +		"the gntdev device");
> +
> +static atomic_t pages_mapped = ATOMIC_INIT(0);
>  
>  struct gntdev_priv {
>  	struct list_head maps;
> -	uint32_t used;
> -	uint32_t limit;
>  	/* lock protects maps from concurrent changes */
>  	spinlock_t lock;
>  	struct mm_struct *mm;
> @@ -82,9 +82,8 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
>  #ifdef DEBUG
>  	struct grant_map *map;
>  
> -	pr_debug("maps list (priv %p, usage %d/%d)\n",
> -	       priv, priv->used, priv->limit);
> -
> +	pr_debug("%s: maps list (priv %p)\n",
> +	       __FUNCTION__, priv);
>  	list_for_each_entry(map, &priv->maps, next)
>  		pr_debug("  index %2d, count %2d %s\n",
>  		       map->index, map->count,
> @@ -121,9 +120,6 @@ static struct grant_map *gntdev_alloc_map(struct
gntdev_priv *priv, int count)
>  	add->count = count;
>  	add->priv  = priv;
>  
> -	if (add->count + priv->used > priv->limit)
> -		goto err;
> -
>  	return add;
>  
>  err:
> @@ -154,7 +150,6 @@ static void gntdev_add_map(struct gntdev_priv *priv,
struct grant_map *add)
>  	list_add_tail(&add->next, &priv->maps);
>  
>  done:
> -	priv->used += add->count;
>  	gntdev_print_maps(priv, "[new]", add->index);
>  }
>  
> @@ -200,7 +195,7 @@ static int gntdev_del_map(struct grant_map *map)
>  		if (map->unmap_ops[i].handle)
>  			return -EBUSY;
>  
> -	map->priv->used -= map->count;
> +	atomic_sub(map->count, &pages_mapped);
>  	list_del(&map->next);
>  	return 0;
>  }
> @@ -386,7 +381,6 @@ static int gntdev_open(struct inode *inode, struct file
*flip)
>  
>  	INIT_LIST_HEAD(&priv->maps);
>  	spin_lock_init(&priv->lock);
> -	priv->limit = limit;
>  
>  	priv->mm = get_task_mm(current);
>  	if (!priv->mm) {
> @@ -443,19 +437,25 @@ static long gntdev_ioctl_map_grant_ref(struct
gntdev_priv *priv,
>  	pr_debug("priv %p, add %d\n", priv, op.count);
>  	if (unlikely(op.count <= 0))
>  		return -EINVAL;
> -	if (unlikely(op.count > priv->limit))
> -		return -EINVAL;
>  
>  	err = -ENOMEM;
>  	map = gntdev_alloc_map(priv, op.count);
>  	if (!map)
>  		return err;
> +
No need.>  	if (copy_from_user(map->grants, &u->refs,
>  			   sizeof(map->grants[0]) * op.count) != 0) {
>  		gntdev_free_map(map);
>  		return err;
>  	}
>  
> +	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
> +	{
> +		pr_debug("%s: can''t map: over limit\n",
__FUNCTION__);
> +		gntdev_free_map(map);
> +		return err;
> +	}
> +
>  	spin_lock(&priv->lock);
>  	gntdev_add_map(priv, map);
>  	op.index = map->index << PAGE_SHIFT;
> @@ -518,23 +518,6 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
gntdev_priv *priv,
>  	return 0;
>  }
>  
> -static long gntdev_ioctl_set_max_grants(struct gntdev_priv *priv,
> -					struct ioctl_gntdev_set_max_grants __user *u)
> -{
> -	struct ioctl_gntdev_set_max_grants op;
> -
> -	if (copy_from_user(&op, u, sizeof(op)) != 0)
> -		return -EFAULT;
> -	pr_debug("priv %p, limit %d\n", priv, op.count);
> -	if (op.count > limit)
> -		return -E2BIG;
> -
> -	spin_lock(&priv->lock);
> -	priv->limit = op.count;
> -	spin_unlock(&priv->lock);
> -	return 0;
> -}
> -
>  static long gntdev_ioctl(struct file *flip,
>  			 unsigned int cmd, unsigned long arg)
>  {
> @@ -551,9 +534,6 @@ static long gntdev_ioctl(struct file *flip,
>  	case IOCTL_GNTDEV_GET_OFFSET_FOR_VADDR:
>  		return gntdev_ioctl_get_offset_for_vaddr(priv, ptr);
>  
> -	case IOCTL_GNTDEV_SET_MAX_GRANTS:
> -		return gntdev_ioctl_set_max_grants(priv, ptr);
> -
>  	default:
>  		pr_debug("priv %p, unknown cmd %x\n", priv, cmd);
>  		return -ENOIOCTLCMD;
> -- 
> 1.7.3.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2011-Jan-14 21:09 UTC

head link

Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

On 01/14/2011 11:20 AM, Konrad Rzeszutek Wilk wrote:> On Fri, Jan 14, 2011 at 10:46:11AM -0500, Daniel De Graaf wrote:
>> Because there is no limitation on how many times a user can open a
>> given device file, an per-file-description limit on the number of
>> pages granted offers little to no benefit. Change to a global limit
>> and remove the ioctl() as the parameter can now be changed via sysfs.
> 
> Can you add here the comment which c/s in the Xen hypervisor
> changed the tools so this won''t cause them to fail.
> 
> Just respond to this e-mail with the improved patch.
> Latest xen-unstable.hg doesn''t have the change; I recall Ian said
he would commit it sometime after -rc1. I''ll post a modified patch
as soon as I see the changeset.

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

Konrad Rzeszutek Wilk

2011-Jan-14 22:19 UTC

head link

Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

On Fri, Jan 14, 2011 at 04:09:54PM -0500, Daniel De Graaf
wrote:> On 01/14/2011 11:20 AM, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jan 14, 2011 at 10:46:11AM -0500, Daniel De Graaf wrote:
> >> Because there is no limitation on how many times a user can open a
> >> given device file, an per-file-description limit on the number of
> >> pages granted offers little to no benefit. Change to a global
limit
> >> and remove the ioctl() as the parameter can now be changed via
sysfs.
> > 
> > Can you add here the comment which c/s in the Xen hypervisor
> > changed the tools so this won''t cause them to fail.
> > 
> > Just respond to this e-mail with the improved patch.
> > 
> Latest xen-unstable.hg doesn''t have the change; I recall Ian said
> he would commit it sometime after -rc1. I''ll post a modified patch
> as soon as I see the changeset.
Sweet. Thanks!


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

Stefano Stabellini

2011-Jan-17 14:28 UTC

head link

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

On Fri, 14 Jan 2011, Daniel De Graaf wrote:> HVM does not allow direct PTE modification, so instead we request
> that Xen change its internal p2m mappings on the allocated pages and
> map the memory into userspace normally.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/gntdev.c      |  117
+++++++++++++++++++++++++++++++--------------
>  drivers/xen/grant-table.c |    6 ++
>  2 files changed, 87 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 8a12857..1931657 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -32,6 +32,7 @@
>  #include <linux/sched.h>
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
> +#include <linux/highmem.h>
>  
>  #include <xen/xen.h>
>  #include <xen/grant_table.h>
> @@ -52,6 +53,8 @@ MODULE_PARM_DESC(limit, "Maximum number of grants
that may be mapped by "
>  
>  static atomic_t pages_mapped = ATOMIC_INIT(0);
>  
> +static int use_ptemod = 0;
> +
>  struct gntdev_priv {
>  	struct list_head maps;
>  	/* lock protects maps from concurrent changes */
> @@ -184,6 +187,9 @@ static void gntdev_put_map(struct grant_map *map)
>  
>  	atomic_sub(map->count, &pages_mapped);
>  
> +	if (!use_ptemod)
> +		unmap_grant_pages(map, 0, map->count);
> +
>  	for (i = 0; i < map->count; i++) {
>  		if (map->pages[i])
>  			__free_page(map->pages[i]);
> @@ -212,9 +218,12 @@ static int find_grant_ptes(pte_t *pte, pgtable_t
token,
>  static int map_grant_pages(struct grant_map *map)
>  {
>  	int i, flags, err = 0;
> +	phys_addr_t addr;
>  	struct gnttab_map_grant_ref* map_ops = NULL;
>  
> -	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
> +	flags = GNTMAP_host_map;
> +	if (use_ptemod)
> +		flags |= GNTMAP_application_map | GNTMAP_contains_pte;
>  	if (map->is_ro)
>  		flags |= GNTMAP_readonly;
>  
> @@ -224,7 +233,11 @@ static int map_grant_pages(struct grant_map *map)
>  		goto out;
>  
>  	for(i=0; i < map->count; i++) {
> -		gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr, flags,
> +		if (use_ptemod)
> +			addr = map->pginfo[i].pte_maddr;
> +		else
> +			addr = (phys_addr_t)pfn_to_kaddr(page_to_pfn(map->pages[i]));
> +		gnttab_set_map_op(&map_ops[i], addr, flags,
>  				  map->pginfo[i].target.ref,
>  				  map->pginfo[i].target.domid);
>  	}
If I am not mistaken you are asking Xen to modify the virtual kernel
mappings of map->pages, but it is not enough to have correctly working
userspace mappings of map->pages: you need gnttab_map_refs to correctly
fix the p2m and add an entry to m2p_override too.
However in the last gntdev patch series I sent, requests with
!GNTMAP_contains_pte are not added to m2p_override and the p2m entries
of map->pages are not updated either.

Therefore you probably need this (untested) patch to make it work:

---

Add support for !GNTMAP_contains_pte mappings to gnttab_map_refs

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 9ef54eb..7d6d2ba 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -459,12 +459,38 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 		return ret;
 
 	for (i = 0; i < count; i++) {
-		/* m2p override only supported for GNTMAP_contains_pte mappings */
-		if (!(map_ops[i].flags & GNTMAP_contains_pte))
-			continue;
-		pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
-				(map_ops[i].host_addr & ~PAGE_MASK));
-		mfn = pte_mfn(*pte);
+		if ((map_ops[i].flags & GNTMAP_contains_pte)) {
+			pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
+					(map_ops[i].host_addr & ~PAGE_MASK));
+			mfn = pte_mfn(*pte);
+		} else {
+			pgd_t *pgd;
+			pud_t *pud;
+			pmd_t *pmd;
+			unsigned long addr = map_ops[i].host_addr;
+			pgd = pgd_offset(current->active_mm, addr);
+			if (pgd_none(*pgd)) {
+				printk(KERN_WARNING "invalid pgd found, skip address=%lx\n",
addr);
+				continue;
+			}
+			pud = pud_offset(pgd, addr);
+			if (pud_none(*pud)) {
+				printk(KERN_WARNING "invalid pud found, skip address=%lx\n",
addr);
+				continue;
+			}
+			pmd = pmd_offset(pud, addr);
+			if (pmd_none(*pmd)) {
+				printk(KERN_WARNING "invalid pmd found, skip address=%lx\n",
addr);
+				continue;
+			}
+			pte = pte_offset_map(pmd, addr);
+			if (pte_none(*pte)) {
+				printk(KERN_WARNING "invalid pte found, skip address=%lx\n",
addr);
+				continue;
+			}
+			mfn = pte_mfn(*pte);
+			pte_unmap(pte);
+		}
 		ret = m2p_add_override(mfn, pages[i]);
 		if (ret)
 			return ret;

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

Daniel De Graaf

2011-Jan-18 16:09 UTC

head link

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

On 01/17/2011 09:28 AM, Stefano Stabellini wrote:> On Fri, 14 Jan 2011, Daniel De Graaf wrote:
>> HVM does not allow direct PTE modification, so instead we request
>> that Xen change its internal p2m mappings on the allocated pages and
>> map the memory into userspace normally.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> ---
>>  drivers/xen/gntdev.c      |  117
+++++++++++++++++++++++++++++++--------------
>>  drivers/xen/grant-table.c |    6 ++
>>  2 files changed, 87 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
>> index 8a12857..1931657 100644
>> --- a/drivers/xen/gntdev.c
>> +++ b/drivers/xen/gntdev.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/sched.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/slab.h>
>> +#include <linux/highmem.h>
>>  
>>  #include <xen/xen.h>
>>  #include <xen/grant_table.h>
>> @@ -52,6 +53,8 @@ MODULE_PARM_DESC(limit, "Maximum number of
grants that may be mapped by "
>>  
>>  static atomic_t pages_mapped = ATOMIC_INIT(0);
>>  
>> +static int use_ptemod = 0;
>> +
>>  struct gntdev_priv {
>>  	struct list_head maps;
>>  	/* lock protects maps from concurrent changes */
>> @@ -184,6 +187,9 @@ static void gntdev_put_map(struct grant_map *map)
>>  
>>  	atomic_sub(map->count, &pages_mapped);
>>  
>> +	if (!use_ptemod)
>> +		unmap_grant_pages(map, 0, map->count);
>> +
>>  	for (i = 0; i < map->count; i++) {
>>  		if (map->pages[i])
>>  			__free_page(map->pages[i]);
>> @@ -212,9 +218,12 @@ static int find_grant_ptes(pte_t *pte, pgtable_t
token,
>>  static int map_grant_pages(struct grant_map *map)
>>  {
>>  	int i, flags, err = 0;
>> +	phys_addr_t addr;
>>  	struct gnttab_map_grant_ref* map_ops = NULL;
>>  
>> -	flags = GNTMAP_host_map | GNTMAP_application_map |
GNTMAP_contains_pte;
>> +	flags = GNTMAP_host_map;
>> +	if (use_ptemod)
>> +		flags |= GNTMAP_application_map | GNTMAP_contains_pte;
>>  	if (map->is_ro)
>>  		flags |= GNTMAP_readonly;
>>  
>> @@ -224,7 +233,11 @@ static int map_grant_pages(struct grant_map *map)
>>  		goto out;
>>  
>>  	for(i=0; i < map->count; i++) {
>> -		gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr,
flags,
>> +		if (use_ptemod)
>> +			addr = map->pginfo[i].pte_maddr;
>> +		else
>> +			addr = (phys_addr_t)pfn_to_kaddr(page_to_pfn(map->pages[i]));
>> +		gnttab_set_map_op(&map_ops[i], addr, flags,
>>  				  map->pginfo[i].target.ref,
>>  				  map->pginfo[i].target.domid);
>>  	}
> 
> If I am not mistaken you are asking Xen to modify the virtual kernel
> mappings of map->pages, but it is not enough to have correctly working
> userspace mappings of map->pages: you need gnttab_map_refs to correctly
> fix the p2m and add an entry to m2p_override too.
> However in the last gntdev patch series I sent, requests with
> !GNTMAP_contains_pte are not added to m2p_override and the p2m entries
> of map->pages are not updated either.
> 
> Therefore you probably need this (untested) patch to make it work:
> 
It looks like this patch only applies if using gnttab_map_refs without
GNTMAP_contains_pte on PV guests; my patch only uses !GNTMAP_contains_pte
on HVM, so it doesn''t need this change.

I think this patch would be needed if we wanted to remove GNTMAP_contains_pte
from the PV case, which might make the code cleaner (it would remove the
dependency on the MM notifier, and allow multiple mmap() of the device in PV).
I didn''t want to change the working PV case when adding HVM support, so
I
left that code mostly alone.

Just from quickly looking at the patch, since it uses current->active_mm,
it seems that it might still have issues with multiple processes mapping
the area. I would assume the update would be purely to the p2m table, not
involving any page tables (since the pages should not yet be mapped).
> ---
> 
> Add support for !GNTMAP_contains_pte mappings to gnttab_map_refs
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 9ef54eb..7d6d2ba 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -459,12 +459,38 @@ int gnttab_map_refs(struct gnttab_map_grant_ref
*map_ops,
>  		return ret;
>  
>  	for (i = 0; i < count; i++) {
> -		/* m2p override only supported for GNTMAP_contains_pte mappings */
> -		if (!(map_ops[i].flags & GNTMAP_contains_pte))
> -			continue;
> -		pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> -				(map_ops[i].host_addr & ~PAGE_MASK));
> -		mfn = pte_mfn(*pte);
> +		if ((map_ops[i].flags & GNTMAP_contains_pte)) {
> +			pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> +					(map_ops[i].host_addr & ~PAGE_MASK));
> +			mfn = pte_mfn(*pte);
> +		} else {
> +			pgd_t *pgd;
> +			pud_t *pud;
> +			pmd_t *pmd;
> +			unsigned long addr = map_ops[i].host_addr;
> +			pgd = pgd_offset(current->active_mm, addr);
> +			if (pgd_none(*pgd)) {
> +				printk(KERN_WARNING "invalid pgd found, skip address=%lx\n",
addr);
> +				continue;
> +			}
> +			pud = pud_offset(pgd, addr);
> +			if (pud_none(*pud)) {
> +				printk(KERN_WARNING "invalid pud found, skip address=%lx\n",
addr);
> +				continue;
> +			}
> +			pmd = pmd_offset(pud, addr);
> +			if (pmd_none(*pmd)) {
> +				printk(KERN_WARNING "invalid pmd found, skip address=%lx\n",
addr);
> +				continue;
> +			}
> +			pte = pte_offset_map(pmd, addr);
> +			if (pte_none(*pte)) {
> +				printk(KERN_WARNING "invalid pte found, skip address=%lx\n",
addr);
> +				continue;
> +			}
> +			mfn = pte_mfn(*pte);
> +			pte_unmap(pte);
> +		}
>  		ret = m2p_add_override(mfn, pages[i]);
>  		if (ret)
>  			return ret;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Stefano Stabellini

2011-Jan-18 17:27 UTC

head link

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

On Tue, 18 Jan 2011, Daniel De Graaf wrote:> >> @@ -224,7 +233,11 @@ static int map_grant_pages(struct grant_map
*map)
> >>  		goto out;
> >>  
> >>  	for(i=0; i < map->count; i++) {
> >> -		gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr,
flags,
> >> +		if (use_ptemod)
> >> +			addr = map->pginfo[i].pte_maddr;
> >> +		else
> >> +			addr =
(phys_addr_t)pfn_to_kaddr(page_to_pfn(map->pages[i]));
> >> +		gnttab_set_map_op(&map_ops[i], addr, flags,
> >>  				  map->pginfo[i].target.ref,
> >>  				  map->pginfo[i].target.domid);
> >>  	}
> > 
> > If I am not mistaken you are asking Xen to modify the virtual kernel
> > mappings of map->pages, but it is not enough to have correctly
working
> > userspace mappings of map->pages: you need gnttab_map_refs to
correctly
> > fix the p2m and add an entry to m2p_override too.
> > However in the last gntdev patch series I sent, requests with
> > !GNTMAP_contains_pte are not added to m2p_override and the p2m entries
> > of map->pages are not updated either.
> > 
> > Therefore you probably need this (untested) patch to make it work:
> > 
> 
> It looks like this patch only applies if using gnttab_map_refs without
> GNTMAP_contains_pte on PV guests; my patch only uses !GNTMAP_contains_pte
> on HVM, so it doesn''t need this change.
> 
> I think this patch would be needed if we wanted to remove
GNTMAP_contains_pte
> from the PV case, which might make the code cleaner (it would remove the
> dependency on the MM notifier, and allow multiple mmap() of the device in
PV).
> I didn''t want to change the working PV case when adding HVM
support, so I
> left that code mostly alone.
Yes, you are right I was thinking at the PV case that it is not relevant
here.


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

Daniel De Graaf

2011-Jan-18 17:57 UTC

head link

[Xen-devel] [PATCH 1/6 v2] xen-gntdev: Change page limit to be global instead of per-open

Because there is no limitation on how many times a user can open a
given device file, an per-file-description limit on the number of
pages granted offers little to no benefit. Change to a global limit
and remove the ioctl() as the parameter can now be changed via sysfs.

Xen tools changeset 22768:f8d801e5573e is needed to eliminate the
error this change produces in xc_gnttab_set_max_grants.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |   52 +++++++++++++++----------------------------------
 1 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 1e31cdc..59e6a51 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -45,15 +45,15 @@ MODULE_AUTHOR("Derek G. Murray
<Derek.Murray@cl.cam.ac.uk>, "
 	      "Gerd Hoffmann <kraxel@redhat.com>");
 MODULE_DESCRIPTION("User-space granted page access driver");
 
-static int limit = 1024;
+static int limit = 1024*1024;
 module_param(limit, int, 0644);
-MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped at
"
-		"once by a gntdev instance");
+MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped by
"
+		"the gntdev device");
+
+static atomic_t pages_mapped = ATOMIC_INIT(0);
 
 struct gntdev_priv {
 	struct list_head maps;
-	uint32_t used;
-	uint32_t limit;
 	/* lock protects maps from concurrent changes */
 	spinlock_t lock;
 	struct mm_struct *mm;
@@ -82,9 +82,8 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
 #ifdef DEBUG
 	struct grant_map *map;
 
-	pr_debug("maps list (priv %p, usage %d/%d)\n",
-	       priv, priv->used, priv->limit);
-
+	pr_debug("%s: maps list (priv %p)\n",
+	       __FUNCTION__, priv);
 	list_for_each_entry(map, &priv->maps, next)
 		pr_debug("  index %2d, count %2d %s\n",
 		       map->index, map->count,
@@ -121,9 +120,6 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv
*priv, int count)
 	add->count = count;
 	add->priv  = priv;
 
-	if (add->count + priv->used > priv->limit)
-		goto err;
-
 	return add;
 
 err:
@@ -154,7 +150,6 @@ static void gntdev_add_map(struct gntdev_priv *priv, struct
grant_map *add)
 	list_add_tail(&add->next, &priv->maps);
 
 done:
-	priv->used += add->count;
 	gntdev_print_maps(priv, "[new]", add->index);
 }
 
@@ -200,7 +195,7 @@ static int gntdev_del_map(struct grant_map *map)
 		if (map->unmap_ops[i].handle)
 			return -EBUSY;
 
-	map->priv->used -= map->count;
+	atomic_sub(map->count, &pages_mapped);
 	list_del(&map->next);
 	return 0;
 }
@@ -386,7 +381,6 @@ static int gntdev_open(struct inode *inode, struct file
*flip)
 
 	INIT_LIST_HEAD(&priv->maps);
 	spin_lock_init(&priv->lock);
-	priv->limit = limit;
 
 	priv->mm = get_task_mm(current);
 	if (!priv->mm) {
@@ -443,19 +437,25 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 	pr_debug("priv %p, add %d\n", priv, op.count);
 	if (unlikely(op.count <= 0))
 		return -EINVAL;
-	if (unlikely(op.count > priv->limit))
-		return -EINVAL;
 
 	err = -ENOMEM;
 	map = gntdev_alloc_map(priv, op.count);
 	if (!map)
 		return err;
+
 	if (copy_from_user(map->grants, &u->refs,
 			   sizeof(map->grants[0]) * op.count) != 0) {
 		gntdev_free_map(map);
 		return err;
 	}
 
+	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
+	{
+		pr_debug("%s: can''t map: over limit\n", __FUNCTION__);
+		gntdev_free_map(map);
+		return err;
+	}
+
 	spin_lock(&priv->lock);
 	gntdev_add_map(priv, map);
 	op.index = map->index << PAGE_SHIFT;
@@ -518,23 +518,6 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
gntdev_priv *priv,
 	return 0;
 }
 
-static long gntdev_ioctl_set_max_grants(struct gntdev_priv *priv,
-					struct ioctl_gntdev_set_max_grants __user *u)
-{
-	struct ioctl_gntdev_set_max_grants op;
-
-	if (copy_from_user(&op, u, sizeof(op)) != 0)
-		return -EFAULT;
-	pr_debug("priv %p, limit %d\n", priv, op.count);
-	if (op.count > limit)
-		return -E2BIG;
-
-	spin_lock(&priv->lock);
-	priv->limit = op.count;
-	spin_unlock(&priv->lock);
-	return 0;
-}
-
 static long gntdev_ioctl(struct file *flip,
 			 unsigned int cmd, unsigned long arg)
 {
@@ -551,9 +534,6 @@ static long gntdev_ioctl(struct file *flip,
 	case IOCTL_GNTDEV_GET_OFFSET_FOR_VADDR:
 		return gntdev_ioctl_get_offset_for_vaddr(priv, ptr);
 
-	case IOCTL_GNTDEV_SET_MAX_GRANTS:
-		return gntdev_ioctl_set_max_grants(priv, ptr);
-
 	default:
 		pr_debug("priv %p, unknown cmd %x\n", priv, cmd);
 		return -ENOIOCTLCMD;

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

Jeremy Fitzhardinge

2011-Jan-19 20:11 UTC

head link

[Xen-devel] Re: [PATCH 2/6] xen-gntdev: Remove unneeded structures from grant_map tracking data

On 01/14/2011 07:46 AM, Daniel De Graaf wrote:> The entire hypercall argument list isn''t required; only selected
> fields from the hypercall need to be tracked between the ioctl, map,
> and unmap operations.
I''m still not terribly happy about this one - specifically because of
how memory allocation is handled when setting up the hypercall
parameters.  Unless there''s a functional need for it, I''d
prefer you
drop it until we can do something better.

    J
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/gntdev.c |  225
++++++++++++++++++++++++++++++--------------------
>  1 files changed, 136 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 59e6a51..d0802b5 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -60,18 +60,24 @@ struct gntdev_priv {
>  	struct mmu_notifier mn;
>  };
>  
> +struct granted_page {
> +	u64 pte_maddr;
> +	union {
> +		struct ioctl_gntdev_grant_ref target;
> +		grant_handle_t handle;
> +	};
> +};
> +
>  struct grant_map {
>  	struct list_head next;
>  	struct gntdev_priv *priv;
>  	struct vm_area_struct *vma;
>  	int index;
>  	int count;
> -	int flags;
> -	int is_mapped;
> -	struct ioctl_gntdev_grant_ref *grants;
> -	struct gnttab_map_grant_ref   *map_ops;
> -	struct gnttab_unmap_grant_ref *unmap_ops;
> +	int is_mapped:1;
> +	int is_ro:1;
>  	struct page **pages;
> +	struct granted_page pginfo[0];
>  };
>  
>  /* ------------------------------------------------------------------ */
> @@ -91,24 +97,19 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
>  #endif
>  }
>  
> -static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int
count)
> +static struct grant_map *gntdev_alloc_map(int count,
> +		struct ioctl_gntdev_grant_ref* grants)
>  {
>  	struct grant_map *add;
>  	int i;
>  
> -	add = kzalloc(sizeof(struct grant_map), GFP_KERNEL);
> -	if (NULL == add)
> +	add = kzalloc(sizeof(*add) + sizeof(add->pginfo[0])*count,
GFP_KERNEL);
> +	if (!add)
>  		return NULL;
>  
> -	add->grants    = kzalloc(sizeof(add->grants[0])    * count,
GFP_KERNEL);
> -	add->map_ops   = kzalloc(sizeof(add->map_ops[0])   * count,
GFP_KERNEL);
> -	add->unmap_ops = kzalloc(sizeof(add->unmap_ops[0]) * count,
GFP_KERNEL);
> -	add->pages     = kzalloc(sizeof(add->pages[0])     * count,
GFP_KERNEL);
> -	if (NULL == add->grants    ||
> -	    NULL == add->map_ops   ||
> -	    NULL == add->unmap_ops ||
> -	    NULL == add->pages)
> -		goto err;
> +	add->pages = kzalloc(sizeof(add->pages[0])*count, GFP_KERNEL);
> +	if (!add->pages)
> +		goto err_nopages;
>  
>  	for (i = 0; i < count; i++) {
>  		add->pages[i] = alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
> @@ -118,20 +119,18 @@ static struct grant_map *gntdev_alloc_map(struct
gntdev_priv *priv, int count)
>  
>  	add->index = 0;
>  	add->count = count;
> -	add->priv  = priv;
> +	for(i = 0; i < count; i++)
> +		add->pginfo[i].target = grants[i];
>  
>  	return add;
>  
>  err:
> -	if (add->pages)
> -		for (i = 0; i < count; i++) {
> -			if (add->pages[i])
> -				__free_page(add->pages[i]);
> -		}
> +	for (i = 0; i < count; i++) {
> +		if (add->pages[i])
> +			__free_page(add->pages[i]);
> +	}
>  	kfree(add->pages);
> -	kfree(add->grants);
> -	kfree(add->map_ops);
> -	kfree(add->unmap_ops);
> +err_nopages:
>  	kfree(add);
>  	return NULL;
>  }
> @@ -140,6 +139,7 @@ static void gntdev_add_map(struct gntdev_priv *priv,
struct grant_map *add)
>  {
>  	struct grant_map *map;
>  
> +	spin_lock(&priv->lock);
>  	list_for_each_entry(map, &priv->maps, next) {
>  		if (add->index + add->count < map->index) {
>  			list_add_tail(&add->next, &map->next);
> @@ -150,7 +150,9 @@ static void gntdev_add_map(struct gntdev_priv *priv,
struct grant_map *add)
>  	list_add_tail(&add->next, &priv->maps);
>  
>  done:
> +	add->priv = priv;
>  	gntdev_print_maps(priv, "[new]", add->index);
> +	spin_unlock(&priv->lock);
>  }
>  
>  static struct grant_map *gntdev_find_map_index(struct gntdev_priv *priv,
> @@ -191,9 +193,10 @@ static int gntdev_del_map(struct grant_map *map)
>  
>  	if (map->vma)
>  		return -EBUSY;
> -	for (i = 0; i < map->count; i++)
> -		if (map->unmap_ops[i].handle)
> -			return -EBUSY;
> +	if (map->is_mapped)
> +		for (i = 0; i < map->count; i++)
> +			if (map->pginfo[i].handle)
> +				return -EBUSY;
>  
>  	atomic_sub(map->count, &pages_mapped);
>  	list_del(&map->next);
> @@ -207,15 +210,11 @@ static void gntdev_free_map(struct grant_map *map)
>  	if (!map)
>  		return;
>  
> -	if (map->pages)
> -		for (i = 0; i < map->count; i++) {
> -			if (map->pages[i])
> -				__free_page(map->pages[i]);
> -		}
> +	for (i = 0; i < map->count; i++) {
> +		if (map->pages[i])
> +			__free_page(map->pages[i]);
> +	}
>  	kfree(map->pages);
> -	kfree(map->grants);
> -	kfree(map->map_ops);
> -	kfree(map->unmap_ops);
>  	kfree(map);
>  }
>  
> @@ -229,50 +228,96 @@ static int find_grant_ptes(pte_t *pte, pgtable_t
token,
>  	u64 pte_maddr;
>  
>  	BUG_ON(pgnr >= map->count);
> +
>  	pte_maddr = arbitrary_virt_to_machine(pte).maddr;
> +	map->pginfo[pgnr].pte_maddr = pte_maddr;
>  
> -	gnttab_set_map_op(&map->map_ops[pgnr], pte_maddr,
> -			  GNTMAP_contains_pte | map->flags,
> -			  map->grants[pgnr].ref,
> -			  map->grants[pgnr].domid);
> -	gnttab_set_unmap_op(&map->unmap_ops[pgnr], pte_maddr,
> -			    GNTMAP_contains_pte | map->flags,
> -			    0 /* handle */);
>  	return 0;
>  }
>  
>  static int map_grant_pages(struct grant_map *map)
>  {
> -	int i, err = 0;
> +	int i, flags, err = 0;
> +	struct gnttab_map_grant_ref* map_ops = NULL;
>  
> -	pr_debug("map %d+%d\n", map->index, map->count);
> -	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
> -	if (err)
> -		return err;
> +	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
> +	if (map->is_ro)
> +		flags |= GNTMAP_readonly;
> +
> +	err = -ENOMEM;
> +	map_ops = kzalloc(sizeof(map_ops[0]) * map->count, GFP_TEMPORARY);
> +	if (!map_ops)
> +		goto out;
> +
> +	for(i=0; i < map->count; i++) {
> +		gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr, flags,
> +				  map->pginfo[i].target.ref,
> +				  map->pginfo[i].target.domid);
> +	}
> +	pr_debug("%s: map %d+%d\n", __FUNCTION__, map->index,
map->count);
> +
> +	err = gnttab_map_refs(map_ops, map->pages, map->count);
> +
> +	if (WARN_ON(err))
> +		goto out;
>  
>  	for (i = 0; i < map->count; i++) {
> -		if (map->map_ops[i].status)
> +		if (map_ops[i].status) {
> +			__free_page(map->pages[i]);
> +			map->pages[i] = NULL;
>  			err = -EINVAL;
> -		map->unmap_ops[i].handle = map->map_ops[i].handle;
> +		} else {
> +			map->pginfo[i].handle = map_ops[i].handle;
> +		}
>  	}
> +
> +out:
> +	kfree(map_ops);
>  	return err;
>  }
>  
> -static int unmap_grant_pages(struct grant_map *map, int offset, int pages)
> +static void unmap_grant_pages(struct grant_map *map, int offset, int
pages)
>  {
> -	int i, err = 0;
> +	int i, flags, err = 0;
> +	struct gnttab_unmap_grant_ref *unmap_ops;
> +	struct gnttab_unmap_grant_ref unmap_single;
> +
> +	if (pages > 1) {
> +		unmap_ops = kzalloc(sizeof(unmap_ops[0]) * pages,
> +		                    GFP_TEMPORARY);
> +		if (unlikely(!unmap_ops)) {
> +			for(i=0; i < pages; i++)
> +				unmap_grant_pages(map, offset + i, 1);
> +			return;
> +		}
> +	} else {
> +		unmap_ops = &unmap_single;
> +	}
> +
> +	flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
> +	if (map->is_ro)
> +		flags |= GNTMAP_readonly;
>  
> +	for(i=0; i < pages; i++)
> +		gnttab_set_unmap_op(&unmap_ops[i],
> +		                    map->pginfo[offset+i].pte_maddr, flags,
> +		                    map->pginfo[offset+i].handle);
>  	pr_debug("map %d+%d [%d+%d]\n", map->index, map->count,
offset, pages);
> -	err = gnttab_unmap_refs(map->unmap_ops + offset, map->pages,
pages);
> +
> +	err = gnttab_unmap_refs(unmap_ops, map->pages + offset, pages);
> +
>  	if (err)
> -		return err;
> +		goto out;
>  
>  	for (i = 0; i < pages; i++) {
> -		if (map->unmap_ops[offset+i].status)
> -			err = -EINVAL;
> -		map->unmap_ops[offset+i].handle = 0;
> +		WARN_ON(unmap_ops[i].status);
> +		__free_page(map->pages[offset+i]);
> +		map->pages[offset+i] = NULL;
> +		map->pginfo[offset+i].handle = 0;
>  	}
> -	return err;
> +out:
> +	if (unmap_ops != &unmap_single)
> +		kfree(unmap_ops);
>  }
>  
>  /* ------------------------------------------------------------------ */
> @@ -309,7 +354,6 @@ static void mn_invl_range_start(struct mmu_notifier
*mn,
>  	struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn);
>  	struct grant_map *map;
>  	unsigned long mstart, mend;
> -	int err;
>  
>  	spin_lock(&priv->lock);
>  	list_for_each_entry(map, &priv->maps, next) {
> @@ -327,10 +371,9 @@ static void mn_invl_range_start(struct mmu_notifier
*mn,
>  				map->index, map->count,
>  				map->vma->vm_start, map->vma->vm_end,
>  				start, end, mstart, mend);
> -		err = unmap_grant_pages(map,
> -					(mstart - map->vma->vm_start) >> PAGE_SHIFT,
> -					(mend - mstart) >> PAGE_SHIFT);
> -		WARN_ON(err);
> +		unmap_grant_pages(map,
> +				  (mstart - map->vma->vm_start) >> PAGE_SHIFT,
> +				  (mend - mstart) >> PAGE_SHIFT);
>  	}
>  	spin_unlock(&priv->lock);
>  }
> @@ -347,7 +390,6 @@ static void mn_release(struct mmu_notifier *mn,
>  {
>  	struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn);
>  	struct grant_map *map;
> -	int err;
>  
>  	spin_lock(&priv->lock);
>  	list_for_each_entry(map, &priv->maps, next) {
> @@ -356,8 +398,7 @@ static void mn_release(struct mmu_notifier *mn,
>  		pr_debug("map %d+%d (%lx %lx)\n",
>  				map->index, map->count,
>  				map->vma->vm_start, map->vma->vm_end);
> -		err = unmap_grant_pages(map, /* offset */ 0, map->count);
> -		WARN_ON(err);
> +		unmap_grant_pages(map, /* offset */ 0, map->count);
>  	}
>  	spin_unlock(&priv->lock);
>  }
> @@ -430,6 +471,7 @@ static long gntdev_ioctl_map_grant_ref(struct
gntdev_priv *priv,
>  {
>  	struct ioctl_gntdev_map_grant_ref op;
>  	struct grant_map *map;
> +	struct ioctl_gntdev_grant_ref* grants;
>  	int err;
>  
>  	if (copy_from_user(&op, u, sizeof(op)) != 0)
> @@ -438,37 +480,44 @@ static long gntdev_ioctl_map_grant_ref(struct
gntdev_priv *priv,
>  	if (unlikely(op.count <= 0))
>  		return -EINVAL;
>  
> -	err = -ENOMEM;
> -	map = gntdev_alloc_map(priv, op.count);
> -	if (!map)
> -		return err;
> +	grants = kmalloc(sizeof(grants[0]) * op.count, GFP_TEMPORARY);
> +	if (!grants)
> +		return -ENOMEM;
>  
> -	if (copy_from_user(map->grants, &u->refs,
> -			   sizeof(map->grants[0]) * op.count) != 0) {
> -		gntdev_free_map(map);
> -		return err;
> +	if (copy_from_user(grants, u->refs, sizeof(grants[0]) * op.count)) {
> +		err = -EFAULT;
> +		goto out_free;
>  	}
>  
> +	err = -ENOMEM;
> +	map = gntdev_alloc_map(op.count, grants);
> +	if (!map)
> +		goto out_free;
> +
>  	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
>  	{
>  		pr_debug("%s: can''t map: over limit\n",
__FUNCTION__);
> -		gntdev_free_map(map);
> -		return err;
> +		goto out_free_map;
>  	}
>  
> -	spin_lock(&priv->lock);
>  	gntdev_add_map(priv, map);
>  	op.index = map->index << PAGE_SHIFT;
> -	spin_unlock(&priv->lock);
>  
> -	if (copy_to_user(u, &op, sizeof(op)) != 0) {
> -		spin_lock(&priv->lock);
> -		gntdev_del_map(map);
> -		spin_unlock(&priv->lock);
> -		gntdev_free_map(map);
> -		return err;
> +	if (copy_to_user(u, &op, sizeof(op))) {
> +		err = -EFAULT;
> +		goto out_remove;
>  	}
> -	return 0;
> +	err = 0;
> +out_free:
> +	kfree(grants);
> +	return err;
> +out_remove:
> +	spin_lock(&priv->lock);
> +	gntdev_del_map(map);
> +	spin_unlock(&priv->lock);
> +out_free_map:
> +	gntdev_free_map(map);
> +	goto out_free;
>  }
>  
>  static long gntdev_ioctl_unmap_grant_ref(struct gntdev_priv *priv,
> @@ -574,9 +623,7 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
>  	vma->vm_private_data = map;
>  	map->vma = vma;
>  
> -	map->flags = GNTMAP_host_map | GNTMAP_application_map;
> -	if (!(vma->vm_flags & VM_WRITE))
> -		map->flags |= GNTMAP_readonly;
> +	map->is_ro = !(vma->vm_flags & VM_WRITE);
>  
>  	spin_unlock(&priv->lock);
>  

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

Konrad Rzeszutek Wilk

2011-Jan-20 21:07 UTC

head link

Re: [Xen-devel] [PATCH 4/6] xen-gntdev: Add reference counting to maps

On Fri, Jan 14, 2011 at 10:46:14AM -0500, Daniel De Graaf
wrote:> This allows userspace to perform mmap() on the gntdev device and then
> immediately close the filehandle or remove the mapping using the
> remove ioctl, with the mapped area remaining valid until unmapped.
> This also fixes an infinite loop when a gntdev device is closed
> without first unmapping all areas.
Can you respin this patch so it applies cleanly without the 
patch #2, please?

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

Daniel De Graaf

2011-Jan-21 14:36 UTC

head link

[Xen-devel] [PATCH v4.2 3/5] xen-gntdev: Add reference counting to maps

This allows userspace to perform mmap() on the gntdev device and then
immediately close the filehandle or remove the mapping using the
remove ioctl, with the mapped area remaining valid until unmapped.
This also fixes an infinite loop when a gntdev device is closed
without first unmapping all areas.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |   69 ++++++++++++++++++++-----------------------------
 1 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 6fe3c3c..11876bb 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -62,12 +62,12 @@ struct gntdev_priv {
 
 struct grant_map {
 	struct list_head next;
-	struct gntdev_priv *priv;
 	struct vm_area_struct *vma;
 	int index;
 	int count;
 	int flags;
 	int is_mapped;
+	atomic_t users;
 	struct ioctl_gntdev_grant_ref *grants;
 	struct gnttab_map_grant_ref   *map_ops;
 	struct gnttab_unmap_grant_ref *unmap_ops;
@@ -118,7 +118,7 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv
*priv, int count)
 
 	add->index = 0;
 	add->count = count;
-	add->priv  = priv;
+	atomic_set(&add->users, 1);
 
 	return add;
 
@@ -168,27 +168,17 @@ static struct grant_map *gntdev_find_map_index(struct
gntdev_priv *priv,
 	return NULL;
 }
 
-static int gntdev_del_map(struct grant_map *map)
-{
-	int i;
-
-	if (map->vma)
-		return -EBUSY;
-	for (i = 0; i < map->count; i++)
-		if (map->unmap_ops[i].handle)
-			return -EBUSY;
-
-	atomic_sub(map->count, &pages_mapped);
-	list_del(&map->next);
-	return 0;
-}
-
-static void gntdev_free_map(struct grant_map *map)
+static void gntdev_put_map(struct grant_map *map)
 {
 	int i;
 
 	if (!map)
 		return;
+	
+	if (!atomic_dec_and_test(&map->users))
+		return;
+	
+	atomic_sub(map->count, &pages_mapped);
 
 	if (map->pages)
 		for (i = 0; i < map->count; i++) {
@@ -268,6 +258,7 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
 	map->is_mapped = 0;
 	map->vma = NULL;
 	vma->vm_private_data = NULL;
+	gntdev_put_map(map);
 }
 
 static int gntdev_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
@@ -389,17 +380,14 @@ static int gntdev_release(struct inode *inode, struct file
*flip)
 {
 	struct gntdev_priv *priv = flip->private_data;
 	struct grant_map *map;
-	int err;
 
 	pr_debug("priv %p\n", priv);
 
 	spin_lock(&priv->lock);
 	while (!list_empty(&priv->maps)) {
 		map = list_entry(priv->maps.next, struct grant_map, next);
-		err = gntdev_del_map(map);
-		if (WARN_ON(err))
-			gntdev_free_map(map);
-
+		list_del(&map->next);
+		gntdev_put_map(map);
 	}
 	spin_unlock(&priv->lock);
 
@@ -426,16 +414,16 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 	if (!map)
 		return err;
 
-	if (copy_from_user(map->grants, &u->refs,
-			   sizeof(map->grants[0]) * op.count) != 0) {
-		gntdev_free_map(map);
-		return err;
-	}
-
 	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
 	{
 		pr_debug("%s: can''t map: over limit\n", __FUNCTION__);
-		gntdev_free_map(map);
+		gntdev_put_map(map);
+		return err;
+	}
+
+	if (copy_from_user(map->grants, &u->refs,
+			   sizeof(map->grants[0]) * op.count) != 0) {
+		gntdev_put_map(map);
 		return err;
 	}
 
@@ -444,13 +432,9 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv
*priv,
 	op.index = map->index << PAGE_SHIFT;
 	spin_unlock(&priv->lock);
 
-	if (copy_to_user(u, &op, sizeof(op)) != 0) {
-		spin_lock(&priv->lock);
-		gntdev_del_map(map);
-		spin_unlock(&priv->lock);
-		gntdev_free_map(map);
-		return err;
-	}
+	if (copy_to_user(u, &op, sizeof(op)) != 0)
+		return -EFAULT;
+
 	return 0;
 }
 
@@ -467,11 +451,12 @@ static long gntdev_ioctl_unmap_grant_ref(struct
gntdev_priv *priv,
 
 	spin_lock(&priv->lock);
 	map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count);
-	if (map)
-		err = gntdev_del_map(map);
+	if (map) {
+		list_del(&map->next);
+		gntdev_put_map(map);
+		err = 0;
+	}
 	spin_unlock(&priv->lock);
-	if (!err)
-		gntdev_free_map(map);
 	return err;
 }
 
@@ -551,6 +536,8 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 		goto unlock_out;
 	}
 
+	atomic_inc(&map->users);
+
 	vma->vm_ops = &gntdev_vmops;
 
 	vma->vm_flags |= VM_RESERVED|VM_DONTCOPY|VM_DONTEXPAND|VM_PFNMAP;
-- 
1.7.3.4


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

Daniel De Graaf

2011-Jan-21 14:38 UTC

head link

[Xen-devel] [PATCH v4.2 4/5] xen-gntdev: Support mapping in HVM domains

HVM does not allow direct PTE modification, so instead we request
that Xen change its internal p2m mappings on the allocated pages and
map the memory into userspace normally.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c      |  115 +++++++++++++++++++++++++++++++-------------
 drivers/xen/grant-table.c |    6 ++
 2 files changed, 87 insertions(+), 34 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 11876bb..c9d009c 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -32,6 +32,7 @@
 #include <linux/sched.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
+#include <linux/highmem.h>
 
 #include <xen/xen.h>
 #include <xen/grant_table.h>
@@ -52,6 +53,8 @@ MODULE_PARM_DESC(limit, "Maximum number of grants that
may be mapped by "
 
 static atomic_t pages_mapped = ATOMIC_INIT(0);
 
+static int use_ptemod = 0;
+
 struct gntdev_priv {
 	struct list_head maps;
 	/* lock protects maps from concurrent changes */
@@ -74,6 +77,8 @@ struct grant_map {
 	struct page **pages;
 };
 
+static int unmap_grant_pages(struct grant_map *map, int offset, int pages);
+
 /* ------------------------------------------------------------------ */
 
 static void gntdev_print_maps(struct gntdev_priv *priv,
@@ -180,11 +185,32 @@ static void gntdev_put_map(struct grant_map *map)
 	
 	atomic_sub(map->count, &pages_mapped);
 
-	if (map->pages)
+	if (map->pages) {
+		if (!use_ptemod)
+			unmap_grant_pages(map, 0, map->count);
+
 		for (i = 0; i < map->count; i++) {
-			if (map->pages[i])
+			uint32_t check, *tmp;
+			if (!map->pages[i])
+				continue;
+			/* XXX When unmapping in an HVM domain, Xen will
+			 * sometimes end up mapping the GFN to an invalid MFN.
+			 * In this case, writes will be discarded and reads will
+			 * return all 0xFF bytes.  Leak these unusable GFNs
+			 * until Xen supports fixing their p2m mapping.
+			 */
+			tmp = kmap(map->pages[i]);
+			*tmp = 0xdeaddead;
+			mb();
+			check = *tmp;
+			kunmap(map->pages[i]);
+			if (check == 0xdeaddead)
 				__free_page(map->pages[i]);
+			else
+				pr_debug("Discard page %d=%ld\n", i,
+					page_to_pfn(map->pages[i]));
 		}
+	}
 	kfree(map->pages);
 	kfree(map->grants);
 	kfree(map->map_ops);
@@ -199,17 +225,16 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
 {
 	struct grant_map *map = data;
 	unsigned int pgnr = (addr - map->vma->vm_start) >> PAGE_SHIFT;
+	int flags = map->flags | GNTMAP_application_map | GNTMAP_contains_pte;
 	u64 pte_maddr;
 
 	BUG_ON(pgnr >= map->count);
 	pte_maddr = arbitrary_virt_to_machine(pte).maddr;
 
-	gnttab_set_map_op(&map->map_ops[pgnr], pte_maddr,
-			  GNTMAP_contains_pte | map->flags,
+	gnttab_set_map_op(&map->map_ops[pgnr], pte_maddr, flags,
 			  map->grants[pgnr].ref,
 			  map->grants[pgnr].domid);
-	gnttab_set_unmap_op(&map->unmap_ops[pgnr], pte_maddr,
-			    GNTMAP_contains_pte | map->flags,
+	gnttab_set_unmap_op(&map->unmap_ops[pgnr], pte_maddr, flags,
 			    0 /* handle */);
 	return 0;
 }
@@ -217,6 +242,19 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
 static int map_grant_pages(struct grant_map *map)
 {
 	int i, err = 0;
+	phys_addr_t addr;
+
+	if (!use_ptemod) {
+		for (i = 0; i < map->count; i++) {
+			addr = (phys_addr_t)
+				pfn_to_kaddr(page_to_pfn(map->pages[i]));
+			gnttab_set_map_op(&map->map_ops[i], addr, map->flags,
+				map->grants[i].ref,
+				map->grants[i].domid);
+			gnttab_set_unmap_op(&map->unmap_ops[i], addr,
+				map->flags, 0 /* handle */);
+		}
+	}
 
 	pr_debug("map %d+%d\n", map->index, map->count);
 	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
@@ -261,17 +299,8 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
 	gntdev_put_map(map);
 }
 
-static int gntdev_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
-{
-	pr_debug("vaddr %p, pgoff %ld (shouldn''t happen)\n",
-			vmf->virtual_address, vmf->pgoff);
-	vmf->flags = VM_FAULT_ERROR;
-	return 0;
-}
-
 static struct vm_operations_struct gntdev_vmops = {
 	.close = gntdev_vma_close,
-	.fault = gntdev_vma_fault,
 };
 
 /* ------------------------------------------------------------------ */
@@ -356,14 +385,16 @@ static int gntdev_open(struct inode *inode, struct file
*flip)
 	INIT_LIST_HEAD(&priv->maps);
 	spin_lock_init(&priv->lock);
 
-	priv->mm = get_task_mm(current);
-	if (!priv->mm) {
-		kfree(priv);
-		return -ENOMEM;
+	if (use_ptemod) {
+		priv->mm = get_task_mm(current);
+		if (!priv->mm) {
+			kfree(priv);
+			return -ENOMEM;
+		}
+		priv->mn.ops = &gntdev_mmu_ops;
+		ret = mmu_notifier_register(&priv->mn, priv->mm);
+		mmput(priv->mm);
 	}
-	priv->mn.ops = &gntdev_mmu_ops;
-	ret = mmu_notifier_register(&priv->mn, priv->mm);
-	mmput(priv->mm);
 
 	if (ret) {
 		kfree(priv);
@@ -391,7 +422,8 @@ static int gntdev_release(struct inode *inode, struct file
*flip)
 	}
 	spin_unlock(&priv->lock);
 
-	mmu_notifier_unregister(&priv->mn, priv->mm);
+	if (use_ptemod)
+		mmu_notifier_unregister(&priv->mn, priv->mm);
 	kfree(priv);
 	return 0;
 }
@@ -517,7 +549,7 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	int index = vma->vm_pgoff;
 	int count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
 	struct grant_map *map;
-	int err = -EINVAL;
+	int i, err = -EINVAL;
 
 	if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags &
VM_SHARED))
 		return -EINVAL;
@@ -529,9 +561,9 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	map = gntdev_find_map_index(priv, index, count);
 	if (!map)
 		goto unlock_out;
-	if (map->vma)
+	if (use_ptemod && map->vma)
 		goto unlock_out;
-	if (priv->mm != vma->vm_mm) {
+	if (use_ptemod && priv->mm != vma->vm_mm) {
 		printk(KERN_WARNING "Huh? Other mm?\n");
 		goto unlock_out;
 	}
@@ -543,20 +575,24 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 	vma->vm_flags |= VM_RESERVED|VM_DONTCOPY|VM_DONTEXPAND|VM_PFNMAP;
 
 	vma->vm_private_data = map;
-	map->vma = vma;
 
-	map->flags = GNTMAP_host_map | GNTMAP_application_map;
+	if (use_ptemod)
+		map->vma = vma;
+
+	map->flags = GNTMAP_host_map;
 	if (!(vma->vm_flags & VM_WRITE))
 		map->flags |= GNTMAP_readonly;
 
 	spin_unlock(&priv->lock);
 
-	err = apply_to_page_range(vma->vm_mm, vma->vm_start,
-				  vma->vm_end - vma->vm_start,
-				  find_grant_ptes, map);
-	if (err) {
-		printk(KERN_WARNING "find_grant_ptes() failure.\n");
-		return err;
+	if (use_ptemod) {
+		err = apply_to_page_range(vma->vm_mm, vma->vm_start,
+					  vma->vm_end - vma->vm_start,
+					  find_grant_ptes, map);
+		if (err) {
+			printk(KERN_WARNING "find_grant_ptes() failure.\n");
+			return err;
+		}
 	}
 
 	err = map_grant_pages(map);
@@ -567,6 +603,15 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
 
 	map->is_mapped = 1;
 
+	if (!use_ptemod) {
+		for(i = 0; i < count; i++) {
+			err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE,
+			                     map->pages[i]);
+			if (err)
+				return err;
+		}
+	}
+
 	return 0;
 
 unlock_out:
@@ -596,6 +641,8 @@ static int __init gntdev_init(void)
 
 	if (!xen_domain())
 		return -ENODEV;
+	
+	use_ptemod = xen_pv_domain();
 
 	err = misc_register(&gntdev_miscdev);
 	if (err != 0) {
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 9ef54eb..9428ced 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -458,6 +458,9 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 	if (ret)
 		return ret;
 
+	if (xen_feature(XENFEAT_auto_translated_physmap))
+		return ret;
+
 	for (i = 0; i < count; i++) {
 		/* m2p override only supported for GNTMAP_contains_pte mappings */
 		if (!(map_ops[i].flags & GNTMAP_contains_pte))
@@ -483,6 +486,9 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref
*unmap_ops,
 	if (ret)
 		return ret;
 
+	if (xen_feature(XENFEAT_auto_translated_physmap))
+		return ret;
+
 	for (i = 0; i < count; i++) {
 		ret = m2p_remove_override(pages[i]);
 		if (ret)
-- 
1.7.3.4


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

Konrad Rzeszutek Wilk

2011-Jan-21 15:02 UTC

head link

[SPAM] Re: [Xen-devel] [PATCH 6/6] xen-gntalloc: Userspace grant allocation driver

On Fri, Jan 14, 2011 at 10:46:16AM -0500, Daniel De Graaf
wrote:> This allows a userspace application to allocate a shared page for
> implementing inter-domain communication or device drivers. These
> shared pages can be mapped using the gntdev device or by the kernel
> in another domain.
I ran this through checkpatch.pl and did it complain. Can you fix that
up and resend it please?> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/Kconfig    |    7 +
>  drivers/xen/Makefile   |    2 +
>  drivers/xen/gntalloc.c |  488
++++++++++++++++++++++++++++++++++++++++++++++++
>  include/xen/gntalloc.h |   70 +++++++
>  4 files changed, 567 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/xen/gntalloc.c
>  create mode 100644 include/xen/gntalloc.h
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 07bec09..1fc06d3 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -79,6 +79,13 @@ config XEN_GNTDEV
>  	select MMU_NOTIFIER
>  	help
>  	  Allows userspace processes to use grants.
> +	  
> +config XEN_GRANT_DEV_ALLOC
> +	tristate "User-space grant reference allocator driver"
> +	depends on XEN
> +	help
> +	  Allows userspace processes to create pages with access granted
> +	  to other domains.
>  
>  config XEN_PLATFORM_PCI
>  	tristate "xen platform pci device driver"
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 5088cc2..9585a1d 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_XEN_XENCOMM)	+= xencomm.o
>  obj-$(CONFIG_XEN_BALLOON)	+= balloon.o
>  obj-$(CONFIG_XEN_DEV_EVTCHN)	+= xen-evtchn.o
>  obj-$(CONFIG_XEN_GNTDEV)	+= xen-gntdev.o
> +obj-$(CONFIG_XEN_GRANT_DEV_ALLOC)	+= xen-gntalloc.o
>  obj-$(CONFIG_XENFS)		+= xenfs/
>  obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
>  obj-$(CONFIG_XEN_PLATFORM_PCI)	+= xen-platform-pci.o
> @@ -18,5 +19,6 @@ obj-$(CONFIG_XEN_DOM0)		+= pci.o
>  
>  xen-evtchn-y			:= evtchn.o
>  xen-gntdev-y				:= gntdev.o
> +xen-gntalloc-y				:= gntalloc.o
>  
>  xen-platform-pci-y		:= platform-pci.o
> diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
> new file mode 100644
> index 0000000..4bee149
> --- /dev/null
> +++ b/drivers/xen/gntalloc.c
> @@ -0,0 +1,488 @@
>
+/******************************************************************************
> + * gntalloc.c
> + *
> + * Device for creating grant references (in user-space) that may be shared
> + * with other domains.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 
USA
> + */
> +
> +/*
> + * This driver exists to allow userspace programs in Linux to allocate
kernel
> + * memory that will later be shared with another domain.  Without this
device,
> + * Linux userspace programs cannot create grant references.
> + *
> + * How this stuff works:
> + *   X -> granting a page to Y
> + *   Y -> mapping the grant from X
> + *
> + *   1. X uses the gntalloc device to allocate a page of kernel memory, P.
> + *   2. X creates an entry in the grant table that says domid(Y) can
access P.
> + *      This is done without a hypercall unless the grant table needs
expansion.
> + *   3. X gives the grant reference identifier, GREF, to Y.
> + *   4. Y maps the page, either directly into kernel memory for use in a
backend
> + *      driver, or via a the gntdev device to map into the address space
of an
> + *      application running in Y. This is the first point at which Xen
does any
> + *      tracking of the page.
> + *   5. A program in X mmap()s a segment of the gntalloc device that
corresponds
> + *      to the shared page, and can now communicate with Y over the shared
page.
> + *
> + *
> + * NOTE TO USERSPACE LIBRARIES:
> + *   The grant allocation and mmap()ing are, naturally, two separate
operations.
> + *   You set up the sharing by calling the create ioctl() and then the
mmap().
> + *   Teardown requires munmap() and either close() or ioctl().
> + *
> + * WARNING: Since Xen does not allow a guest to forcibly end the use of a
grant
> + * reference, this device can be used to consume kernel memory by leaving
grant
> + * references mapped by another domain when an application exits.
Therefore,
> + * there is a global limit on the number of pages that can be allocated.
When
> + * all references to the page are unmapped, it will be freed during the
next
> + * grant operation.
> + */
> +
> +#include <asm/atomic.h>
> +#include <linux/module.h>
> +#include <linux/miscdevice.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/device.h>
> +#include <linux/mm.h>
> +#include <asm/uaccess.h>
> +#include <linux/types.h>
> +#include <linux/list.h>
> +
> +#include <xen/xen.h>
> +#include <xen/page.h>
> +#include <xen/grant_table.h>
> +#include <xen/gntalloc.h>
> +
> +static int debug = 0;
> +module_param(debug, int, 0644);
> +
> +static int limit = 1024;
> +module_param(limit, int, 0644);
> +
> +static LIST_HEAD(gref_list);
> +static DEFINE_SPINLOCK(gref_lock);
> +static int gref_size = 0;
> +
> +/* Metadata on a grant reference. */
> +struct gntalloc_gref {
> +	struct list_head next_all;   /* list entry gref_list */
> +	struct list_head next_file;  /* list entry file->list, if open */
> +	struct page* page;	     /* The shared page */
> +	uint64_t file_index;         /* File offset for mmap() */
> +	unsigned int users;          /* Use count - when zero, waiting on Xen */
> +	grant_ref_t gref_id;         /* The grant reference number */
> +};
> +
> +struct gntalloc_file_private_data {
> +	struct list_head list;
> +	uint64_t index;
> +};
> +
> +static void __del_gref(struct gntalloc_gref *gref);
> +
> +static void do_cleanup(void)
> +{
> +	struct gntalloc_gref *gref, *n;
> +	list_for_each_entry_safe(gref, n, &gref_list, next_all) {
> +		if (!gref->users)
> +			__del_gref(gref);
> +	}
> +}
> +
> +static int add_grefs(struct ioctl_gntalloc_alloc_gref* op,
> +	uint32_t* gref_ids, struct gntalloc_file_private_data *priv)
> +{
> +	int i, rc, readonly;
> +	LIST_HEAD(queue_all);
> +	LIST_HEAD(queue_file);
> +	struct gntalloc_gref *gref;
> +
> +	readonly = !(op->flags & GNTALLOC_FLAG_WRITABLE);
> +	rc = -ENOMEM;
> +	for(i=0; i < op->count; i++) {
> +		gref = kzalloc(sizeof(*gref), GFP_KERNEL);
> +		if (!gref)
> +			goto undo;
> +		list_add_tail(&gref->next_all, &queue_all);
> +		list_add_tail(&gref->next_file, &queue_file);
> +		gref->users = 1;
> +		gref->file_index = op->index + i * PAGE_SIZE;
> +		gref->page = alloc_page(GFP_KERNEL|__GFP_ZERO);
> +		if (!gref->page)
> +			goto undo;
> +
> +		/* Grant foreign access to the page. */
> +		gref->gref_id = gnttab_grant_foreign_access(op->domid,
> +			pfn_to_mfn(page_to_pfn(gref->page)), readonly);
> +		if (gref->gref_id < 0) {
> +			rc = gref->gref_id;
> +			goto undo;
> +		}
> +		gref_ids[i] = gref->gref_id;
> +	}
> +
> +	/* Add to gref lists. */
> +	spin_lock(&gref_lock);
> +	list_splice_tail(&queue_all, &gref_list);
> +	list_splice_tail(&queue_file, &priv->list);
> +	spin_unlock(&gref_lock);
> +
> +	return 0;
> +
> +undo:
> +	list_for_each_entry(gref, &queue_file, next_file) {
> +		if (gref->page && gref->gref_id >= 0) {
> +			__del_gref(gref);
> +		} else {
> +			if (gref->page)
> +				__free_page(gref->page);
> +			list_del(&gref->next_all);
> +			kfree(gref);
> +		}
> +	}
> +
> +	/* It''s possible for the target domain to map the just-allocated
grant
> +	 * references by blindly guessing their IDs; if this is done, then
> +	 * __del_gref will leave them in the queue_all list. They need to be
> +	 * added to the global list so that we can free them when they are no
> +	 * longer referenced.
> +	 */
> +	if (unlikely(!list_empty(&queue_all))) {
> +		spin_lock(&gref_lock);
> +		list_splice_tail(&queue_all, &gref_list);
> +		spin_unlock(&gref_lock);
> +	}
> +	return rc;
> +}
> +
> +static void __del_gref(struct gntalloc_gref *gref)
> +{
> +	if (gnttab_query_foreign_access(gref->gref_id))
> +		return;
> +
> +	if (!gnttab_end_foreign_access_ref(gref->gref_id, 0))
> +		return;
> +
> +	gref_size--;
> +	list_del(&gref->next_all);
> +
> +	__free_page(gref->page);
> +	kfree(gref);
> +}
> +
> +// finds contiguous grant references in a file, returns the first
> +static struct gntalloc_gref* find_grefs(struct gntalloc_file_private_data
*priv,
> +                                       uint64_t index, uint32_t count)
> +{
> +	struct gntalloc_gref *rv = NULL, *gref;
> +	list_for_each_entry(gref, &priv->list, next_file) {
> +		if (gref->file_index == index && !rv)
> +			rv = gref;
> +		if (rv) {
> +			if (gref->file_index != index)
> +				return NULL;
> +			index += PAGE_SIZE;
> +			count--;
> +			if (count == 0)
> +				return rv;
> +		}
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * -------------------------------------
> + *  File operations.
> + * -------------------------------------
> + */
> +static int gntalloc_open(struct inode *inode, struct file *filp)
> +{
> +	struct gntalloc_file_private_data *priv;
> +
> +	try_module_get(THIS_MODULE);
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		goto out_nomem;
> +	INIT_LIST_HEAD(&priv->list);
> +
> +	filp->private_data = priv;
> +
> +	if (debug)
> +		printk("%s: priv %p\n", __FUNCTION__, priv);
> +
> +	return 0;
> +
> +out_nomem:
> +	return -ENOMEM;
> +}
> +
> +static int gntalloc_release(struct inode *inode, struct file *filp)
> +{
> +	struct gntalloc_file_private_data *priv = filp->private_data;
> +	struct gntalloc_gref *gref;
> +
> +	if (debug)
> +		printk("%s: priv %p\n", __FUNCTION__, priv);
> +
> +	spin_lock(&gref_lock);
> +	while (!list_empty(&priv->list)) {
> +		gref = list_entry(priv->list.next,
> +			struct gntalloc_gref, next_file);
> +		list_del(&gref->next_file);
> +		gref->users--;
> +		if (gref->users == 0)
> +			__del_gref(gref);
> +	}
> +	kfree(priv);
> +	spin_unlock(&gref_lock);
> +
> +	module_put(THIS_MODULE);
> +
> +	return 0;
> +}
> +
> +static long gntalloc_ioctl_alloc(struct gntalloc_file_private_data *priv,
> +                                 struct ioctl_gntalloc_alloc_gref __user
*arg)
> +{
> +	int rc = 0;
> +	struct ioctl_gntalloc_alloc_gref op;
> +	uint32_t* gref_ids;
> +
> +	if (debug)
> +		printk("%s: priv %p\n", __FUNCTION__, priv);
> +
> +	if (copy_from_user(&op, arg, sizeof(op))) {
> +		rc = -EFAULT;
> +		goto out;
> +	}
> +
> +	gref_ids = kzalloc(sizeof(gref_ids[0]) * op.count, GFP_TEMPORARY);
> +	if (!gref_ids) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	spin_lock(&gref_lock);
> +	do_cleanup();
> +	if (gref_size + op.count > limit) {
> +		spin_unlock(&gref_lock);
> +		rc = -ENOSPC;
> +		goto out_free;
> +	}
> +	gref_size += op.count;
> +	op.index = priv->index;
> +	priv->index += op.count * PAGE_SIZE;
> +	spin_unlock(&gref_lock);
> +
> +	rc = add_grefs(&op, gref_ids, priv);
> +	if (rc < 0)
> +		goto out_free;
> +
> +	if (copy_to_user(arg, &op, sizeof(op))) {
> +		rc = -EFAULT;
> +		goto out_free;
> +	}
> +	if (copy_to_user(arg->gref_ids, gref_ids,
> +	                   sizeof(gref_ids[0]) * op.count)) {
> +		rc = -EFAULT;
> +		goto out_free;
> +	}
> +
> +out_free:
> +	kfree(gref_ids);
> +out:
> +	return rc;
> +}
> +
> +static long gntalloc_ioctl_dealloc(struct gntalloc_file_private_data
*priv,
> +                                   void __user *arg)
> +{
> +	int i, rc = 0;
> +	struct ioctl_gntalloc_dealloc_gref op;
> +	struct gntalloc_gref *gref, *n;
> +
> +	if (debug)
> +		printk("%s: priv %p\n", __FUNCTION__, priv);
> +
> +	if (copy_from_user(&op, arg, sizeof(op))) {
> +		rc = -EFAULT;
> +		goto dealloc_grant_out;
> +	}
> +
> +	spin_lock(&gref_lock);
> +	gref = find_grefs(priv, op.index, op.count);
> +	if (gref) {
> +		for(i=0; i < op.count; i++) {
> +			n = list_entry(gref->next_file.next,
> +			               struct gntalloc_gref, next_file);
> +			list_del(&gref->next_file);
> +			gref->users--;
> +			gref = n;
> +		}
> +	} else {
> +		rc = -EINVAL;
> +	}
> +
> +	do_cleanup();
> +
> +	spin_unlock(&gref_lock);
> +dealloc_grant_out:
> +	return rc;
> +}
> +
> +static long gntalloc_ioctl(struct file *filp, unsigned int cmd,
> +		unsigned long arg)
> +{
> +	struct gntalloc_file_private_data *priv = filp->private_data;
> +
> +	switch (cmd) {
> +	case IOCTL_GNTALLOC_ALLOC_GREF:
> +		return gntalloc_ioctl_alloc(priv, (void __user*)arg);
> +
> +	case IOCTL_GNTALLOC_DEALLOC_GREF:
> +		return gntalloc_ioctl_dealloc(priv, (void __user*)arg);
> +
> +	default:
> +		return -ENOIOCTLCMD;
> +	}
> +
> +	return 0;
> +}
> +
> +static void gntalloc_vma_close(struct vm_area_struct *vma)
> +{
> +	struct gntalloc_gref *gref = vma->vm_private_data;
> +	if (!gref)
> +		return;
> +
> +	spin_lock(&gref_lock);
> +	gref->users--;
> +	if (gref->users == 0)
> +		__del_gref(gref);
> +	spin_unlock(&gref_lock);
> +}
> +
> +static struct vm_operations_struct gntalloc_vmops = {
> +	.close = gntalloc_vma_close,
> +};
> +
> +static int gntalloc_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	struct gntalloc_file_private_data *priv = filp->private_data;
> +	struct gntalloc_gref *gref;
> +	int count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +	int rv, i;
> +
> +	if (debug)
> +		printk("%s: priv %p, page %lu+%d\n", __func__,
> +		       priv, vma->vm_pgoff, count);
> +
> +	if (!(vma->vm_flags & VM_SHARED)) {
> +		printk(KERN_ERR "%s: Mapping must be shared.\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	spin_lock(&gref_lock);
> +	gref = find_grefs(priv, vma->vm_pgoff << PAGE_SHIFT, count);
> +	if (gref == NULL) {
> +		rv = -ENOENT;
> +		if (debug)
> +			printk("%s: Could not find grant reference",
> +				__func__);
> +		goto out_unlock;
> +	}
> +
> +	vma->vm_private_data = gref;
> +
> +	vma->vm_flags |= VM_RESERVED;
> +	vma->vm_flags |= VM_DONTCOPY;
> +	vma->vm_flags |= VM_IO;
> +	vma->vm_flags |= VM_PFNMAP | VM_PFN_AT_MMAP;
> +
> +	vma->vm_ops = &gntalloc_vmops;
> +
> +	for(i=0; i < count; i++) {
> +		gref->users++;
> +		rv = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE,
> +		                    gref->page);
> +		if (rv)
> +			goto out_unlock;
> +
> +		gref = list_entry(gref->next_file.next,
> +		                  struct gntalloc_gref, next_file);
> +	}
> +	rv = 0;
> +
> +out_unlock:
> +	spin_unlock(&gref_lock);
> +	return rv;
> +}
> +
> +static const struct file_operations gntalloc_fops = {
> +	.owner = THIS_MODULE,
> +	.open = gntalloc_open,
> +	.release = gntalloc_release,
> +	.unlocked_ioctl = gntalloc_ioctl,
> +	.mmap = gntalloc_mmap
> +};
> +
> +/*
> + * -------------------------------------
> + * Module creation/destruction.
> + * -------------------------------------
> + */
> +static struct miscdevice gntalloc_miscdev = {
> +	.minor	= MISC_DYNAMIC_MINOR,
> +	.name	= "xen/gntalloc",
> +	.fops	= &gntalloc_fops,
> +};
> +
> +static int __init gntalloc_init(void)
> +{
> +	int err;
> +
> +	if (!xen_domain()) {
> +		if (debug)
> +			printk(KERN_ERR "gntalloc: You must be running Xen\n");
> +		return -ENODEV;
> +	}
> +
> +	err = misc_register(&gntalloc_miscdev);
> +	if (err != 0) {
> +		printk(KERN_ERR "Could not register misc gntalloc device\n");
> +		return err;
> +	}
> +
> +	if (debug)
> +		printk(KERN_INFO "Created grant allocation device at %d,%d\n",
> +			MISC_MAJOR, gntalloc_miscdev.minor);
> +
> +	return 0;
> +}
> +
> +static void __exit gntalloc_exit(void)
> +{
> +	misc_deregister(&gntalloc_miscdev);
> +}
> +
> +module_init(gntalloc_init);
> +module_exit(gntalloc_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Carter Weatherly <carter.weatherly@jhuapl.edu>,
"
> +              "Daniel De Graaf <dgdegra@tycho.nsa.gov>");
> +MODULE_DESCRIPTION("User-space grant reference allocator
driver");
> diff --git a/include/xen/gntalloc.h b/include/xen/gntalloc.h
> new file mode 100644
> index 0000000..e1d6d0f
> --- /dev/null
> +++ b/include/xen/gntalloc.h
> @@ -0,0 +1,70 @@
>
+/******************************************************************************
> + * gntalloc.h
> + *
> + * Interface to /dev/xen/gntalloc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation; or, when distributed
> + * separately from the Linux kernel or incorporated into other
> + * software packages, subject to the following license:
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
copy
> + * of this source file (the "Software"), to deal in the Software
without
> + * restriction, including without limitation the rights to use, copy,
modify,
> + * merge, publish, distribute, sublicense, and/or sell copies of the
Software,
> + * and to permit persons to whom the Software is furnished to do so,
subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included
in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifndef __LINUX_PUBLIC_GNTALLOC_H__
> +#define __LINUX_PUBLIC_GNTALLOC_H__
> +
> +/*
> + * Allocates a new page and creates a new grant reference.
> + */
> +#define IOCTL_GNTALLOC_ALLOC_GREF \
> +_IOC(_IOC_NONE, ''G'', 5, sizeof(struct
ioctl_gntalloc_alloc_gref))
> +struct ioctl_gntalloc_alloc_gref {
> +	/* IN parameters */
> +	/* The ID of the domain to be given access to the grants. */
> +	uint16_t domid;
> +	/* Flags for this mapping */
> +	uint16_t flags;
> +	/* Number of pages to map */
> +	uint32_t count;
> +	/* OUT parameters */
> +	/* The offset to be used on a subsequent call to mmap(). */
> +	uint64_t index;
> +	/* The grant references of the newly created grant, one per page */
> +	/* Variable size, depending on count */
> +	uint32_t gref_ids[1];
> +};
> +
> +#define GNTALLOC_FLAG_WRITABLE 1
> +
> +/*
> + * Deallocates the grant reference, allowing the associated page to be
freed if
> + * no other domains are using it.
> + */
> +#define IOCTL_GNTALLOC_DEALLOC_GREF \
> +_IOC(_IOC_NONE, ''G'', 6, sizeof(struct
ioctl_gntalloc_dealloc_gref))
> +struct ioctl_gntalloc_dealloc_gref {
> +	/* IN parameters */
> +	/* The offset returned in the map operation */
> +	uint64_t index;
> +	/* Number of references to unmap */
> +	uint32_t count;
> +};
> +#endif /* __LINUX_PUBLIC_GNTALLOC_H__ */
> -- 
> 1.7.3.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Konrad Rzeszutek Wilk

2011-Jan-21 15:04 UTC

head link

Re: [Xen-devel] [PATCH 1/6 v2] xen-gntdev: Change page limit to be global instead of per-open

On Tue, Jan 18, 2011 at 12:57:19PM -0500, Daniel De Graaf
wrote:> Because there is no limitation on how many times a user can open a
> given device file, an per-file-description limit on the number of
> pages granted offers little to no benefit. Change to a global limit
> and remove the ioctl() as the parameter can now be changed via sysfs.
> 
> Xen tools changeset 22768:f8d801e5573e is needed to eliminate the
> error this change produces in xc_gnttab_set_max_grants.
Can you also run this through the checkpath.pl tool and fix up the
failures, please?
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/gntdev.c |   52
+++++++++++++++----------------------------------
>  1 files changed, 16 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 1e31cdc..59e6a51 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -45,15 +45,15 @@ MODULE_AUTHOR("Derek G. Murray
<Derek.Murray@cl.cam.ac.uk>, "
>  	      "Gerd Hoffmann <kraxel@redhat.com>");
>  MODULE_DESCRIPTION("User-space granted page access driver");
>  
> -static int limit = 1024;
> +static int limit = 1024*1024;
>  module_param(limit, int, 0644);
> -MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped
at "
> -		"once by a gntdev instance");
> +MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped
by "
> +		"the gntdev device");
> +
> +static atomic_t pages_mapped = ATOMIC_INIT(0);
>  
>  struct gntdev_priv {
>  	struct list_head maps;
> -	uint32_t used;
> -	uint32_t limit;
>  	/* lock protects maps from concurrent changes */
>  	spinlock_t lock;
>  	struct mm_struct *mm;
> @@ -82,9 +82,8 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
>  #ifdef DEBUG
>  	struct grant_map *map;
>  
> -	pr_debug("maps list (priv %p, usage %d/%d)\n",
> -	       priv, priv->used, priv->limit);
> -
> +	pr_debug("%s: maps list (priv %p)\n",
> +	       __FUNCTION__, priv);
>  	list_for_each_entry(map, &priv->maps, next)
>  		pr_debug("  index %2d, count %2d %s\n",
>  		       map->index, map->count,
> @@ -121,9 +120,6 @@ static struct grant_map *gntdev_alloc_map(struct
gntdev_priv *priv, int count)
>  	add->count = count;
>  	add->priv  = priv;
>  
> -	if (add->count + priv->used > priv->limit)
> -		goto err;
> -
>  	return add;
>  
>  err:
> @@ -154,7 +150,6 @@ static void gntdev_add_map(struct gntdev_priv *priv,
struct grant_map *add)
>  	list_add_tail(&add->next, &priv->maps);
>  
>  done:
> -	priv->used += add->count;
>  	gntdev_print_maps(priv, "[new]", add->index);
>  }
>  
> @@ -200,7 +195,7 @@ static int gntdev_del_map(struct grant_map *map)
>  		if (map->unmap_ops[i].handle)
>  			return -EBUSY;
>  
> -	map->priv->used -= map->count;
> +	atomic_sub(map->count, &pages_mapped);
>  	list_del(&map->next);
>  	return 0;
>  }
> @@ -386,7 +381,6 @@ static int gntdev_open(struct inode *inode, struct file
*flip)
>  
>  	INIT_LIST_HEAD(&priv->maps);
>  	spin_lock_init(&priv->lock);
> -	priv->limit = limit;
>  
>  	priv->mm = get_task_mm(current);
>  	if (!priv->mm) {
> @@ -443,19 +437,25 @@ static long gntdev_ioctl_map_grant_ref(struct
gntdev_priv *priv,
>  	pr_debug("priv %p, add %d\n", priv, op.count);
>  	if (unlikely(op.count <= 0))
>  		return -EINVAL;
> -	if (unlikely(op.count > priv->limit))
> -		return -EINVAL;
>  
>  	err = -ENOMEM;
>  	map = gntdev_alloc_map(priv, op.count);
>  	if (!map)
>  		return err;
> +
>  	if (copy_from_user(map->grants, &u->refs,
>  			   sizeof(map->grants[0]) * op.count) != 0) {
>  		gntdev_free_map(map);
>  		return err;
>  	}
>  
> +	if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
> +	{
> +		pr_debug("%s: can''t map: over limit\n",
__FUNCTION__);
> +		gntdev_free_map(map);
> +		return err;
> +	}
> +
>  	spin_lock(&priv->lock);
>  	gntdev_add_map(priv, map);
>  	op.index = map->index << PAGE_SHIFT;
> @@ -518,23 +518,6 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
gntdev_priv *priv,
>  	return 0;
>  }
>  
> -static long gntdev_ioctl_set_max_grants(struct gntdev_priv *priv,
> -					struct ioctl_gntdev_set_max_grants __user *u)
> -{
> -	struct ioctl_gntdev_set_max_grants op;
> -
> -	if (copy_from_user(&op, u, sizeof(op)) != 0)
> -		return -EFAULT;
> -	pr_debug("priv %p, limit %d\n", priv, op.count);
> -	if (op.count > limit)
> -		return -E2BIG;
> -
> -	spin_lock(&priv->lock);
> -	priv->limit = op.count;
> -	spin_unlock(&priv->lock);
> -	return 0;
> -}
> -
>  static long gntdev_ioctl(struct file *flip,
>  			 unsigned int cmd, unsigned long arg)
>  {
> @@ -551,9 +534,6 @@ static long gntdev_ioctl(struct file *flip,
>  	case IOCTL_GNTDEV_GET_OFFSET_FOR_VADDR:
>  		return gntdev_ioctl_get_offset_for_vaddr(priv, ptr);
>  
> -	case IOCTL_GNTDEV_SET_MAX_GRANTS:
> -		return gntdev_ioctl_set_max_grants(priv, ptr);
> -
>  	default:
>  		pr_debug("priv %p, unknown cmd %x\n", priv, cmd);
>  		return -ENOIOCTLCMD;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Jan 2011 - [PATCH v4] Userspace grant communication

[Xen-devel] [PATCH v4] Userspace grant communication

[Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

[Xen-devel] [PATCH 2/6] xen-gntdev: Remove unneeded structures from grant_map tracking data

[Xen-devel] [PATCH 3/6] xen-gntdev: Use find_vma rather than iterating our vma list manually

[Xen-devel] [PATCH 4/6] xen-gntdev: Add reference counting to maps

[Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

[Xen-devel] [PATCH 6/6] xen-gntalloc: Userspace grant allocation driver

Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

[Xen-devel] [PATCH 1/6 v2] xen-gntdev: Change page limit to be global instead of per-open

[Xen-devel] Re: [PATCH 2/6] xen-gntdev: Remove unneeded structures from grant_map tracking data

Re: [Xen-devel] [PATCH 4/6] xen-gntdev: Add reference counting to maps

[Xen-devel] [PATCH v4.2 3/5] xen-gntdev: Add reference counting to maps

[Xen-devel] [PATCH v4.2 4/5] xen-gntdev: Support mapping in HVM domains

[SPAM] Re: [Xen-devel] [PATCH 6/6] xen-gntalloc: Userspace grant allocation driver

Re: [Xen-devel] [PATCH 1/6 v2] xen-gntdev: Change page limit to be global instead of per-open