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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.03.15 at 14:18, <julien.grall@xxxxxxxxxx> wrote:
> On 10/03/15 13:06, Jan Beulich wrote:
>>>>> On 10.03.15 at 13:51, <julien.grall@xxxxxxxxxx> 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.

Ah, right, I mixed this up with iommu_hap_pt_share. Roger - looks
like this could indeed be replaced by

     if ( iommu_enabled && iommu_use_hap_pt(d) )
         ops->share_p2m(d);

and the corresponding check in VT-d and AMD Vi code could then
also be dropped.

Jan


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