Re: [PATCH v2 3/4] xen/arm: introduce GNTTABOP_cache_flush

[David Vrabel <david.vrabel@citrix.com>]

On 03/10/14 15:50, 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.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
[...]
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -2641,6 +2641,79 @@ do_grant_table_op(
[...]
> +
> +        page = mfn_to_page(mfn);
> +        owner = page_get_owner_and_reference(page);
> +        if ( !owner )
> +        {
> +            rcu_unlock_domain(d);
> +            return -EFAULT;
> +        }
> +
> +        spin_lock(&owner->grant_table->lock);

The grant table lock is already heavily contended,  so you should skip
the lock and the grant_map_exists() check if d == owner.

> +
> +        if ( !grant_map_exists(d, owner->grant_table, mfn) )

Looping over all grant table entries or all maptrack entries looks
expensive to me.

Perhaps consider allowing suitably privileged domains to
clean/invalidate any address without having to check if it's been granted.

Instead of this hypercall, could the guest clean/invalidate by set/way?
 I guess this would need a suitable IPA which could be obtained by some
(offset) 1:1 mapping in the stage 2 tables?

David