Re: [Xen-devel] [PATCH v5 05/15] iommu: don't domain_crash() inside iommu_map/unmap_page()

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx]
> Sent: Saturday, August 4, 2018 1:22 AM
> 
> Turn iommu_map/unmap_page() into straightforward wrappers that check
> the

iommu_iotlb_flush is also changed.

> existence of the relevant iommu_op and call through to it. This makes them
> usable by PV IOMMU code to be delivered in future patches.
> Leave the decision on whether to invoke domain_crash() up to the caller.
> This has the added benefit that the (module/line number) message that
> domain_crash() spits out will be more indicative of where the problem lies.
> 
> NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry()
>       replacing use of p2m->domain with the domain pointer passed into the
>       function.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Reviewed-by: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>, with one small comment:

> 
>      if ( need_iommu(p2m->domain) &&
>           (lpae_valid(orig_pte) || lpae_valid(*entry)) )
> +    {
>          rc = iommu_iotlb_flush(p2m->domain, _bfn(gfn_x(sgfn)),
>                                 1UL << page_order);
> +        if ( unlikely(rc) && !is_hardware_domain(p2m->domain) )
> +            domain_crash(p2m->domain);
> +    }

to reduce duplication, does it make sense to introduce a wrapper
like domain_crash_nd ('nd' indicate !is_hardware_domain, and
becomes a nop for hardware domain)? Then it becomes:

        if ( unlikely(rc) )
                domain_crash_nd(p2m->domain);

Thanks
Kevin

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