On Tue, 2022-07-05 at 23:27 -0700, Nicolin Chen wrote:
> This driver is the only caller of vfio_pin/unpin_pages that might
> pass
> in a non-contiguous PFN list, but in many cases it has a contiguous
> PFN
> list to process. So letting VFIO API handle a non-contiguous PFN list
> is actually counterproductive.
>
> Add a pair of simple loops to pass in contiguous PFNs only, to have
> an
> efficient implementation in VFIO.
>
> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
Reviewed-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
> drivers/s390/cio/vfio_ccw_cp.c | 70 +++++++++++++++++++++++++++-----
> --
> 1 file changed, 56 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/s390/cio/vfio_ccw_cp.c
> b/drivers/s390/cio/vfio_ccw_cp.c
> index 0c2be9421ab7..3b94863ad24e 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -90,6 +90,38 @@ static int pfn_array_alloc(struct pfn_array *pa,
> u64 iova, unsigned int len)
> return 0;
> }
>
> +/*
> + * pfn_array_unpin() - Unpin user pages in memory
> + * @pa: pfn_array on which to perform the operation
> + * @vdev: the vfio device to perform the operation
> + * @pa_nr: number of user pages to unpin
> + *
> + * Only unpin if any pages were pinned to begin with, i.e. pa_nr >
> 0,
> + * otherwise only clear pa->pa_nr
> + */
> +static void pfn_array_unpin(struct pfn_array *pa,
> + struct vfio_device *vdev, int pa_nr)
> +{
> + int unpinned = 0, npage = 1;
> +
> + while (unpinned < pa_nr) {
> + unsigned long *first = &pa->pa_iova_pfn[unpinned];
> + unsigned long *last = &first[npage];
> +
> + if (unpinned + npage < pa_nr &&
> + *first + npage == *last) {
> + npage++;
> + continue;
> + }
> +
> + vfio_unpin_pages(vdev, first, npage);
> + unpinned += npage;
> + npage = 1;
> + }
> +
> + pa->pa_nr = 0;
> +}
> +
> /*
> * pfn_array_pin() - Pin user pages in memory
> * @pa: pfn_array on which to perform the operation
> @@ -101,34 +133,44 @@ static int pfn_array_alloc(struct pfn_array
> *pa, u64 iova, unsigned int len)
> */
> static int pfn_array_pin(struct pfn_array *pa, struct vfio_device
> *vdev)
> {
> + int pinned = 0, npage = 1;
> int ret = 0;
>
> - ret = vfio_pin_pages(vdev, pa->pa_iova_pfn, pa->pa_nr,
> - IOMMU_READ | IOMMU_WRITE, pa->pa_pfn);
> + while (pinned < pa->pa_nr) {
> + unsigned long *first = &pa->pa_iova_pfn[pinned];
> + unsigned long *last = &first[npage];
>
> - if (ret < 0) {
> - goto err_out;
> - } else if (ret > 0 && ret != pa->pa_nr) {
> - vfio_unpin_pages(vdev, pa->pa_iova_pfn, ret);
> - ret = -EINVAL;
> - goto err_out;
> + if (pinned + npage < pa->pa_nr &&
> + *first + npage == *last) {
> + npage++;
> + continue;
> + }
> +
> + ret = vfio_pin_pages(vdev, first, npage,
> + IOMMU_READ | IOMMU_WRITE,
> + &pa->pa_pfn[pinned]);
> + if (ret < 0) {
> + goto err_out;
> + } else if (ret > 0 && ret != npage) {
> + pinned += ret;
> + ret = -EINVAL;
> + goto err_out;
> + }
> + pinned += npage;
> + npage = 1;
> }
>
> return ret;
>
> err_out:
> - pa->pa_nr = 0;
> -
> + pfn_array_unpin(pa, vdev, pinned);
> return ret;
> }
>
> /* Unpin the pages before releasing the memory. */
> static void pfn_array_unpin_free(struct pfn_array *pa, struct
> vfio_device *vdev)
> {
> - /* Only unpin if any pages were pinned to begin with */
> - if (pa->pa_nr)
> - vfio_unpin_pages(vdev, pa->pa_iova_pfn, pa->pa_nr);
> - pa->pa_nr = 0;
> + pfn_array_unpin(pa, vdev, pa->pa_nr);
> kfree(pa->pa_iova_pfn);
> }
>
