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

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On Wed, Mar 22, 2017 at 5:54 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 15.03.17 at 21:05, <olekstysh@xxxxxxxxx> wrote:
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -177,36 +177,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>>
>>      register_keyhandler('o', &iommu_dump_p2m_table, "dump iommu p2m table", 
>> 0);
>>      d->need_iommu = !!iommu_dom0_strict;
>> -    if ( need_iommu(d) && !iommu_use_hap_pt(d) )
>> -    {
>> -        struct page_info *page;
>> -        unsigned int i = 0;
>> -        int rc = 0;
>>
>> -        page_list_for_each ( page, &d->page_list )
>> -        {
>> -            unsigned long mfn = page_to_mfn(page);
>> -            unsigned long gfn = mfn_to_gmfn(d, mfn);
>> -            unsigned int mapping = IOMMUF_readable;
>> -            int ret;
>> -
>> -            if ( ((page->u.inuse.type_info & PGT_count_mask) == 0) ||
>> -                 ((page->u.inuse.type_info & PGT_type_mask)
>> -                  == PGT_writable_page) )
>> -                mapping |= IOMMUF_writable;
>> -
>> -            ret = hd->platform_ops->map_page(d, gfn, mfn, mapping);
>> -            if ( !rc )
>> -                rc = ret;
>> -
>> -            if ( !(i++ & 0xfffff) )
>> -                process_pending_softirqs();
>> -        }
>> -
>> -        if ( rc )
>> -            printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n",
>> -                   d->domain_id, rc);
>> -    }
>> +    arch_iommu_hwdom_init(d);
>
> 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.
>
> Jan
>

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.

-- 
Regards,

Oleksandr Tyshchenko

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