Re: [Xen-devel] [PATCH v1 06/10] iommu: Add extra use_iommu argument to iommu_domain_init()

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

Hi, Jan.

On Fri, May 12, 2017 at 5:31 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 10.05.17 at 16:03, <olekstysh@xxxxxxxxx> wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>
>> The presence of this flag lets us know that the guest
>> has devices which will most likely be used for passthrough
>> and as the result the use of IOMMU is expected for this domain.
>> In that case we have to call iommu_construct(), actually
>> what the real assign_device call usually does.
>>
>> As iommu_domain_init() is called with use_iommu flag being forced
>> to false for now, no functional change is intended for both ARM and x86.
>>
>> Basically, this patch is needed for non-shared IOMMUs on ARM only
>> since the non-shared IOMMUs on x86 are ok if iommu_construct() is called
>> later. But, in order to be more generic and for possible future optimization
>> make this change appliable for both platforms.
>
> I continue to be unconvinced that this is wanted / needed, as I
> continue to not see why shared vs unshared really matters here.
> After all we have both modes working on x86 without this flag.
I know. But, for ARM we need this hint. We can't reuse the "retrieving
mapping" code I moved to x86-specific part in patch #8 (due to lack of
M2P on ARM) .

>
>> @@ -142,7 +142,14 @@ int iommu_domain_init(struct domain *d)
>>          return 0;
>>
>>      hd->platform_ops = iommu_get_ops();
>> -    return hd->platform_ops->init(d);
>> +    ret = hd->platform_ops->init(d);
>> +    if ( ret )
>> +        return ret;
>> +
>> +    if ( use_iommu && !is_hardware_domain(d) )
>> +        ret = iommu_construct(d);
>
> You don't handle the -ERESTART you may (and likely will) get here
> or in the caller.
Indeed. I forgot about it.

I most likely rework this patch not to call iommu_construct at all.
But, there are an open questions here and in patch #7 I would like to
clarify the first.
1. Are you against extra arguments at all?
2. If this question still makes sense, Shall I add extra need_iommu
argument to "init" callback too and just pass thought
    incoming flag to IOMMU drivers? This change wouldn't affect x86 at
all since we set this flag for ARM only (see patch #9).
    For x86 this flag would be always treated as unused.

>
> Jan
>


-- 
Regards,

Oleksandr Tyshchenko

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