Re: [PATCH v4 18/21] xen/arm: p2m: Clean cache PT when the IOMMU doesn't support coherent walk

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

On 04/28/2014 03:09 PM, Ian Campbell wrote:
>>  xen/arch/arm/p2m.c              |   24 ++++++++++++++++++------
>>  xen/drivers/passthrough/iommu.c |   11 +++++++++++
>>  xen/include/xen/iommu.h         |    5 +++++
>>  3 files changed, 34 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 21219de..996d2bd 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -274,6 +274,18 @@ enum p2m_operation {
>>      CACHEFLUSH,
>>  };
>>  
>> +static void unmap_coherent_domain_page(struct domain *d, const void *va)
>> +{
>> +    /* Some IOMMU doesn't support coherent PT walk. When the p2m is
> 
> "don't support"
> 
>> +     * shared with the CPU, Xen has to make sure that the PT changes have
>> +     * reached the memory
>> +     */
>> +    if ( need_iommu(d) && !iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK) )
>> +        clean_xen_dcache_va_range(va, PAGE_SIZE);
> 
> This is a fairly large hammer when you might only have touched a few
> bytes, if any, in the page.
> 
> Wouldn't it be better to do this is write_pte, or just after the calls
> to write_pte?

I didn't think that the cache flush on a page was divided in small chunks.

> iommu_has_feature has a lot of callbacks -- worth calling once in
> apply_p2m_changes?

I will create a write_pte_coherent helpers which will take in parameter
a boolean to know if we need to flush the cache or not.

Regards,

-- 
Julien Grall