Re: [Xen-devel] [PATCH] xen: arm: invalidate caches after map_domain_page done

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-08-01 at 21:54 +0300, Andrii Tseglytskyi wrote:
>  Hi Julien,
> 
> On Fri, Aug 1, 2014 at 8:49 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> >
> >
> > On 01/08/14 16:06, Andrii Tseglytskyi wrote:
> >>
> >> Looks like I see where is the issue:
> >> After mapping done kernel driver calls flush_tlb_all() function, which
> >> just invalidates cache, it does the similar command, as the following
> >
> >
> > flush_tlb_all doesn't invalidate the cache but the TLB.
> >
> >
> >> Xen macros:
> >>
> >> #define DTLBIALL        p15,0,c8,c6,0   /* Invalidate data TLB */
> >>
> >> Then after mapping done, remoteproc_iommu starts translation, calls
> >> map_domain_page() -> flush_xen_data_tlb_range_va_local(),
> >> which is described with following macros:
> >>
> >> #define TLBIMVAH        p15,4,c8,c7,1   /* Invalidate Unified Hyp. TLB by
> >> MVA */
> >>
> >> So, I got 2 invalidates and no cleans. And when I started using
> >> clean_and_invalidate_xen_dcache_va_range() I got both:
> >>
> >> #define DCCIMVAC        p15,0,c7,c14,1  /* Data cache clean and
> >> invalidate by MVA */
> >>
> >> I need both - clean and invalidate. If I don't have clean - data may
> >> still present in cache and not flushed to RAM - I will see invalid
> >> data after map_domain_page() call
> >
> >
> > You seem to mix TLB and cache in your mail. If the page has been mapped with
> > cache attribute (should be done by kmalloc), then it should not have any
> > issue in Xen.
> >
> > Your patch is removing the TLB flush and you are very lucky that Xen is
> > still working correctly...
> 
> I will not remove TLB flush and modify common code. In any case
> map_domain_page() does not work as is for me.
> And I think that having any dependencies on how page is mapped in
> kernel - cacheable or not is not the best solution for me.

I'd much prefer that it was understood why the kernel's supposedly
cacheable mappings + maintenance done by the driver are not working for
you before adding new API on the Xen side.

You message at 01/08/14 16:06 seems to be a bit confused WRT caches vs.
TLBs and you also said that the kernel was only invalidating the caches,
not cleaning (invalidating == throw away data in the cache, exposing
whatever was in the underlying RAM).

I think you need to sort all of that out on the kernel side before
considering hypervisor patches.

Ian.

> I think I'll introduce a new wrapper with map_domain_page() and proper
> cache invalidation API, which will work for me.
> 
> Thanks a lot for detailed review and clear explanations ))
> 
> Regards,
> Andrii
> 
> >
> > Regards,
> >
> > --
> > Julien Grall
> 
> 
> 



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