|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 4/5] xen/arm: introduce GNTTABOP_cache_flush
On 08/10/14 14:00, Stefano Stabellini wrote:
> Introduce a new hypercall to perform cache maintenance operation on
> behalf of the guest. The argument is a machine address and a size. The
> implementation checks that the memory range is owned by the guest or the
> guest has been granted access to it by another domain.
>
> Introduce grant_map_exists: an internal grant table function to check
> whether an mfn has been granted to a given domain on a target grant
> table.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>
> ---
>
> Changes in v3:
> - reduce the time the grant_table lock is held;
> - fix warning message;
> - s/EFAULT/EPERM;
> - s/llx/PRIx64;
> - check offset and size independetly before checking their sum;
> - rcu_lock_current_domain cannot fail;
> - s/ENOSYS/EOPNOTSUPP;
> - use clean_and_invalidate_xen_dcache_va_range to do both operations at
> once;
> - fold grant_map_exists in this patch;
> - support "count" argument;
> - make correspondent changes to compat/grant_table.c;
> - introduce GNTTAB_CACHE_SOURCE_GREF to select the type of input in the
> union;
> - rename size field to length;
> - make length and offset uint16_t;
> - only take spin_lock if d != owner.
>
> Changes in v2:
> - do not check for mfn_to_page errors;
> - take a reference to the page;
> - replace printk with gdprintk;
> - split long line;
> - remove out label;
> - move rcu_lock_current_domain down before the loop.
> - move the hypercall to GNTTABOP;
> - take a spin_lock before calling grant_map_exists.
> ---
> xen/common/compat/grant_table.c | 8 +++
> xen/common/grant_table.c | 135
> ++++++++++++++++++++++++++++++++++++++
> xen/include/public/grant_table.h | 20 ++++++
> xen/include/xlat.lst | 1 +
> 4 files changed, 164 insertions(+)
>
> diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
> index 6c00b09..4a3a23d 100644
> --- a/xen/common/compat/grant_table.c
> +++ b/xen/common/compat/grant_table.c
> @@ -51,6 +51,10 @@ CHECK_gnttab_get_version;
> CHECK_gnttab_swap_grant_ref;
> #undef xen_gnttab_swap_grant_ref
>
> +#define xen_gnttab_cache_flush gnttab_cache_flush
> +CHECK_gnttab_cache_flush;
> +#undef xen_gnttab_cache_flush
> +
> int compat_grant_table_op(unsigned int cmd,
> XEN_GUEST_HANDLE_PARAM(void) cmp_uop,
> unsigned int count)
> @@ -106,6 +110,10 @@ int compat_grant_table_op(unsigned int cmd,
> CASE(swap_grant_ref);
> #endif
>
> +#ifndef CHECK_gnttab_cache_flush
> + CASE(cache_flush);
> +#endif
> +
> #undef CASE
> default:
> return do_grant_table_op(cmd, cmp_uop, count);
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index a56a1bb..c16c11c 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -501,6 +501,36 @@ static int _set_status(unsigned gt_version,
> return _set_status_v2(domid, readonly, mapflag, shah, act, status);
> }
>
> +static bool_t grant_map_exists(const struct domain *ld,
> + struct grant_table *rgt,
> + unsigned long mfn)
> +{
> + const struct active_grant_entry *act;
> + grant_ref_t ref;
> + bool_t ret = 0;
> +
> + spin_is_locked(&rgt->lock);
This is now dead code. spin_is_locked() is just a predicate.
> +
> + for ( ref = 0; ref != nr_grant_entries(rgt); ref++ )
> + {
> + act = &active_entry(rgt, ref);
> +
> + if ( !act->pin )
> + continue;
> +
> + if ( act->domid != ld->domain_id )
> + continue;
> +
> + if ( act->frame != mfn )
> + continue;
> +
> + ret = 1;
> + break;
how about return 1 here...
> + }
> +
> + return ret;
and return 0 here?
> +}
> +
> static void mapcount(
> struct grant_table *lgt, struct domain *rd, unsigned long mfn,
> unsigned int *wrc, unsigned int *rdc)
> @@ -2498,6 +2528,97 @@
> gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop,
> return 0;
> }
>
> +static int __gnttab_cache_flush(gnttab_cache_flush_t cflush)
This should be passed via pointer.
> +{
> + struct domain *d, *owner;
> + struct page_info *page;
> + uint64_t mfn;
> + void *v;
> +
> + if ( cflush.offset > PAGE_SIZE ||
> + cflush.length > PAGE_SIZE ||
> + cflush.offset + cflush.length > PAGE_SIZE )
Brackets, as per Xen style, please.
> + return -EINVAL;
> +
> + if ( cflush.length == 0 || cflush.op == 0 )
> + return 0;
> +
> + /* currently unimplemented */
> + if ( cflush.op & GNTTAB_CACHE_SOURCE_GREF )
> + return -EOPNOTSUPP;
> +
> + d = rcu_lock_current_domain();
> + mfn = cflush.a.dev_bus_addr >> PAGE_SHIFT;
> +
> + if ( !mfn_valid(mfn) )
> + {
> + rcu_unlock_domain(d);
> + return -EINVAL;
> + }
> +
> + page = mfn_to_page(mfn);
> + owner = page_get_owner_and_reference(page);
> + if ( !owner )
> + {
> + rcu_unlock_domain(d);
> + return -EPERM;
> + }
> +
> + if ( d != owner )
> + {
> + spin_lock(&owner->grant_table->lock);
> +
> + if ( !grant_map_exists(d, owner->grant_table, mfn) )
> + {
> + spin_unlock(&owner->grant_table->lock);
> + rcu_unlock_domain(d);
> + put_page(page);
> + gdprintk(XENLOG_G_ERR, "mfn %"PRIx64" hasn't been granted by d%d
> to d%d\n",
> + mfn, owner->domain_id, d->domain_id);
> + return -EINVAL;
> + }
> + }
> +
> + v = map_domain_page(mfn);
> + v += cflush.offset;
> +
> + if ( (cflush.op & GNTTAB_CACHE_INVAL) && (cflush.op &
> GNTTAB_CACHE_CLEAN) )
> + clean_and_invalidate_xen_dcache_va_range(v, cflush.length);
> + else if ( cflush.op & GNTTAB_CACHE_INVAL )
> + invalidate_xen_dcache_va_range(v, cflush.length);
> + else if ( cflush.op & GNTTAB_CACHE_CLEAN )
> + clean_xen_dcache_va_range(v, cflush.length);
> +
> + if ( d != owner )
> + spin_unlock(&owner->grant_table->lock);
> + unmap_domain_page(v);
> + put_page(page);
> +
> + return 0;
> +}
> +
> +static long
> +gnttab_cache_flush(XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) uop,
> + unsigned int count)
> +{
> + int i, ret;
> + gnttab_cache_flush_t op;
> +
> + for ( i = 0; i < count; i++ )
i must be unsigned, or this can be an infinite loop, as count in an
unverified user parameter at this point.
~Andrew
> + {
> + if ( i && hypercall_preempt_check() )
> + return i;
> + if ( unlikely(__copy_from_guest(&op, uop, 1)) )
> + return -EFAULT;
> + ret = __gnttab_cache_flush(op);
> + if ( ret < 0 )
> + return -ret;
> + guest_handle_add_offset(uop, 1);
> + }
> + return 0;
> +}
> +
> +
> long
> do_grant_table_op(
> unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
> @@ -2627,6 +2748,20 @@ do_grant_table_op(
> }
> break;
> }
> + case GNTTABOP_cache_flush:
> + {
> + XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) cflush =
> + guest_handle_cast(uop, gnttab_cache_flush_t);
> + if ( unlikely(!guest_handle_okay(cflush, count)) )
> + goto out;
> + rc = gnttab_cache_flush(cflush, count);
> + if ( rc > 0 )
> + {
> + guest_handle_add_offset(cflush, rc);
> + uop = guest_handle_cast(cflush, void);
> + }
> + break;
> + }
> default:
> rc = -ENOSYS;
> break;
> diff --git a/xen/include/public/grant_table.h
> b/xen/include/public/grant_table.h
> index b8a3d6c..20d4e77 100644
> --- a/xen/include/public/grant_table.h
> +++ b/xen/include/public/grant_table.h
> @@ -309,6 +309,7 @@ typedef uint16_t grant_status_t;
> #define GNTTABOP_get_status_frames 9
> #define GNTTABOP_get_version 10
> #define GNTTABOP_swap_grant_ref 11
> +#define GNTTABOP_cache_flush 12
> #endif /* __XEN_INTERFACE_VERSION__ */
> /* ` } */
>
> @@ -574,6 +575,25 @@ struct gnttab_swap_grant_ref {
> typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
> DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
>
> +/*
> + * Issue one or more cache maintenance operations on a portion of a
> + * page granted to the calling domain by a foreign domain.
> + */
> +struct gnttab_cache_flush {
> + union {
> + uint64_t dev_bus_addr;
> + grant_ref_t ref;
> + } a;
> + uint16_t offset; /* offset from start of grant */
> + uint16_t length; /* size within the grant */
> +#define GNTTAB_CACHE_CLEAN (1<<0)
> +#define GNTTAB_CACHE_INVAL (1<<1)
> +#define GNTTAB_CACHE_SOURCE_GREF (1<<31)
> + uint32_t op;
> +};
> +typedef struct gnttab_cache_flush gnttab_cache_flush_t;
> +DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
> +
> #endif /* __XEN_INTERFACE_VERSION__ */
>
> /*
> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> index 9a35dd7..3822b00 100644
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -51,6 +51,7 @@
> ? grant_entry_header grant_table.h
> ? grant_entry_v2 grant_table.h
> ? gnttab_swap_grant_ref grant_table.h
> +? gnttab_cache_flush grant_table.h
> ? kexec_exec kexec.h
> ! kexec_image kexec.h
> ! kexec_range kexec.h
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |