Re: [PATCH v4 07/14] iommu: make map, unmap and flush all take both an order and a count

[Jan Beulich <jbeulich@suse.com>]

On 04.08.2020 15:42, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> 
> At the moment iommu_map() and iommu_unmap() take a page order but not a
> count, whereas iommu_iotlb_flush() takes a count but not a page order.
> This patch simply makes them consistent with each other.

Why can't we do with just a count, where order gets worked out by
functions knowing how to / wanting to deal with higher order pages?

> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -843,7 +843,7 @@ out:
>           need_modify_vtd_table )
>      {
>          if ( iommu_use_hap_pt(d) )
> -            rc = iommu_iotlb_flush(d, _dfn(gfn), (1u << order),
> +            rc = iommu_iotlb_flush(d, _dfn(gfn), (1u << order), 1,

Forgot to drop the "1 << "? (There are then I think two more instances
further down.)

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -851,12 +851,12 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
>  
>          this_cpu(iommu_dont_flush_iotlb) = 0;
>  
> -        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done,
> +        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), 0, done,

Arguments wrong way round? (This risk of inverting their order is
one of the primary reasons why I think we want just a count.) I'm
also uncertain about the use of 0 vs PAGE_ORDER_4K here.

>                                  IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
>          if ( unlikely(ret) && rc >= 0 )
>              rc = ret;
>  
> -        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done,
> +        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), 0, done,

Same here then.

Jan