Re: [Xen-devel] xen/arm: Alternative start of day cache coherency

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-01-22 at 17:32 +0000, Stefano Stabellini wrote:
> > +int xc_domain_cacheflush(xc_interface *xch, uint32_t domid)
> > +{
> > +    DECLARE_DOMCTL;
> > +    domctl.cmd = XEN_DOMCTL_cacheflush;
> > +    domctl.domain = (domid_t)domid;
> > +    domctl.u.cacheflush.start_mfn = 0;
> > +    return do_domctl(xch, &domctl);
> > +}
> 
> Do we really need to flush the entire p2m, or just things we have
> written to?

I think we need to flush everything (well, all RAM backed pages, the
patch skips everything else).

Even things which we haven't explicitly written to will have been
scrubbed and therefore have scrubbed data in the cache but data
belonging to the previous owner in actual RAM. So we would really want
to clean in that case too.

We could do the clean at scrub time which arguably be better anyway and
would potentially allows us to only invalidate instead of clean
+invalidate some subset of pages, but we would need to track which sort
of page was which -- e.g. with a special p2m type for a page which had
been foreign mapped or some other bit of metadata.

> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index 85ca330..f35ed57 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -228,15 +228,26 @@ enum p2m_operation {
> >      ALLOCATE,
> >      REMOVE,
> >      RELINQUISH,
> > +    CACHEFLUSH,
> >  };
> >  
> > +static void do_one_cacheflush(paddr_t mfn)
> > +{
> > +    void *v = map_domain_page(mfn);
> > +
> > +    flush_xen_dcache_va_range(v, PAGE_SIZE);
> > +
> > +    unmap_domain_page(v);
> > +}
> 
> A pity that we need to map a page just to flush the dcache.  It could be
> expensive, especially if we really have to map every single guest mfn.

Remember that this is basically free for arm64 and for arm32 we actually
map 2MB regions and cache, so it is only actually one map per 2MB
region.

> I wonder if we could use DCCSW instead.

There is no way to use this by VMID so we would have to blow the entire
cache. DCCSW is also very tricky to use in an SMP system, you might need
to do some sort of stop-machine trick (although perhaps for this use
case we know the tools in dom0 only has a single thread touching the
foreign memory and the guest itself isn't running). I'd need to think
very carefully about that case but since it involves flushing the entire
cache I'm not inclined to go down that path in the first place.

> > +        case RELINQUISH:
> > +        case CACHEFLUSH:
> > +            if (count >= 0x2000 && hypercall_preempt_check() )
> >              {
> >                  p2m->next_gfn_to_relinquish = addr >> PAGE_SHIFT;
> 
> If we are taking this code path for cache flushes, then we should rename
> next_gfn_to_relinquish to something more generic.

Yes, this was just a proof of concept so I didn't bother, but really
this is minimum_mapped_p2m.

create_p2m_entries should also be walk_p2m_entries or some such.

Ian.


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