On 04.08.2020 15:42, Paul Durrant wrote:
> From: Paul Durrant <[email protected]>
>
> 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