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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.07.17 at 19:26, <olekstysh@xxxxxxxxx> wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> 
> The hardware domains require IOMMU to be used in the most cases and
> a decision to use it is made at hardware domain construction time.
> But, it is not the best moment for the non-shared IOMMUs due to
> the necessity of retrieving all mapping which could happen in a period
> of time between IOMMU per-domain initialization and this moment.

Which mappings are you talking about here? Just like with the earlier
patch - the reason for the change needs to be clear to someone
reading just this commit message.

> @@ -141,6 +141,15 @@ int iommu_domain_init(struct domain *d, bool use_iommu)
>      if ( !iommu_enabled )
>          return 0;
>  
> +    if ( is_hardware_domain(d) )
> +    {
> +        if ( (paging_mode_translate(d) && !iommu_passthrough) ||
> +              iommu_dom0_strict )
> +            use_iommu = 1;
> +        else
> +            use_iommu = 0;

I'd prefer if you used a simple assignment here, rather than if/else.

> @@ -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)?

Jan


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