Re: [PATCH v3 3/3] xen/arm: introduce XENFEAT_grant_map_identity

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

On 07/24/2014 04:02 PM, Stefano Stabellini wrote:
> On Thu, 24 Jul 2014, Julien Grall wrote:
>> On 07/24/2014 03:43 PM, Jan Beulich wrote:
>>>>>> On 24.07.14 at 16:10, <stefano.stabellini@eu.citrix.com> wrote:
>>>> On Thu, 24 Jul 2014, Jan Beulich wrote:
>>>>>>>> On 24.07.14 at 15:31, <stefano.stabellini@eu.citrix.com> wrote:
>>>>>> --- a/xen/common/grant_table.c
>>>>>> +++ b/xen/common/grant_table.c
>>>>>> @@ -727,7 +727,7 @@ __gnttab_map_grant_ref(
>>>>>>  
>>>>>>      double_gt_lock(lgt, rgt);
>>>>>>  
>>>>>> -    if ( gnttab_need_iommu_mapping(ld) )
>>>>>> +    if ( gnttab_need_iommu_mapping(ld) || gnttab_need_identity_mapping(ld) 
>>>> )
>>>>>
>>>>> As before I think this change is pointless.
>>>>
>>>> I don't understand how you propose to solve the problem.
>>>
>>> I think ARM's gnttab_need_iommu_mapping() needs to just be
>>> is_domain_direct_mapped() then.
> 
> That's not correct either. It would work, but it is conceptually wrong.
> From the ARM perspective we have 2 distinct cases to cover:
> - we have an smmu and we need to create a mapping for it
> - we don't have an smmu and we need an identity mapping
> 
> 
>> So ARM people will think we need an iommu to add the identity mapping.
>> Why don't we rename gnttab_need_iommu_mapping into
>> gnttab_need_identity_mapping?
>>
>> It would less confusing for both x86 and ARM.
> 
> I don't think that would be any better.
> We have 2 distinct cases to cover, we need to treat them as such.

The cases are not so distinct. We need the 1:1 grant mapping for the
IOMMU because of the swiotlb and the 1:1 memory mapping.

Without it we can easily bypass this code.

-- 
Julien Grall