Re: [Xen-devel] [PATCH v3 4/5] xen/arm: introduce GNTTABOP_cache_flush

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.10.14 at 15:00, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> @@ -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)
> +{
> +    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 )

Indentation.

> +        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++ )
> +    {
> +        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;

Why "-ret"? Especially with ...

> @@ -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;
> +    }

... a positive value here meaning "continuation needed".

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel