diff mbox series

[v1,15/16] vfio/ccw: don't group contiguous pages on 2K IDAWs

Message ID 20221121214056.1187700-16-farman@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series vfio/ccw: channel program cleanup | expand

Commit Message

Eric Farman Nov. 21, 2022, 9:40 p.m. UTC
The vfio_pin_pages() interface allows contiguous pages to be
pinned as a single request, which is great for the 4K pages
that are normally processed. Old IDA formats operate on 2K
chunks, which makes this logic more difficult.

Since these formats are rare, let's just invoke the page
pinning one-at-a-time, instead of trying to group them.
We can rework this code at a later date if needed.

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 drivers/s390/cio/vfio_ccw_cp.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Matthew Rosato Dec. 19, 2022, 8:40 p.m. UTC | #1
On 11/21/22 4:40 PM, Eric Farman wrote:
> The vfio_pin_pages() interface allows contiguous pages to be
> pinned as a single request, which is great for the 4K pages
> that are normally processed. Old IDA formats operate on 2K
> chunks, which makes this logic more difficult.
> 
> Since these formats are rare, let's just invoke the page
> pinning one-at-a-time, instead of trying to group them.
> We can rework this code at a later date if needed.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  drivers/s390/cio/vfio_ccw_cp.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 9527f3d8da77..3829c346583c 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -88,7 +88,7 @@ static int page_array_alloc(struct page_array *pa, unsigned int len)
>   * otherwise only clear pa->pa_nr
>   */
>  static void page_array_unpin(struct page_array *pa,
> -			     struct vfio_device *vdev, int pa_nr)
> +			     struct vfio_device *vdev, int pa_nr, bool unaligned)

Please add 'unaligned' the comment block above this function with a short explanation...

>  {
>  	int unpinned = 0, npage = 1;
>  
> @@ -97,7 +97,8 @@ static void page_array_unpin(struct page_array *pa,
>  		dma_addr_t *last = &first[npage];
>  
>  		if (unpinned + npage < pa_nr &&
> -		    *first + npage * PAGE_SIZE == *last) {
> +		    *first + npage * PAGE_SIZE == *last &&
> +		    !unaligned) {
>  			npage++;
>  			continue;
>  		}
> @@ -119,7 +120,7 @@ static void page_array_unpin(struct page_array *pa,
>   * If the pin request partially succeeds, or fails completely,
>   * all pages are left unpinned and a negative error value is returned.
>   */
> -static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
> +static int page_array_pin(struct page_array *pa, struct vfio_device *vdev, bool unaligned)

...  Also here.  Otherwise, I agree re-work can be done later since this is not a common case.

With those changes:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

>  {
>  	int pinned = 0, npage = 1;
>  	int ret = 0;
> @@ -129,7 +130,8 @@ static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
>  		dma_addr_t *last = &first[npage];
>  
>  		if (pinned + npage < pa->pa_nr &&
> -		    *first + npage * PAGE_SIZE == *last) {
> +		    *first + npage * PAGE_SIZE == *last &&
> +		    !unaligned) {
>  			npage++;
>  			continue;
>  		}
> @@ -151,14 +153,14 @@ static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
>  	return ret;
>  
>  err_out:
> -	page_array_unpin(pa, vdev, pinned);
> +	page_array_unpin(pa, vdev, pinned, unaligned);
>  	return ret;
>  }
>  
>  /* Unpin the pages before releasing the memory. */
> -static void page_array_unpin_free(struct page_array *pa, struct vfio_device *vdev)
> +static void page_array_unpin_free(struct page_array *pa, struct vfio_device *vdev, bool unaligned)
>  {
> -	page_array_unpin(pa, vdev, pa->pa_nr);
> +	page_array_unpin(pa, vdev, pa->pa_nr, unaligned);
>  	kfree(pa->pa_page);
>  	kfree(pa->pa_iova);
>  }
> @@ -638,7 +640,7 @@ static int ccwchain_fetch_ccw(struct ccw1 *ccw,
>  	}
>  
>  	if (ccw_does_data_transfer(ccw)) {
> -		ret = page_array_pin(pa, vdev);
> +		ret = page_array_pin(pa, vdev, idal_is_2k(cp));
>  		if (ret < 0)
>  			goto out_unpin;
>  	} else {
> @@ -654,7 +656,7 @@ static int ccwchain_fetch_ccw(struct ccw1 *ccw,
>  	return 0;
>  
>  out_unpin:
> -	page_array_unpin_free(pa, vdev);
> +	page_array_unpin_free(pa, vdev, idal_is_2k(cp));
>  out_free_idaws:
>  	kfree(idaws);
>  out_init:
> @@ -752,7 +754,7 @@ void cp_free(struct channel_program *cp)
>  	cp->initialized = false;
>  	list_for_each_entry_safe(chain, temp, &cp->ccwchain_list, next) {
>  		for (i = 0; i < chain->ch_len; i++) {
> -			page_array_unpin_free(chain->ch_pa + i, vdev);
> +			page_array_unpin_free(chain->ch_pa + i, vdev, idal_is_2k(cp));
>  			ccwchain_cda_free(chain, i);
>  		}
>  		ccwchain_free(chain);
diff mbox series

Patch

diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index 9527f3d8da77..3829c346583c 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -88,7 +88,7 @@  static int page_array_alloc(struct page_array *pa, unsigned int len)
  * otherwise only clear pa->pa_nr
  */
 static void page_array_unpin(struct page_array *pa,
-			     struct vfio_device *vdev, int pa_nr)
+			     struct vfio_device *vdev, int pa_nr, bool unaligned)
 {
 	int unpinned = 0, npage = 1;
 
@@ -97,7 +97,8 @@  static void page_array_unpin(struct page_array *pa,
 		dma_addr_t *last = &first[npage];
 
 		if (unpinned + npage < pa_nr &&
-		    *first + npage * PAGE_SIZE == *last) {
+		    *first + npage * PAGE_SIZE == *last &&
+		    !unaligned) {
 			npage++;
 			continue;
 		}
@@ -119,7 +120,7 @@  static void page_array_unpin(struct page_array *pa,
  * If the pin request partially succeeds, or fails completely,
  * all pages are left unpinned and a negative error value is returned.
  */
-static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
+static int page_array_pin(struct page_array *pa, struct vfio_device *vdev, bool unaligned)
 {
 	int pinned = 0, npage = 1;
 	int ret = 0;
@@ -129,7 +130,8 @@  static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
 		dma_addr_t *last = &first[npage];
 
 		if (pinned + npage < pa->pa_nr &&
-		    *first + npage * PAGE_SIZE == *last) {
+		    *first + npage * PAGE_SIZE == *last &&
+		    !unaligned) {
 			npage++;
 			continue;
 		}
@@ -151,14 +153,14 @@  static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
 	return ret;
 
 err_out:
-	page_array_unpin(pa, vdev, pinned);
+	page_array_unpin(pa, vdev, pinned, unaligned);
 	return ret;
 }
 
 /* Unpin the pages before releasing the memory. */
-static void page_array_unpin_free(struct page_array *pa, struct vfio_device *vdev)
+static void page_array_unpin_free(struct page_array *pa, struct vfio_device *vdev, bool unaligned)
 {
-	page_array_unpin(pa, vdev, pa->pa_nr);
+	page_array_unpin(pa, vdev, pa->pa_nr, unaligned);
 	kfree(pa->pa_page);
 	kfree(pa->pa_iova);
 }
@@ -638,7 +640,7 @@  static int ccwchain_fetch_ccw(struct ccw1 *ccw,
 	}
 
 	if (ccw_does_data_transfer(ccw)) {
-		ret = page_array_pin(pa, vdev);
+		ret = page_array_pin(pa, vdev, idal_is_2k(cp));
 		if (ret < 0)
 			goto out_unpin;
 	} else {
@@ -654,7 +656,7 @@  static int ccwchain_fetch_ccw(struct ccw1 *ccw,
 	return 0;
 
 out_unpin:
-	page_array_unpin_free(pa, vdev);
+	page_array_unpin_free(pa, vdev, idal_is_2k(cp));
 out_free_idaws:
 	kfree(idaws);
 out_init:
@@ -752,7 +754,7 @@  void cp_free(struct channel_program *cp)
 	cp->initialized = false;
 	list_for_each_entry_safe(chain, temp, &cp->ccwchain_list, next) {
 		for (i = 0; i < chain->ch_len; i++) {
-			page_array_unpin_free(chain->ch_pa + i, vdev);
+			page_array_unpin_free(chain->ch_pa + i, vdev, idal_is_2k(cp));
 			ccwchain_cda_free(chain, i);
 		}
 		ccwchain_free(chain);