Re: [Xen-devel] [RFC PATCH 8/9] iommu: Split iommu_hwdom_init() into arch specific parts

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On Wed, Apr 19, 2017 at 9:09 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi,
Hi, Julien.

>
> Sorry for the late answer.
>
> On 23/03/17 12:40, Oleksandr Tyshchenko wrote:
>>
>> On Thu, Mar 23, 2017 at 11:08 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>
>>>>>> On 22.03.17 at 19:40, <olekstysh@xxxxxxxxx> wrote:
>>>>
>>>> On Wed, Mar 22, 2017 at 5:54 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>>>
>>>>>>>> On 15.03.17 at 21:05, <olekstysh@xxxxxxxxx> wrote:
>>>>>
>>>>> The code being moved from here has survived the previous separation
>>>>> of arch-independent from arch-specific code, and looking at the code
>>>>> I also can't immediately see what's x86-specific here, so please make
>>>>> the description explain why the code as is can't be used.
>>>>
>>>>
>>>> You are right, there is nothing x86-specific here at first sight.
>>>> The reason why I moved this code to x86 folder is that we don't need
>>>> to retrieve IOMMU mappings on ARM this way. With need_iommu being
>>>> explicitly set at the hardware domain creation time
>>>> we just need to ask unshared IOMMU driver to allocate its page table
>>>> to be ready to receive and process IOMMU mappings from P2M code.
>>>>
>>>> Other points that had prevented me from using it as is.
>>>> If the hardware domain isn't 1:1 mapped we won't know gfn. IIRC, we
>>>> can find mfn by gfn for particular domain on ARM, but not vise versa.
>>>> Also this iommu_hwdom_init() is being called before allocating domain
>>>> memory on ARM. What is the point?
>>>> So, the d->page_list is empty during it execution.
>>>
>>>
>>> In which case the question even more so is - why move the code
>>> if it simply does nothing in your case?
>
>
> I didn't move the code, because back then we were only planning to support
> shared page table (iommu_use_hap_pt(...) always returns true on ARM so far).
>
> With more perspective, this code cannot work on ARM because of mfn_to_gmfn
> (we don't have an M2P). If we keep the code like that after this series,
> this will at least expose a bug (the helper always assume a direct mapping).
Agree.

>
> So I think moving the code is the right solution.
>
>>
>> Reasonable question. Before answering you I would like to clarify the
>> reason
>> why the iommu_hwdom_init() is being called before allocating domain
>> memory on ARM.
>> Is it a bug or there is an explanation for doing this.
>
>
> No real reason, I didn't see a reason to call this function later on. I
> would be interested to know whether there is a latent bug.
>
> Anyway, I think this is a good idea to fully initialize the IOMMU early for
> DOM0 as the builder will take care of assigning the non-PCI device
> protected.
Does this patch do right things or I have missed something?

>
> Cheers,
>
> --
> Julien Grall



-- 
Regards,

Oleksandr Tyshchenko

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