Re: [Xen-devel] [PATCH v1 08/10] iommu: Split iommu_hwdom_init() into arch specific parts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.05.17 at 20:06, <olekstysh@xxxxxxxxx> wrote:
> On Thu, May 18, 2017 at 11:53 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 17.05.17 at 22:30, <julien.grall@xxxxxxx> wrote:
>>> On 05/17/2017 07:51 PM, Oleksandr Tyshchenko wrote:
>>>> On Wed, May 17, 2017 at 7:01 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> Well, if the ARM maintainers insist on baking their own thing every
>>>>> time we'd use the M2P if it was there, I think I can't reasonably
>>>>> block this patch. Otoh I'd prefer to not see the non-x86-specific
>>>>> code move to x86/, so perhaps the whole patch wants
>>>>> re-structuring using either a manifest definition in the ARM headers
>>>>> or e.g. CONFIG_M2P (which x86 would select, but ARM wouldn't).
>>>> Jan, I am afraid but I didn't get what you meant here:
>>>> "manifest definition in the ARM headers"
>>>
>>> I think he meant to have either a define in the header mentioning the
>>> absence/presence of M2P.
>>
>> Yes, at least in a way.
>>
>>> But my preference would be using the Kconfig here.
>>
>> Depends on the symbol used: If such a symbol solely _indicates_
>> the presence, Kconfig would be better indeed. If, however, the
>> symbol is e.g. a macro resolving to a per-arch implementation,
>> with common code providing a default definition when the arch
>> doesn't provide any, then the non-Kconfig variant may be
>> preferable.
> 
> Thank you for your comments.
> I have already posted a common comment regarding several patches in
> the current series
> as they are interrelated (please see patch #6), but I duplicate here
> only related to this patch part.
> 
> ...
> patch #8: As we always allocate the page table for hardware domain,
> this patch should be reworked.
> The use_iommu flag should be set for both archs in case of hardware
> domain. Having d->need_iommu set at the early stage we won't skip
> IOMMU mapping updates anymore. And as the result the existing in
> iommu_hwdom_init() code that goes through the list of page and tries
> to retrieve mapping could be just dropped
> instead of moving it to the arch-specific part.
> ...
> 
> Does the described above make sense?

As just said in the other reply - only if there weren't all these extra
overrides (one of which is even on by default, albeit we've been
discussing recently whether that's actually [still] appropriate).

Jan


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