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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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:
>>> --- 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.
> 
> 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?

Jan


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