From ef91082e90491ac99343a13f9aeff4669835c6cc Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Thu, 3 Feb 2011 12:18:59 -0500 Subject: 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 Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 50 ++++++++++++++------------------------------------ 1 file changed, 14 insertions(+), 36 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 1e31cdcdae1..23d208a219f 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -45,15 +45,15 @@ MODULE_AUTHOR("Derek G. Murray , " "Gerd Hoffmann "); 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,7 @@ 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", __func__, priv); list_for_each_entry(map, &priv->maps, next) pr_debug(" index %2d, count %2d %s\n", map->index, map->count, @@ -121,9 +119,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 +149,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 +194,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 +380,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 +436,24 @@ 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("can't map: over limit\n"); + gntdev_free_map(map); + return err; + } + spin_lock(&priv->lock); gntdev_add_map(priv, map); op.index = map->index << PAGE_SHIFT; @@ -518,23 +516,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 +532,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; -- cgit v1.2.3-70-g09d2 From a879211bf1d70339e429603805c014450c275f2a Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Thu, 3 Feb 2011 12:19:00 -0500 Subject: 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 Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 23d208a219f..ce8c37c2b67 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -167,23 +167,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; @@ -494,22 +477,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; -- cgit v1.2.3-70-g09d2 From 68b025c813c2eb41ff25628e3d4952d5185eb1a4 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Thu, 3 Feb 2011 12:19:01 -0500 Subject: 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 Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 67 +++++++++++++++++++++------------------------------- 1 file changed, 27 insertions(+), 40 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index ce8c37c2b67..256162b5669 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; @@ -117,7 +117,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; @@ -167,28 +167,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; - 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++) { if (map->pages[i]) @@ -267,6 +257,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) @@ -388,17 +379,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); @@ -425,15 +413,15 @@ 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); + if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit)) { + pr_debug("can't map: over limit\n"); + gntdev_put_map(map); return err; } - if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit)) { - pr_debug("can't map: over limit\n"); - gntdev_free_map(map); + if (copy_from_user(map->grants, &u->refs, + sizeof(map->grants[0]) * op.count) != 0) { + gntdev_put_map(map); return err; } @@ -442,13 +430,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; } @@ -465,11 +449,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; } @@ -549,6 +534,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; -- cgit v1.2.3-70-g09d2 From aab8f11a6b4641fcb8c139420f2eae879b5d1698 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Thu, 3 Feb 2011 12:19:02 -0500 Subject: 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. Note: The HVM path for map and unmap is slightly different: HVM keeps the pages mapped until the area is deleted, while the PV case (use_ptemod being true) must unmap them when userspace unmaps the range. In the normal use case, this makes no difference to users since unmap time is deletion time. [v2: Expanded commit descr.] Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 117 ++++++++++++++++++++++++++++++++-------------- drivers/xen/grant-table.c | 6 +++ 2 files changed, 89 insertions(+), 34 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 256162b5669..bcaf797216d 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -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; + 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, @@ -179,11 +184,34 @@ 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. + * + * Confirmed present in Xen 4.1-RC3 with HVM source + */ + 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); @@ -198,17 +226,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; } @@ -216,6 +243,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); @@ -260,17 +300,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, }; /* ------------------------------------------------------------------ */ @@ -355,14 +386,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); @@ -390,7 +423,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; } @@ -515,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; @@ -527,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; } @@ -541,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); @@ -565,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: @@ -595,6 +642,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 9ef54ebc119..9428ced0480 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) -- cgit v1.2.3-70-g09d2 From bdc612dc6903c4ea06e40d02f84ad5e25d93459d Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Thu, 3 Feb 2011 12:19:04 -0500 Subject: xen/gntalloc,gntdev: Add unmap notify ioctl This ioctl allows the users of a shared page to be notified when the other end exits abnormally. [v2: updated description in structs] Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntalloc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/xen/gntdev.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++- include/xen/gntalloc.h | 32 ++++++++++++++++++++++++++ include/xen/gntdev.h | 31 +++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 1 deletion(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c index d06bf2b4cd0..a7ffdfe19fc 100644 --- a/drivers/xen/gntalloc.c +++ b/drivers/xen/gntalloc.c @@ -60,11 +60,13 @@ #include #include #include +#include #include #include #include #include +#include static int limit = 1024; module_param(limit, int, 0644); @@ -75,6 +77,12 @@ static LIST_HEAD(gref_list); static DEFINE_SPINLOCK(gref_lock); static int gref_size; +struct notify_info { + uint16_t pgoff:12; /* Bits 0-11: Offset of the byte to clear */ + uint16_t flags:2; /* Bits 12-13: Unmap notification flags */ + int event; /* Port (event channel) to notify */ +}; + /* Metadata on a grant reference. */ struct gntalloc_gref { struct list_head next_gref; /* list entry gref_list */ @@ -83,6 +91,7 @@ struct gntalloc_gref { 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 notify_info notify; /* Unmap notification */ }; struct gntalloc_file_private_data { @@ -164,6 +173,16 @@ undo: static void __del_gref(struct gntalloc_gref *gref) { + if (gref->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) { + uint8_t *tmp = kmap(gref->page); + tmp[gref->notify.pgoff] = 0; + kunmap(gref->page); + } + if (gref->notify.flags & UNMAP_NOTIFY_SEND_EVENT) + notify_remote_via_evtchn(gref->notify.event); + + gref->notify.flags = 0; + if (gref->gref_id > 0) { if (gnttab_query_foreign_access(gref->gref_id)) return; @@ -349,6 +368,43 @@ dealloc_grant_out: return rc; } +static long gntalloc_ioctl_unmap_notify(struct gntalloc_file_private_data *priv, + void __user *arg) +{ + struct ioctl_gntalloc_unmap_notify op; + struct gntalloc_gref *gref; + uint64_t index; + int pgoff; + int rc; + + if (copy_from_user(&op, arg, sizeof(op))) + return -EFAULT; + + index = op.index & ~(PAGE_SIZE - 1); + pgoff = op.index & (PAGE_SIZE - 1); + + spin_lock(&gref_lock); + + gref = find_grefs(priv, index, 1); + if (!gref) { + rc = -ENOENT; + goto unlock_out; + } + + if (op.action & ~(UNMAP_NOTIFY_CLEAR_BYTE|UNMAP_NOTIFY_SEND_EVENT)) { + rc = -EINVAL; + goto unlock_out; + } + + gref->notify.flags = op.action; + gref->notify.pgoff = pgoff; + gref->notify.event = op.event_channel_port; + rc = 0; + unlock_out: + spin_unlock(&gref_lock); + return rc; +} + static long gntalloc_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { @@ -361,6 +417,9 @@ static long gntalloc_ioctl(struct file *filp, unsigned int cmd, case IOCTL_GNTALLOC_DEALLOC_GREF: return gntalloc_ioctl_dealloc(priv, (void __user *)arg); + case IOCTL_GNTALLOC_SET_UNMAP_NOTIFY: + return gntalloc_ioctl_unmap_notify(priv, (void __user *)arg); + default: return -ENOIOCTLCMD; } diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index bcaf797216d..9694a1a8b2e 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -63,6 +64,13 @@ struct gntdev_priv { struct mmu_notifier mn; }; +struct unmap_notify { + int flags; + /* Address relative to the start of the grant_map */ + int addr; + int event; +}; + struct grant_map { struct list_head next; struct vm_area_struct *vma; @@ -71,6 +79,7 @@ struct grant_map { int flags; int is_mapped; atomic_t users; + struct unmap_notify notify; struct ioctl_gntdev_grant_ref *grants; struct gnttab_map_grant_ref *map_ops; struct gnttab_unmap_grant_ref *unmap_ops; @@ -165,7 +174,7 @@ static struct grant_map *gntdev_find_map_index(struct gntdev_priv *priv, list_for_each_entry(map, &priv->maps, next) { if (map->index != index) continue; - if (map->count != count) + if (count && map->count != count) continue; return map; } @@ -184,6 +193,10 @@ static void gntdev_put_map(struct grant_map *map) atomic_sub(map->count, &pages_mapped); + if (map->notify.flags & UNMAP_NOTIFY_SEND_EVENT) { + notify_remote_via_evtchn(map->notify.event); + } + if (map->pages) { if (!use_ptemod) unmap_grant_pages(map, 0, map->count); @@ -274,6 +287,16 @@ static int unmap_grant_pages(struct grant_map *map, int offset, int pages) { int i, err = 0; + if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) { + int pgno = (map->notify.addr >> PAGE_SHIFT); + if (pgno >= offset && pgno < offset + pages) { + uint8_t *tmp = kmap(map->pages[pgno]); + tmp[map->notify.addr & (PAGE_SIZE-1)] = 0; + kunmap(map->pages[pgno]); + map->notify.flags &= ~UNMAP_NOTIFY_CLEAR_BYTE; + } + } + 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); if (err) @@ -519,6 +542,39 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct gntdev_priv *priv, return 0; } +static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u) +{ + struct ioctl_gntdev_unmap_notify op; + struct grant_map *map; + int rc; + + if (copy_from_user(&op, u, sizeof(op))) + return -EFAULT; + + if (op.action & ~(UNMAP_NOTIFY_CLEAR_BYTE|UNMAP_NOTIFY_SEND_EVENT)) + return -EINVAL; + + spin_lock(&priv->lock); + + list_for_each_entry(map, &priv->maps, next) { + uint64_t begin = map->index << PAGE_SHIFT; + uint64_t end = (map->index + map->count) << PAGE_SHIFT; + if (op.index >= begin && op.index < end) + goto found; + } + rc = -ENOENT; + goto unlock_out; + + found: + map->notify.flags = op.action; + map->notify.addr = op.index - (map->index << PAGE_SHIFT); + map->notify.event = op.event_channel_port; + rc = 0; + unlock_out: + spin_unlock(&priv->lock); + return rc; +} + static long gntdev_ioctl(struct file *flip, unsigned int cmd, unsigned long arg) { @@ -535,6 +591,9 @@ 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_UNMAP_NOTIFY: + return gntdev_ioctl_notify(priv, ptr); + default: pr_debug("priv %p, unknown cmd %x\n", priv, cmd); return -ENOIOCTLCMD; diff --git a/include/xen/gntalloc.h b/include/xen/gntalloc.h index bc3b85e8bff..76bd58065f4 100644 --- a/include/xen/gntalloc.h +++ b/include/xen/gntalloc.h @@ -47,4 +47,36 @@ struct ioctl_gntalloc_dealloc_gref { /* Number of references to unmap */ uint32_t count; }; + +/* + * Sets up an unmap notification within the page, so that the other side can do + * cleanup if this side crashes. Required to implement cross-domain robust + * mutexes or close notification on communication channels. + * + * Each mapped page only supports one notification; multiple calls referring to + * the same page overwrite the previous notification. You must clear the + * notification prior to the IOCTL_GNTALLOC_DEALLOC_GREF if you do not want it + * to occur. + */ +#define IOCTL_GNTALLOC_SET_UNMAP_NOTIFY \ +_IOC(_IOC_NONE, 'G', 7, sizeof(struct ioctl_gntalloc_unmap_notify)) +struct ioctl_gntalloc_unmap_notify { + /* IN parameters */ + /* Offset in the file descriptor for a byte within the page (same as + * used in mmap). If using UNMAP_NOTIFY_CLEAR_BYTE, this is the byte to + * be cleared. Otherwise, it can be any byte in the page whose + * notification we are adjusting. + */ + uint64_t index; + /* Action(s) to take on unmap */ + uint32_t action; + /* Event channel to notify */ + uint32_t event_channel_port; +}; + +/* Clear (set to zero) the byte specified by index */ +#define UNMAP_NOTIFY_CLEAR_BYTE 0x1 +/* Send an interrupt on the indicated event channel */ +#define UNMAP_NOTIFY_SEND_EVENT 0x2 + #endif /* __LINUX_PUBLIC_GNTALLOC_H__ */ diff --git a/include/xen/gntdev.h b/include/xen/gntdev.h index eb23f4188f5..5304bd3c84c 100644 --- a/include/xen/gntdev.h +++ b/include/xen/gntdev.h @@ -116,4 +116,35 @@ struct ioctl_gntdev_set_max_grants { uint32_t count; }; +/* + * Sets up an unmap notification within the page, so that the other side can do + * cleanup if this side crashes. Required to implement cross-domain robust + * mutexes or close notification on communication channels. + * + * Each mapped page only supports one notification; multiple calls referring to + * the same page overwrite the previous notification. You must clear the + * notification prior to the IOCTL_GNTALLOC_DEALLOC_GREF if you do not want it + * to occur. + */ +#define IOCTL_GNTDEV_SET_UNMAP_NOTIFY \ +_IOC(_IOC_NONE, 'G', 7, sizeof(struct ioctl_gntdev_unmap_notify)) +struct ioctl_gntdev_unmap_notify { + /* IN parameters */ + /* Offset in the file descriptor for a byte within the page (same as + * used in mmap). If using UNMAP_NOTIFY_CLEAR_BYTE, this is the byte to + * be cleared. Otherwise, it can be any byte in the page whose + * notification we are adjusting. + */ + uint64_t index; + /* Action(s) to take on unmap */ + uint32_t action; + /* Event channel to notify */ + uint32_t event_channel_port; +}; + +/* Clear (set to zero) the byte specified by index */ +#define UNMAP_NOTIFY_CLEAR_BYTE 0x1 +/* Send an interrupt on the indicated event channel */ +#define UNMAP_NOTIFY_SEND_EVENT 0x2 + #endif /* __LINUX_PUBLIC_GNTDEV_H__ */ -- cgit v1.2.3-70-g09d2 From 90b6f30548a52f3a59cda5c7db0b9c2a99ebb156 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Thu, 3 Feb 2011 14:16:54 -0500 Subject: xen-gntdev: Fix memory leak when mmap fails The error path did not decrement the reference count of the grant structure. Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 9694a1a8b2e..2aa83166da3 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -650,15 +650,13 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) find_grant_ptes, map); if (err) { printk(KERN_WARNING "find_grant_ptes() failure.\n"); - return err; + goto out_put_map; } } err = map_grant_pages(map); - if (err) { - printk(KERN_WARNING "map_grant_pages() failure.\n"); - return err; - } + if (err) + goto out_put_map; map->is_mapped = 1; @@ -667,7 +665,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, map->pages[i]); if (err) - return err; + goto out_put_map; } } @@ -676,6 +674,10 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) unlock_out: spin_unlock(&priv->lock); return err; + +out_put_map: + gntdev_put_map(map); + return err; } static const struct file_operations gntdev_fops = { -- cgit v1.2.3-70-g09d2 From 0ea22f072fb1b3da4307573c280ce904f0bf1589 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Tue, 8 Feb 2011 09:14:06 -0500 Subject: xen-gntdev: Fix unmap notify on PV domains In paravirtualized guests, the struct page* for mappings is only a placeholder, and cannot be used to access the granted memory. Use the userspace mapping that we have set up in order to implement UNMAP_NOTIFY_CLEAR_BYTE. Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 2aa83166da3..75f8037c983 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -289,7 +289,12 @@ static int unmap_grant_pages(struct grant_map *map, int offset, int pages) if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) { int pgno = (map->notify.addr >> PAGE_SHIFT); - if (pgno >= offset && pgno < offset + pages) { + if (pgno >= offset && pgno < offset + pages && use_ptemod) { + void __user *tmp; + tmp = map->vma->vm_start + map->notify.addr; + copy_to_user(tmp, &err, 1); + map->notify.flags &= ~UNMAP_NOTIFY_CLEAR_BYTE; + } else if (pgno >= offset && pgno < offset + pages) { uint8_t *tmp = kmap(map->pages[pgno]); tmp[map->notify.addr & (PAGE_SIZE-1)] = 0; kunmap(map->pages[pgno]); @@ -298,7 +303,7 @@ static int unmap_grant_pages(struct grant_map *map, int offset, int pages) } 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(map->unmap_ops + offset, map->pages + offset, pages); if (err) return err; -- cgit v1.2.3-70-g09d2 From 84e4075d60fc8f1c0b937765620bc784dd0c3d39 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 9 Feb 2011 15:11:59 -0500 Subject: xen-gntdev: Use map->vma for checking map validity The is_mapped flag used to be set at the completion of the map operation, but was not checked in all error paths. Use map->vma instead, which will now be cleared if the initial grant mapping fails. Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 75f8037c983..4ca4262d3d1 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -77,7 +77,6 @@ struct grant_map { int index; int count; int flags; - int is_mapped; atomic_t users; struct unmap_notify notify; struct ioctl_gntdev_grant_ref *grants; @@ -322,7 +321,6 @@ static void gntdev_vma_close(struct vm_area_struct *vma) struct grant_map *map = vma->vm_private_data; pr_debug("close %p\n", vma); - map->is_mapped = 0; map->vma = NULL; vma->vm_private_data = NULL; gntdev_put_map(map); @@ -347,8 +345,6 @@ static void mn_invl_range_start(struct mmu_notifier *mn, list_for_each_entry(map, &priv->maps, next) { if (!map->vma) continue; - if (!map->is_mapped) - continue; if (map->vma->vm_start >= end) continue; if (map->vma->vm_end <= start) @@ -663,8 +659,6 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) if (err) goto out_put_map; - map->is_mapped = 1; - if (!use_ptemod) { for (i = 0; i < count; i++) { err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, @@ -681,6 +675,8 @@ unlock_out: return err; out_put_map: + if (use_ptemod) + map->vma = NULL; gntdev_put_map(map); return err; } -- cgit v1.2.3-70-g09d2 From b57c18694ea1641b691fa05ed8af0ce339fa430b Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 9 Feb 2011 15:12:00 -0500 Subject: xen-gntdev: Avoid unmapping ranges twice In paravirtualized domains, mn_invl_page or mn_invl_range_start can unmap a segment of a mapped region without unmapping all pages. When the region is later released, the pages will be unmapped twice, leading to an incorrect -EINVAL return. Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 4ca4262d3d1..4687cd557c9 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -282,7 +282,7 @@ static int map_grant_pages(struct grant_map *map) return err; } -static int unmap_grant_pages(struct grant_map *map, int offset, int pages) +static int __unmap_grant_pages(struct grant_map *map, int offset, int pages) { int i, err = 0; @@ -301,7 +301,6 @@ static int unmap_grant_pages(struct grant_map *map, int offset, int pages) } } - pr_debug("map %d+%d [%d+%d]\n", map->index, map->count, offset, pages); err = gnttab_unmap_refs(map->unmap_ops + offset, map->pages + offset, pages); if (err) return err; @@ -314,6 +313,36 @@ static int unmap_grant_pages(struct grant_map *map, int offset, int pages) return err; } +static int unmap_grant_pages(struct grant_map *map, int offset, int pages) +{ + int range, err = 0; + + pr_debug("unmap %d+%d [%d+%d]\n", map->index, map->count, offset, pages); + + /* It is possible the requested range will have a "hole" where we + * already unmapped some of the grants. Only unmap valid ranges. + */ + while (pages && !err) { + while (pages && !map->unmap_ops[offset].handle) { + offset++; + pages--; + } + range = 0; + while (range < pages) { + if (!map->unmap_ops[offset+range].handle) { + range--; + break; + } + range++; + } + err = __unmap_grant_pages(map, offset, range); + offset += range; + pages -= range; + } + + return err; +} + /* ------------------------------------------------------------------ */ static void gntdev_vma_close(struct vm_area_struct *vma) -- cgit v1.2.3-70-g09d2 From 12996fc38a2d760f3b30c9ceae26d0eeb92fe52d Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 9 Feb 2011 16:11:32 -0500 Subject: xen-gntdev: Avoid double-mapping memory If an already-mapped area of the device was mapped into userspace a second time, a hypercall was incorrectly made to remap the memory again. Avoid the hypercall on later mmap calls, and fail the mmap call if a writable mapping is attempted on a read-only range. Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 4687cd557c9..2c4cc940c42 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -258,6 +258,9 @@ static int map_grant_pages(struct grant_map *map) phys_addr_t addr; if (!use_ptemod) { + /* Note: it could already be mapped */ + if (map->map_ops[0].handle) + return 0; for (i = 0; i < map->count; i++) { addr = (phys_addr_t) pfn_to_kaddr(page_to_pfn(map->pages[i])); @@ -668,9 +671,15 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) if (use_ptemod) map->vma = vma; - map->flags = GNTMAP_host_map; - if (!(vma->vm_flags & VM_WRITE)) - map->flags |= GNTMAP_readonly; + if (map->flags) { + if ((vma->vm_flags & VM_WRITE) && + (map->flags & GNTMAP_readonly)) + return -EINVAL; + } else { + map->flags = GNTMAP_host_map; + if (!(vma->vm_flags & VM_WRITE)) + map->flags |= GNTMAP_readonly; + } spin_unlock(&priv->lock); -- cgit v1.2.3-70-g09d2 From 9960be970cea52c1cb7d7c747ff6da367e1c01b5 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 9 Feb 2011 18:15:50 -0500 Subject: xen-gntdev: prevent using UNMAP_NOTIFY_CLEAR_BYTE on read-only mappings Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 2c4cc940c42..2a4733c621c 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -294,7 +294,9 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages) if (pgno >= offset && pgno < offset + pages && use_ptemod) { void __user *tmp; tmp = map->vma->vm_start + map->notify.addr; - copy_to_user(tmp, &err, 1); + err = copy_to_user(tmp, &err, 1); + if (err) + return err; map->notify.flags &= ~UNMAP_NOTIFY_CLEAR_BYTE; } else if (pgno >= offset && pgno < offset + pages) { uint8_t *tmp = kmap(map->pages[pgno]); @@ -599,6 +601,12 @@ static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u) goto unlock_out; found: + if ((op.action & UNMAP_NOTIFY_CLEAR_BYTE) && + (map->flags & GNTMAP_readonly)) { + rc = -EINVAL; + goto unlock_out; + } + map->notify.flags = op.action; map->notify.addr = op.index - (map->index << PAGE_SHIFT); map->notify.event = op.event_channel_port; -- cgit v1.2.3-70-g09d2 From 77c35acb7c81cd94c6b30a3bef488dd2d8145131 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 23 Feb 2011 08:11:35 -0500 Subject: xen-gntdev: Fix incorrect use of zero handle The handle with numeric value 0 is a valid map handle, so it cannot be used to indicate that a page has not been mapped. Use -1 instead. Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 2a4733c621c..cdc28dc8b5d 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -126,6 +126,8 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) add->pages[i] = alloc_page(GFP_KERNEL | __GFP_HIGHMEM); if (add->pages[i] == NULL) goto err; + add->map_ops[i].handle = -1; + add->unmap_ops[i].handle = -1; } add->index = 0; @@ -248,7 +250,7 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token, map->grants[pgnr].ref, map->grants[pgnr].domid); gnttab_set_unmap_op(&map->unmap_ops[pgnr], pte_maddr, flags, - 0 /* handle */); + -1 /* handle */); return 0; } @@ -259,7 +261,7 @@ static int map_grant_pages(struct grant_map *map) if (!use_ptemod) { /* Note: it could already be mapped */ - if (map->map_ops[0].handle) + if (map->map_ops[0].handle != -1) return 0; for (i = 0; i < map->count; i++) { addr = (phys_addr_t) @@ -268,7 +270,7 @@ static int map_grant_pages(struct grant_map *map) map->grants[i].ref, map->grants[i].domid); gnttab_set_unmap_op(&map->unmap_ops[i], addr, - map->flags, 0 /* handle */); + map->flags, -1 /* handle */); } } @@ -280,7 +282,11 @@ static int map_grant_pages(struct grant_map *map) for (i = 0; i < map->count; i++) { if (map->map_ops[i].status) err = -EINVAL; - map->unmap_ops[i].handle = map->map_ops[i].handle; + else { + BUG_ON(map->map_ops[i].handle == -1); + map->unmap_ops[i].handle = map->map_ops[i].handle; + pr_debug("map handle=%d\n", map->map_ops[i].handle); + } } return err; } @@ -313,7 +319,10 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages) for (i = 0; i < pages; i++) { if (map->unmap_ops[offset+i].status) err = -EINVAL; - map->unmap_ops[offset+i].handle = 0; + pr_debug("unmap handle=%d st=%d\n", + map->unmap_ops[offset+i].handle, + map->unmap_ops[offset+i].status); + map->unmap_ops[offset+i].handle = -1; } return err; } @@ -328,13 +337,13 @@ static int unmap_grant_pages(struct grant_map *map, int offset, int pages) * already unmapped some of the grants. Only unmap valid ranges. */ while (pages && !err) { - while (pages && !map->unmap_ops[offset].handle) { + while (pages && map->unmap_ops[offset].handle == -1) { offset++; pages--; } range = 0; while (range < pages) { - if (!map->unmap_ops[offset+range].handle) { + if (map->unmap_ops[offset+range].handle == -1) { range--; break; } -- cgit v1.2.3-70-g09d2 From f4ee4af447b67135de7eb8a6615811c13ce938e2 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 23 Feb 2011 08:11:36 -0500 Subject: xen-gntdev: Add cast to pointer Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index cdc28dc8b5d..d43ff3072c9 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -298,8 +298,8 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages) if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) { int pgno = (map->notify.addr >> PAGE_SHIFT); if (pgno >= offset && pgno < offset + pages && use_ptemod) { - void __user *tmp; - tmp = map->vma->vm_start + map->notify.addr; + void __user *tmp = (void __user *) + map->vma->vm_start + map->notify.addr; err = copy_to_user(tmp, &err, 1); if (err) return err; -- cgit v1.2.3-70-g09d2 From 38eaeb0fd8819dce424a61579500bd9987d5c930 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Tue, 8 Mar 2011 16:56:43 +0000 Subject: xen: gntdev: fix build warning addr is actually a virtual address so use an unsigned long. Fixes: CC drivers/xen/gntdev.o drivers/xen/gntdev.c: In function 'map_grant_pages': drivers/xen/gntdev.c:268: warning: cast from pointer to integer of different size Reduce the scope of the variable at the same time. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index d43ff3072c9..d96d311b858 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -257,14 +257,13 @@ 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) { /* Note: it could already be mapped */ if (map->map_ops[0].handle != -1) return 0; for (i = 0; i < map->count; i++) { - addr = (phys_addr_t) + unsigned long addr = (unsigned long) pfn_to_kaddr(page_to_pfn(map->pages[i])); gnttab_set_map_op(&map->map_ops[i], addr, map->flags, map->grants[i].ref, -- cgit v1.2.3-70-g09d2 From ca47ceaa2c407bbddd395c1807b616042365bd65 Mon Sep 17 00:00:00 2001 From: Daniel De Graaf Date: Wed, 9 Mar 2011 18:07:34 -0500 Subject: xen-gntdev: Use ballooned pages for grant mappings Grant mappings cause the PFN<->MFN mapping to be lost on the pages used for the mapping. Instead of leaking memory, use pages that have already been ballooned out and so have no valid mapping. This removes the need for the bad-page leak workaround as pages are repopulated by the balloon driver. Acked-by: Ian Campbell Signed-off-by: Daniel De Graaf Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index d96d311b858..017ce600fbc 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -122,10 +123,10 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) NULL == add->pages) goto err; + if (alloc_xenballooned_pages(count, add->pages)) + goto err; + for (i = 0; i < count; i++) { - add->pages[i] = alloc_page(GFP_KERNEL | __GFP_HIGHMEM); - if (add->pages[i] == NULL) - goto err; add->map_ops[i].handle = -1; add->unmap_ops[i].handle = -1; } @@ -137,11 +138,6 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) return add; err: - if (add->pages) - 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); @@ -184,8 +180,6 @@ static struct grant_map *gntdev_find_map_index(struct gntdev_priv *priv, static void gntdev_put_map(struct grant_map *map) { - int i; - if (!map) return; @@ -202,29 +196,7 @@ static void gntdev_put_map(struct grant_map *map) if (!use_ptemod) unmap_grant_pages(map, 0, map->count); - for (i = 0; i < map->count; 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. - * - * Confirmed present in Xen 4.1-RC3 with HVM source - */ - 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])); - } + free_xenballooned_pages(map->count, map->pages); } kfree(map->pages); kfree(map->grants); -- cgit v1.2.3-70-g09d2 From 12f0258d5b44b3b5a9442ec461bbac1f7edab8c6 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 19 Mar 2011 08:44:34 +0300 Subject: xen-gntdev: return -EFAULT on copy_to_user failure copy_to_user() returns the amount of data remaining to be copied. We want to return a negative error code here. The upper layers just call WARN_ON() if we return non-zero so this doesn't change the behavior. But returning -EFAULT is still cleaner. Signed-off-by: Dan Carpenter Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 017ce600fbc..00b3accc43e 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -273,7 +273,7 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages) map->vma->vm_start + map->notify.addr; err = copy_to_user(tmp, &err, 1); if (err) - return err; + return -EFAULT; map->notify.flags &= ~UNMAP_NOTIFY_CLEAR_BYTE; } else if (pgno >= offset && pgno < offset + pages) { uint8_t *tmp = kmap(map->pages[pgno]); -- cgit v1.2.3-70-g09d2 From a93e20a83077f57638f8574b1c072ce6712e95ec Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 19 Mar 2011 08:45:43 +0300 Subject: xen-gntdev: unlock on error path in gntdev_mmap() We should unlock here and also decrement the number of &map->users. Signed-off-by: Dan Carpenter Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/gntdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/xen/gntdev.c') diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 00b3accc43e..b0f9e8fb005 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -662,7 +662,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) if (map->flags) { if ((vma->vm_flags & VM_WRITE) && (map->flags & GNTMAP_readonly)) - return -EINVAL; + goto out_unlock_put; } else { map->flags = GNTMAP_host_map; if (!(vma->vm_flags & VM_WRITE)) @@ -700,6 +700,8 @@ unlock_out: spin_unlock(&priv->lock); return err; +out_unlock_put: + spin_unlock(&priv->lock); out_put_map: if (use_ptemod) map->vma = NULL; -- cgit v1.2.3-70-g09d2