Re: [PATCH v2] xen/iommu: fix usage of shared EPT/IOMMU page tables on PVH guests

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

On 10/03/15 13:06, Jan Beulich wrote:
>>>> On 10.03.15 at 13:51, <julien.grall@linaro.org> wrote:
>> Hi,
>>
>> On 27/02/15 11:33, Roger Pau Monne wrote:
>>> diff --git a/xen/drivers/passthrough/iommu.c 
>> b/xen/drivers/passthrough/iommu.c
>>> index cc12735..7fcbbb1 100644
>>> --- a/xen/drivers/passthrough/iommu.c
>>> +++ b/xen/drivers/passthrough/iommu.c
>>> @@ -332,7 +332,8 @@ void iommu_share_p2m_table(struct domain* d)
>>>  {
>>>      const struct iommu_ops *ops = iommu_get_ops();
>>>  
>>> -    if ( iommu_enabled && is_hvm_domain(d) )
>>> +    ASSERT( hap_enabled(d) );
>>
>> This line is breaking compilation on ARM.
>>
>> Shouldn't it be replaced by iommu_use_hap_pt?
> 
> No, that's a different thing. But shouldn't ARM have a stub
> hap_enabled() evaluating to constant true?

I'm not sure if we should introduce hap_enabled. It's not something that
we should used in general.

What are we trying to catch with this ASSERT? I guess wrong caller?

If so, every share_p2m callbacks have a check "if iommu_use_hap_pt()"
which contains a check to hap_enabled on x86.

So I don't think this check is worthwhile in the common iommu code.

Regards,

-- 
Julien Grall