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

[Julien Grall <julien.grall@linaro.org>]

Hi Andrii,

On 01/08/14 08:25, Andrii Tseglytskyi wrote:
> In some cases, memory page returned by map_domain_page() contains
> invalid data. Issue is observed when map_domain_page() is used
> immediately after p2m_lookup() function, when random page of
> guest domain memory is need to be mapped to xen. Data on this
> already memory page may be not valid. Issue is fixed when
> caches are invalidated after mapping is done.
>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> ---
>   xen/arch/arm/mm.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 0a243b0..085780a 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -304,7 +304,7 @@ void *map_domain_page(unsigned long mfn)
>        * We may not have flushed this specific subpage at map time,
>        * since we only flush the 4k page not the superpage
>        */
> -    flush_xen_data_tlb_range_va_local(va, PAGE_SIZE);

Why did you remove the flush TLB? It's requirement to make sure the VA 
will pointed to the right PA.

> +    clean_and_invalidate_xen_dcache_va_range((void *)va, PAGE_SIZE);

This is not the right approach, map_domain_page is heavily used to map 
hypercall data page. Those pages must reside in normal inner-cacheable
memory. So cleaning the cache is useless and time consuming.

If you want to clean and invalidate the cache, even though I don't think 
this right by reading the commit message, you have to introduce a new 
helper.

>       return (void *)va;
>   }
>

Regards,

-- 
Julien Grall