[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush



>>> On 17.10.14 at 17:31, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> +static int grant_map_exists(const struct domain *ld,
> +                            struct grant_table *rgt,
> +                            unsigned long mfn,
> +                            grant_ref_t *ref_count)
> +{
> +    const struct active_grant_entry *act;
> +    grant_ref_t ref;
> +
> +    ASSERT(spin_is_locked(&rgt->lock));
> +
> +    for ( ref = *ref_count; ref < nr_grant_entries(rgt); ref++ )
> +    {
> +        if ( (ref % MAX_GRANT_ENTRIES_ITERATIONS) == 0 && ref != *ref_count )
> +        {
> +            *ref_count = ref;
> +            return ref;
> +        }
> +
> +        act = &active_entry(rgt, ref);
> +
> +        if ( !act->pin )
> +            continue;
> +
> +        if ( act->domid != ld->domain_id )
> +            continue;
> +
> +        if ( act->frame != mfn )
> +            continue;
> +
> +        return 0;
> +    }
> +
> +    return -1;

Return values need to be adjusted throughout the function: The
-1 here gets passed up all the way, and hence will be seen as -EPERM
by the caller of the hypercall, which clearly isn't right. Also returning
ref (effectively a uint32_t) above doesn't fit with the function
returning (signed) int. The caller doesn't care about the actual count,
so just returning 1 in that case would seem fine.

> +#define OPAQUE_CONTINUATION_ARG_SHIFT 16
> +#define CMD_MASK                      ((1<<OPAQUE_CONTINUATION_ARG_SHIFT)-1)

Along the lines of MEMOP_CMD_MASK, GNTTABOP_CMD_MASK
please. Also this needs to be moved and the compat wrapper
needs to be updated to cope with this extension. And finally 16
bits here and 11 bits in grant_map_exists() make 27, but you're
nowhere enforcing this upper limit on grant entry count. Either
do this, or adjust the value such that together with
MAX_GRANT_ENTRIES_ITERATIONS you can encode to full
value range.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.