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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 6 Oct 2014, David Vrabel wrote:
> 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@xxxxxxxxxxxxx>
> [...]
> > --- 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.

OK, but it is going to make the code uglier.


> > +
> > +        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.
 
I think that would weaken our security guarantees.


> 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?

I wish I could: as the clean/invalidate by set/way is implementation
specific, there is now way to use it to clean a specific range of
addresses.

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