|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH 6/6] xen/gntalloc, gntdev: Add unmap notify ioctl
On Fri, Jan 21, 2011 at 10:59:08AM -0500, Daniel De Graaf wrote:
> This ioctl allows the users of a shared page to be notified when
> the other end exits abnormally.
>
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
> drivers/xen/gntalloc.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> drivers/xen/gntdev.c | 56
> +++++++++++++++++++++++++++++++++++++++++++++++-
> include/xen/gntalloc.h | 28 ++++++++++++++++++++++++
> include/xen/gntdev.h | 27 +++++++++++++++++++++++
> 4 files changed, 165 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
> index a230dc4..8ac0dfc 100644
> --- a/drivers/xen/gntalloc.c
> +++ b/drivers/xen/gntalloc.c
> @@ -60,11 +60,13 @@
> #include <linux/uaccess.h>
> #include <linux/types.h>
> #include <linux/list.h>
> +#include <linux/highmem.h>
>
> #include <xen/xen.h>
> #include <xen/page.h>
> #include <xen/grant_table.h>
> #include <xen/gntalloc.h>
> +#include <xen/events.h>
>
> static int limit = 1024;
> module_param(limit, int, 0644);
> @@ -83,6 +85,9 @@ 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 */
> + unsigned notify_flags:2; /* Unmap notification flags */
Can you add to the comment: bits 0-1
> + unsigned notify_pgoff:12; /* Offset of the byte to clear */
^^ in the page
And "bits 2-14.".
> + int notify_event; /* Port (event channel) to notify */
> };
>
> struct gntalloc_file_private_data {
> @@ -169,6 +174,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 (gnttab_query_foreign_access(gref->gref_id))
> return;
>
> @@ -339,6 +354,43 @@ dealloc_grant_out:
> return rc;
> }
>
> +static long gntalloc_ioctl_notify(struct gntalloc_file_private_data *priv,
> + void __user *arg)
Call it '..ioctl_unmap_notify'. When I read it first time I thought this
would do just notify.. while in actuality it can do both.
> +{
> + 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);
You don't want to tell the user if the messed up? Say 0xdeadf00d?
> +
> + 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)
> {
> @@ -351,6 +403,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_notify(priv, (void __user *)arg);
> +
> default:
> return -ENOIOCTLCMD;
> }
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index a5710e8..e360d0a 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -37,6 +37,7 @@
> #include <xen/xen.h>
> #include <xen/grant_table.h>
> #include <xen/gntdev.h>
> +#include <xen/events.h>
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/page.h>
> @@ -70,6 +71,9 @@ struct grant_map {
> int count;
> int flags;
> int is_mapped;
> + int notify_flags;
> + int notify_offset;
> + int notify_event;
Would it make sense to put these inside a struct?
> atomic_t users;
> struct ioctl_gntdev_grant_ref *grants;
> struct gnttab_map_grant_ref *map_ops;
> @@ -165,7 +169,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 +188,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);
> @@ -272,6 +280,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_offset >> PAGE_SHIFT);
> + if (pgno >= offset && pgno < pages + offset) {
Whoa, that looks to violate what 'notify_offset' actually means.
Perhaps a better name for that variable? notify_addr?
> + uint8_t *tmp = kmap(map->pages[pgno]);
> + tmp[map->notify_offset & (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)
> @@ -517,6 +535,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_offset = op.index - (map->index << PAGE_SHIFT);
Minus? So say the index is 0xdeadbeef, wouldn't this throw this off?
Should you also check the index to make sure it is within a PAGE_SIZE
offset? (the ioctl accepts 64-bit values for the index).
> + 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)
> {
> @@ -533,6 +584,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 e1d6d0f..92339f5 100644
> --- a/include/xen/gntalloc.h
> +++ b/include/xen/gntalloc.h
> @@ -67,4 +67,32 @@ 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 */
> + /* Index of a byte in the page */
> + 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 eb23f41..5d9b9b4 100644
> --- a/include/xen/gntdev.h
> +++ b/include/xen/gntdev.h
> @@ -116,4 +116,31 @@ 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 */
> + /* Index of a byte in the page */
> + 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__ */
> --
> 1.7.3.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |