Re: [PATCH 4/6] remove remaining uses of iommu_legacy_map/unmap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.07.2020 18:46, Paul Durrant wrote:
> ---
>  xen/arch/x86/mm.c               | 22 +++++++++++++++-----
>  xen/arch/x86/mm/p2m-ept.c       | 22 +++++++++++++-------
>  xen/arch/x86/mm/p2m-pt.c        | 17 +++++++++++----
>  xen/arch/x86/mm/p2m.c           | 28 ++++++++++++++++++-------
>  xen/arch/x86/x86_64/mm.c        | 27 ++++++++++++++++++------
>  xen/common/grant_table.c        | 36 +++++++++++++++++++++++++-------
>  xen/common/memory.c             |  7 +++----
>  xen/drivers/passthrough/iommu.c | 37 +--------------------------------
>  xen/include/xen/iommu.h         | 20 +++++-------------
>  9 files changed, 123 insertions(+), 93 deletions(-)

Overall more code. I wonder whether a map-and-flush function (named
differently than the current ones) wouldn't still be worthwhile to
have.

> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1225,11 +1225,25 @@ map_grant_ref(
>              kind = IOMMUF_readable;
>          else
>              kind = 0;
> -        if ( kind && iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0, kind) )
> +        if ( kind )
>          {
> -            double_gt_unlock(lgt, rgt);
> -            rc = GNTST_general_error;
> -            goto undo_out;
> +            dfn_t dfn = _dfn(mfn_x(mfn));
> +            unsigned int flush_flags = 0;
> +            int err;
> +
> +            err = iommu_map(ld, dfn, mfn, 0, kind, &flush_flags);
> +            if ( err )
> +                rc = GNTST_general_error;
> +
> +            err = iommu_iotlb_flush(ld, dfn, 1, flush_flags);
> +            if ( err )
> +                rc = GNTST_general_error;
> +
> +            if ( rc != GNTST_okay )
> +            {
> +                double_gt_unlock(lgt, rgt);
> +                goto undo_out;
> +            }
>          }

The mapping needs to happen with at least ld's lock held, yes. But
is the same true also for the flushing? Can't (not necessarily
right in this change) the flush be pulled out of the function and
instead done once per batch that got processed?

Jan