Re: [Xen-devel] [PATCH v2 07/13] iommu: Make decision about needing IOMMU for hardware domains in advance

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

Hi, Jan

On Thu, Dec 7, 2017 at 3:57 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 07.12.17 at 14:50, <olekstysh@xxxxxxxxx> wrote:
>> On Thu, Dec 7, 2017 at 10:57 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>> On 06.12.17 at 20:23, <olekstysh@xxxxxxxxx> wrote:
>>>> On Wed, Dec 6, 2017 at 7:01 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>>> On 25.07.17 at 19:26, <olekstysh@xxxxxxxxx> wrote:
>>>>>> @@ -175,37 +182,6 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>>>>>>          return;
>>>>>>
>>>>>>      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_pages(d, gfn, mfn, 0, 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);
>>>>>> -    }
>>>>>>
>>>>>>      return hd->platform_ops->hwdom_init(d);
>>>>>>  }
>>>>>
>>>>> Just to double check - this change was tested on x86 Dom0, at
>>>>> least PV (for PVH I'd at least expect that you've did some static
>>>>> code analysis to make sure this doesn't put in further roadblocks)?
>>>>
>>>> I am afraid I didn't get the second part of this sentence.
>>>
>>> Understandably, since I've broken grammar in the course of
>>> re-phrasing a number of times before sending. Dom0 PVH isn't
>>> complete at this point, so I can't ask you to actually test it. But
>>> I want to be reasonably certain that the change you make won't
>>> further complicate this enabling work (you may want to also Cc
>>> Roger on future versions of the patch for this very reason), the
>>> more that on AMD we've been unconditionally using non-shared
>>> page tables for quite some time. (In fact I see chances that the
>>> change might actually help the time it takes to set up PVH Dom0,
>>> especially when a sufficiently large chunk of memory is being
>>> handed to it.)
>>
>> As I understand, the current patch was tested on x86 with PV dom0
>> (thanks for doing that),
>
> This sounds as if you believe I would have tested anything. I
> certainly didn't (or at least I don't recall), and never meant to.
>
>> but wasn't
>> tested with PVH dom0 since the latter wasn't ready. And there is some
>> activity for bringing PVH dom0
>> which the current patch may affect in a negative way (complicate, brake,
>> etc).
>>
>> What pending patch(es) or a part of already existing code on x86
>> should I pay special attention to?
>
> The question is not so much pending patches, but making sure your
> changes don't adversely affect what's already in the tree.
Sure.

> Beyond  that I'll defer to Roger.

>
>> Sorry for the maybe naive question, but what should be done from my
>> side for this patch to be accepted,
>> except addressing comments regarding the patch itself?
>
> You will want (need) to assess the impact of your changes on
> code paths you can't possibly test.
Sure.

I would like to clarify: I haven't tested this patch on x86. Only on ARM.

>
> Jan
>



-- 
Regards,

Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel