Re: [PATCH v4 09/14] common/grant_table: batch flush I/O TLB

[Jan Beulich <jbeulich@suse.com>]

On 04.08.2020 15:42, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> 
> This patch avoids calling iommu_iotlb_flush() for each individual GNTTABOP and
> insteads calls iommu_iotlb_flush_all() at the end of the hypercall. This
> should mean batched map/unmap operations perform better but may be slightly
> detrimental to singleton performance.

I would strongly suggest keeping singleton operations do single-DFN flushes.

> @@ -1329,20 +1326,30 @@ gnttab_map_grant_ref(
>              return i;

This one line is part of a path which you can't bypass as far as flushing
is concerned. In this regard the description is also slightly misleading:
It's not just "at the end of the hypercall" when flushing needs doing,
but also on every preemption.

>          if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
> -            return -EFAULT;
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }
>  
> -        map_grant_ref(&op);
> +        map_grant_ref(&op, &flush_flags);
>  
>          if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) )
> -            return -EFAULT;
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }
>      }
>  
> -    return 0;
> +    err = iommu_iotlb_flush_all(current->domain, flush_flags);
> +    if ( !rc )
> +        rc = err;

Not sure how important it is to retain performance upon errors: Strictly
speaking there's no need to flush when i == 0 and rc != 0.

Jan