[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 23.03.17 at 13:40, <olekstysh@xxxxxxxxx> 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:
>>>>> --- 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?
> 
> 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.

Well, as that's different from x86 I'm afraid I'm the wrong one to ask
- the ARM maintainers would likely be in a much better position to
explain. The code sequence in each arch's construct_dom0() is
pretty clearly very different in this regard (ARM calls the function
almost first, while x86 calls it almost last.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.