On 25/04/2022 09:32, Jan Beulich wrote:
> As of 68a8aa5d7264 ("iommu: make map and unmap take a page count,
> similar to flush") there's no need anymore to have a loop here.
>
> Suggested-by: Roger Pau Monné <[email protected]>
> Signed-off-by: Jan Beulich <[email protected]>
> ---
> v3: New.
>
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -308,11 +308,9 @@ int iommu_map(struct domain *d, dfn_t df
> d->domain_id, dfn_x(dfn_add(dfn, i)),
> mfn_x(mfn_add(mfn, i)), rc);
>
> - while ( i-- )
> - /* if statement to satisfy __must_check */
> - if ( iommu_call(hd->platform_ops, unmap_page, d, dfn_add(dfn, i),
> - flush_flags) )
> - continue;
> + /* while statement to satisfy __must_check */
> + while ( iommu_unmap(d, dfn, i, flush_flags) )
> + break;
How can this possibly be correct?
The map_page() calls are made one 4k page at a time, and this while loop
is undoing every iteration, one 4k page at a time.
Without this while loop, any failure after the first page will end up
not being unmapped.
~Andrew
