2024-02-26 07:26:12

by Konstantin Taranov

[permalink] [raw]
Subject: [PATCH rdma-next v2 0/2] RDMA/mana_ib: Improve dma region creation

From: Konstantin Taranov <[email protected]>

This patch series fixes an incorrect offset calculation for dma
regions and adds new functions to create dma regions:
1) with iova
2) without iova but with zero dma offset

Konstantin Taranov (2):
RDMA/mana_ib: Fix bug in creation of dma regions
RDMA/mana_ib: Use virtual address in dma regions for MRs

drivers/infiniband/hw/mana/cq.c | 4 +--
drivers/infiniband/hw/mana/main.c | 42 +++++++++++++++++++++-------
drivers/infiniband/hw/mana/mana_ib.h | 7 +++--
drivers/infiniband/hw/mana/mr.c | 4 +--
drivers/infiniband/hw/mana/qp.c | 6 ++--
drivers/infiniband/hw/mana/wq.c | 4 +--
6 files changed, 46 insertions(+), 21 deletions(-)


base-commit: 14b526f55ba5916856126f9793309fd6de5c5e7e
--
2.43.0



2024-02-26 07:26:21

by Konstantin Taranov

[permalink] [raw]
Subject: [PATCH rdma-next v2 1/2] RDMA/mana_ib: Fix bug in creation of dma regions

From: Konstantin Taranov <[email protected]>

Use ib_umem_dma_offset() helper to calculate correct dma offset.

Fixes: 0266a177631d ("RDMA/mana_ib: Add a driver for Microsoft Azure Network Adapter")
Signed-off-by: Konstantin Taranov <[email protected]>
---
drivers/infiniband/hw/mana/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index 29dd2438d..dd570832d 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -348,7 +348,7 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
sizeof(struct gdma_create_dma_region_resp));

create_req->length = umem->length;
- create_req->offset_in_page = umem->address & (page_sz - 1);
+ create_req->offset_in_page = ib_umem_dma_offset(umem, page_sz);
create_req->gdma_page_type = order_base_2(page_sz) - PAGE_SHIFT;
create_req->page_count = num_pages_total;

--
2.43.0


2024-02-26 07:26:26

by Konstantin Taranov

[permalink] [raw]
Subject: [PATCH rdma-next v2 2/2] RDMA/mana_ib: Use virtual address in dma regions for MRs

From: Konstantin Taranov <[email protected]>

Introduce mana_ib_create_dma_region() to create dma regions with iova
for MRs.

For dma regions that must have a zero dma offset (e.g., for queues),
mana_ib_create_zero_offset_dma_region() is added.
To get the zero offset, ib_umem_find_best_pgoff() is used with zero
pgoff_bitmask.

Signed-off-by: Konstantin Taranov <[email protected]>
---
drivers/infiniband/hw/mana/cq.c | 4 +--
drivers/infiniband/hw/mana/main.c | 40 +++++++++++++++++++++-------
drivers/infiniband/hw/mana/mana_ib.h | 7 +++--
drivers/infiniband/hw/mana/mr.c | 4 +--
drivers/infiniband/hw/mana/qp.c | 6 ++---
drivers/infiniband/hw/mana/wq.c | 4 +--
6 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index 83d20c3f0..4a71e678d 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -48,7 +48,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
return err;
}

- err = mana_ib_gd_create_dma_region(mdev, cq->umem, &cq->gdma_region);
+ err = mana_ib_create_zero_offset_dma_region(mdev, cq->umem, &cq->gdma_region);
if (err) {
ibdev_dbg(ibdev,
"Failed to create dma region for create cq, %d\n",
@@ -57,7 +57,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
}

ibdev_dbg(ibdev,
- "mana_ib_gd_create_dma_region ret %d gdma_region 0x%llx\n",
+ "create_dma_region ret %d gdma_region 0x%llx\n",
err, cq->gdma_region);

/*
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index dd570832d..30b874938 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -301,8 +301,8 @@ mana_ib_gd_add_dma_region(struct mana_ib_dev *dev, struct gdma_context *gc,
return 0;
}

-int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
- mana_handle_t *gdma_region)
+static int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
+ mana_handle_t *gdma_region, unsigned long page_sz)
{
struct gdma_dma_region_add_pages_req *add_req = NULL;
size_t num_pages_processed = 0, num_pages_to_handle;
@@ -314,7 +314,6 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
size_t max_pgs_create_cmd;
struct gdma_context *gc;
size_t num_pages_total;
- unsigned long page_sz;
unsigned int tail = 0;
u64 *page_addr_list;
void *request_buf;
@@ -323,12 +322,6 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
gc = mdev_to_gc(dev);
hwc = gc->hwc.driver_data;

- /* Hardware requires dma region to align to chosen page size */
- page_sz = ib_umem_find_best_pgsz(umem, PAGE_SZ_BM, 0);
- if (!page_sz) {
- ibdev_dbg(&dev->ib_dev, "failed to find page size.\n");
- return -ENOMEM;
- }
num_pages_total = ib_umem_num_dma_blocks(umem, page_sz);

max_pgs_create_cmd =
@@ -414,6 +407,35 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
return err;
}

+int mana_ib_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
+ mana_handle_t *gdma_region, u64 virt)
+{
+ unsigned long page_sz;
+
+ page_sz = ib_umem_find_best_pgsz(umem, PAGE_SZ_BM, virt);
+ if (!page_sz) {
+ ibdev_dbg(&dev->ib_dev, "Failed to find page size.\n");
+ return -EINVAL;
+ }
+
+ return mana_ib_gd_create_dma_region(dev, umem, gdma_region, page_sz);
+}
+
+int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
+ mana_handle_t *gdma_region)
+{
+ unsigned long page_sz;
+
+ /* Hardware requires dma region to align to chosen page size */
+ page_sz = ib_umem_find_best_pgoff(umem, PAGE_SZ_BM, 0);
+ if (!page_sz) {
+ ibdev_dbg(&dev->ib_dev, "Failed to find page size.\n");
+ return -ENOMEM;
+ }
+
+ return mana_ib_gd_create_dma_region(dev, umem, gdma_region, page_sz);
+}
+
int mana_ib_gd_destroy_dma_region(struct mana_ib_dev *dev, u64 gdma_region)
{
struct gdma_context *gc = mdev_to_gc(dev);
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index 6a03ae645..f83390eeb 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -160,8 +160,11 @@ static inline struct net_device *mana_ib_get_netdev(struct ib_device *ibdev, u32

int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq);

-int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
- mana_handle_t *gdma_region);
+int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
+ mana_handle_t *gdma_region);
+
+int mana_ib_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
+ mana_handle_t *gdma_region, u64 virt);

int mana_ib_gd_destroy_dma_region(struct mana_ib_dev *dev,
mana_handle_t gdma_region);
diff --git a/drivers/infiniband/hw/mana/mr.c b/drivers/infiniband/hw/mana/mr.c
index ee4d4f834..b70b13484 100644
--- a/drivers/infiniband/hw/mana/mr.c
+++ b/drivers/infiniband/hw/mana/mr.c
@@ -127,7 +127,7 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
goto err_free;
}

- err = mana_ib_gd_create_dma_region(dev, mr->umem, &dma_region_handle);
+ err = mana_ib_create_dma_region(dev, mr->umem, &dma_region_handle, iova);
if (err) {
ibdev_dbg(ibdev, "Failed create dma region for user-mr, %d\n",
err);
@@ -135,7 +135,7 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
}

ibdev_dbg(ibdev,
- "mana_ib_gd_create_dma_region ret %d gdma_region %llx\n", err,
+ "create_dma_region ret %d gdma_region %llx\n", err,
dma_region_handle);

mr_params.pd_handle = pd->pd_handle;
diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
index 5d4c05dcd..6e7627745 100644
--- a/drivers/infiniband/hw/mana/qp.c
+++ b/drivers/infiniband/hw/mana/qp.c
@@ -357,8 +357,8 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
}
qp->sq_umem = umem;

- err = mana_ib_gd_create_dma_region(mdev, qp->sq_umem,
- &qp->sq_gdma_region);
+ err = mana_ib_create_zero_offset_dma_region(mdev, qp->sq_umem,
+ &qp->sq_gdma_region);
if (err) {
ibdev_dbg(&mdev->ib_dev,
"Failed to create dma region for create qp-raw, %d\n",
@@ -367,7 +367,7 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
}

ibdev_dbg(&mdev->ib_dev,
- "mana_ib_gd_create_dma_region ret %d gdma_region 0x%llx\n",
+ "create_dma_region ret %d gdma_region 0x%llx\n",
err, qp->sq_gdma_region);

/* Create a WQ on the same port handle used by the Ethernet */
diff --git a/drivers/infiniband/hw/mana/wq.c b/drivers/infiniband/hw/mana/wq.c
index 372d36151..7c9c69962 100644
--- a/drivers/infiniband/hw/mana/wq.c
+++ b/drivers/infiniband/hw/mana/wq.c
@@ -46,7 +46,7 @@ struct ib_wq *mana_ib_create_wq(struct ib_pd *pd,
wq->wq_buf_size = ucmd.wq_buf_size;
wq->rx_object = INVALID_MANA_HANDLE;

- err = mana_ib_gd_create_dma_region(mdev, wq->umem, &wq->gdma_region);
+ err = mana_ib_create_zero_offset_dma_region(mdev, wq->umem, &wq->gdma_region);
if (err) {
ibdev_dbg(&mdev->ib_dev,
"Failed to create dma region for create wq, %d\n",
@@ -55,7 +55,7 @@ struct ib_wq *mana_ib_create_wq(struct ib_pd *pd,
}

ibdev_dbg(&mdev->ib_dev,
- "mana_ib_gd_create_dma_region ret %d gdma_region 0x%llx\n",
+ "create_dma_region ret %d gdma_region 0x%llx\n",
err, wq->gdma_region);

/* WQ ID is returned at wq_create time, doesn't know the value yet */
--
2.43.0


2024-03-02 02:58:45

by Long Li

[permalink] [raw]
Subject: RE: [PATCH rdma-next v2 2/2] RDMA/mana_ib: Use virtual address in dma regions for MRs

> Subject: [PATCH rdma-next v2 2/2] RDMA/mana_ib: Use virtual address in
> dma regions for MRs
>
> From: Konstantin Taranov <[email protected]>
>
> Introduce mana_ib_create_dma_region() to create dma regions with iova for
> MRs.
>
> For dma regions that must have a zero dma offset (e.g., for queues),
> mana_ib_create_zero_offset_dma_region() is added.
> To get the zero offset, ib_umem_find_best_pgoff() is used with zero
> pgoff_bitmask.
>
> Signed-off-by: Konstantin Taranov <[email protected]>

Do you need a "Fixes:" for this patch?

Also, you need to include a "change log" in the cover letter 0/2 since this is a v2 seires.

Long

> ---
> drivers/infiniband/hw/mana/cq.c | 4 +--
> drivers/infiniband/hw/mana/main.c | 40 +++++++++++++++++++++-------
> drivers/infiniband/hw/mana/mana_ib.h | 7 +++--
> drivers/infiniband/hw/mana/mr.c | 4 +--
> drivers/infiniband/hw/mana/qp.c | 6 ++---
> drivers/infiniband/hw/mana/wq.c | 4 +--
> 6 files changed, 45 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mana/cq.c
> b/drivers/infiniband/hw/mana/cq.c index 83d20c3f0..4a71e678d 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
> @@ -48,7 +48,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct
> ib_cq_init_attr *attr,
> return err;
> }
>
> - err = mana_ib_gd_create_dma_region(mdev, cq->umem, &cq-
> >gdma_region);
> + err = mana_ib_create_zero_offset_dma_region(mdev, cq->umem,
> +&cq->gdma_region);
> if (err) {
> ibdev_dbg(ibdev,
> "Failed to create dma region for create cq, %d\n",
> @@ -57,7 +57,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct
> ib_cq_init_attr *attr,
> }
>
> ibdev_dbg(ibdev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region
> 0x%llx\n",
> + "create_dma_region ret %d gdma_region 0x%llx\n",
> err, cq->gdma_region);
>
> /*
> diff --git a/drivers/infiniband/hw/mana/main.c
> b/drivers/infiniband/hw/mana/main.c
> index dd570832d..30b874938 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -301,8 +301,8 @@ mana_ib_gd_add_dma_region(struct mana_ib_dev
> *dev, struct gdma_context *gc,
> return 0;
> }
>
> -int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct
> ib_umem *umem,
> - mana_handle_t *gdma_region)
> +static int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct
> ib_umem *umem,
> + mana_handle_t *gdma_region,
> unsigned long page_sz)
> {
> struct gdma_dma_region_add_pages_req *add_req = NULL;
> size_t num_pages_processed = 0, num_pages_to_handle; @@ -314,7
> +314,6 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev,
> struct ib_umem *umem,
> size_t max_pgs_create_cmd;
> struct gdma_context *gc;
> size_t num_pages_total;
> - unsigned long page_sz;
> unsigned int tail = 0;
> u64 *page_addr_list;
> void *request_buf;
> @@ -323,12 +322,6 @@ int mana_ib_gd_create_dma_region(struct
> mana_ib_dev *dev, struct ib_umem *umem,
> gc = mdev_to_gc(dev);
> hwc = gc->hwc.driver_data;
>
> - /* Hardware requires dma region to align to chosen page size */
> - page_sz = ib_umem_find_best_pgsz(umem, PAGE_SZ_BM, 0);
> - if (!page_sz) {
> - ibdev_dbg(&dev->ib_dev, "failed to find page size.\n");
> - return -ENOMEM;
> - }
> num_pages_total = ib_umem_num_dma_blocks(umem, page_sz);
>
> max_pgs_create_cmd =
> @@ -414,6 +407,35 @@ int mana_ib_gd_create_dma_region(struct
> mana_ib_dev *dev, struct ib_umem *umem,
> return err;
> }
>
> +int mana_ib_create_dma_region(struct mana_ib_dev *dev, struct ib_umem
> *umem,
> + mana_handle_t *gdma_region, u64 virt) {
> + unsigned long page_sz;
> +
> + page_sz = ib_umem_find_best_pgsz(umem, PAGE_SZ_BM, virt);
> + if (!page_sz) {
> + ibdev_dbg(&dev->ib_dev, "Failed to find page size.\n");
> + return -EINVAL;
> + }
> +
> + return mana_ib_gd_create_dma_region(dev, umem, gdma_region,
> page_sz);
> +}
> +
> +int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev,
> struct ib_umem *umem,
> + mana_handle_t *gdma_region)
> +{
> + unsigned long page_sz;
> +
> + /* Hardware requires dma region to align to chosen page size */
> + page_sz = ib_umem_find_best_pgoff(umem, PAGE_SZ_BM, 0);
> + if (!page_sz) {
> + ibdev_dbg(&dev->ib_dev, "Failed to find page size.\n");
> + return -ENOMEM;
> + }
> +
> + return mana_ib_gd_create_dma_region(dev, umem, gdma_region,
> page_sz);
> +}
> +
> int mana_ib_gd_destroy_dma_region(struct mana_ib_dev *dev, u64
> gdma_region) {
> struct gdma_context *gc = mdev_to_gc(dev); diff --git
> a/drivers/infiniband/hw/mana/mana_ib.h
> b/drivers/infiniband/hw/mana/mana_ib.h
> index 6a03ae645..f83390eeb 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -160,8 +160,11 @@ static inline struct net_device
> *mana_ib_get_netdev(struct ib_device *ibdev, u32
>
> int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq
> *cq);
>
> -int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct
> ib_umem *umem,
> - mana_handle_t *gdma_region);
> +int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev,
> struct ib_umem *umem,
> + mana_handle_t *gdma_region);
> +
> +int mana_ib_create_dma_region(struct mana_ib_dev *dev, struct ib_umem
> *umem,
> + mana_handle_t *gdma_region, u64 virt);
>
> int mana_ib_gd_destroy_dma_region(struct mana_ib_dev *dev,
> mana_handle_t gdma_region);
> diff --git a/drivers/infiniband/hw/mana/mr.c
> b/drivers/infiniband/hw/mana/mr.c index ee4d4f834..b70b13484 100644
> --- a/drivers/infiniband/hw/mana/mr.c
> +++ b/drivers/infiniband/hw/mana/mr.c
> @@ -127,7 +127,7 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd
> *ibpd, u64 start, u64 length,
> goto err_free;
> }
>
> - err = mana_ib_gd_create_dma_region(dev, mr->umem,
> &dma_region_handle);
> + err = mana_ib_create_dma_region(dev, mr->umem,
> &dma_region_handle,
> +iova);
> if (err) {
> ibdev_dbg(ibdev, "Failed create dma region for user-mr,
> %d\n",
> err);
> @@ -135,7 +135,7 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd
> *ibpd, u64 start, u64 length,
> }
>
> ibdev_dbg(ibdev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region
> %llx\n", err,
> + "create_dma_region ret %d gdma_region %llx\n", err,
> dma_region_handle);
>
> mr_params.pd_handle = pd->pd_handle;
> diff --git a/drivers/infiniband/hw/mana/qp.c
> b/drivers/infiniband/hw/mana/qp.c index 5d4c05dcd..6e7627745 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -357,8 +357,8 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp,
> struct ib_pd *ibpd,
> }
> qp->sq_umem = umem;
>
> - err = mana_ib_gd_create_dma_region(mdev, qp->sq_umem,
> - &qp->sq_gdma_region);
> + err = mana_ib_create_zero_offset_dma_region(mdev, qp->sq_umem,
> + &qp->sq_gdma_region);
> if (err) {
> ibdev_dbg(&mdev->ib_dev,
> "Failed to create dma region for create qp-raw,
> %d\n", @@ -367,7 +367,7 @@ static int mana_ib_create_qp_raw(struct ib_qp
> *ibqp, struct ib_pd *ibpd,
> }
>
> ibdev_dbg(&mdev->ib_dev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region
> 0x%llx\n",
> + "create_dma_region ret %d gdma_region 0x%llx\n",
> err, qp->sq_gdma_region);
>
> /* Create a WQ on the same port handle used by the Ethernet */ diff -
> -git a/drivers/infiniband/hw/mana/wq.c b/drivers/infiniband/hw/mana/wq.c
> index 372d36151..7c9c69962 100644
> --- a/drivers/infiniband/hw/mana/wq.c
> +++ b/drivers/infiniband/hw/mana/wq.c
> @@ -46,7 +46,7 @@ struct ib_wq *mana_ib_create_wq(struct ib_pd *pd,
> wq->wq_buf_size = ucmd.wq_buf_size;
> wq->rx_object = INVALID_MANA_HANDLE;
>
> - err = mana_ib_gd_create_dma_region(mdev, wq->umem, &wq-
> >gdma_region);
> + err = mana_ib_create_zero_offset_dma_region(mdev, wq->umem,
> +&wq->gdma_region);
> if (err) {
> ibdev_dbg(&mdev->ib_dev,
> "Failed to create dma region for create wq, %d\n",
> @@ -55,7 +55,7 @@ struct ib_wq *mana_ib_create_wq(struct ib_pd *pd,
> }
>
> ibdev_dbg(&mdev->ib_dev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region
> 0x%llx\n",
> + "create_dma_region ret %d gdma_region 0x%llx\n",
> err, wq->gdma_region);
>
> /* WQ ID is returned at wq_create time, doesn't know the value yet */
> --
> 2.43.0


2024-03-02 05:34:54

by Zhu Yanjun

[permalink] [raw]
Subject: Re: [PATCH rdma-next v2 2/2] RDMA/mana_ib: Use virtual address in dma regions for MRs

在 2024/2/26 8:25, Konstantin Taranov 写道:
> From: Konstantin Taranov <[email protected]>
>
> Introduce mana_ib_create_dma_region() to create dma regions with iova
> for MRs.
>
> For dma regions that must have a zero dma offset (e.g., for queues),
> mana_ib_create_zero_offset_dma_region() is added.
> To get the zero offset, ib_umem_find_best_pgoff() is used with zero
> pgoff_bitmask.
>
> Signed-off-by: Konstantin Taranov <[email protected]>
> ---
> drivers/infiniband/hw/mana/cq.c | 4 +--
> drivers/infiniband/hw/mana/main.c | 40 +++++++++++++++++++++-------
> drivers/infiniband/hw/mana/mana_ib.h | 7 +++--
> drivers/infiniband/hw/mana/mr.c | 4 +--
> drivers/infiniband/hw/mana/qp.c | 6 ++---
> drivers/infiniband/hw/mana/wq.c | 4 +--
> 6 files changed, 45 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> index 83d20c3f0..4a71e678d 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
> @@ -48,7 +48,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> return err;
> }
>
> - err = mana_ib_gd_create_dma_region(mdev, cq->umem, &cq->gdma_region);
> + err = mana_ib_create_zero_offset_dma_region(mdev, cq->umem, &cq->gdma_region);
> if (err) {
> ibdev_dbg(ibdev,
> "Failed to create dma region for create cq, %d\n",
> @@ -57,7 +57,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> }
>
> ibdev_dbg(ibdev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region 0x%llx\n",
> + "create_dma_region ret %d gdma_region 0x%llx\n",
> err, cq->gdma_region);
>
> /*
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index dd570832d..30b874938 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -301,8 +301,8 @@ mana_ib_gd_add_dma_region(struct mana_ib_dev *dev, struct gdma_context *gc,
> return 0;
> }
>
> -int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> - mana_handle_t *gdma_region)
> +static int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> + mana_handle_t *gdma_region, unsigned long page_sz)
> {
> struct gdma_dma_region_add_pages_req *add_req = NULL;
> size_t num_pages_processed = 0, num_pages_to_handle;
> @@ -314,7 +314,6 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> size_t max_pgs_create_cmd;
> struct gdma_context *gc;
> size_t num_pages_total;
> - unsigned long page_sz;
> unsigned int tail = 0;
> u64 *page_addr_list;
> void *request_buf;
> @@ -323,12 +322,6 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> gc = mdev_to_gc(dev);
> hwc = gc->hwc.driver_data;
>
> - /* Hardware requires dma region to align to chosen page size */
> - page_sz = ib_umem_find_best_pgsz(umem, PAGE_SZ_BM, 0);
> - if (!page_sz) {
> - ibdev_dbg(&dev->ib_dev, "failed to find page size.\n");
> - return -ENOMEM;
> - }
> num_pages_total = ib_umem_num_dma_blocks(umem, page_sz);
>
> max_pgs_create_cmd =
> @@ -414,6 +407,35 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> return err;
> }
>
> +int mana_ib_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> + mana_handle_t *gdma_region, u64 virt)
> +{
> + unsigned long page_sz;
> +
> + page_sz = ib_umem_find_best_pgsz(umem, PAGE_SZ_BM, virt);
> + if (!page_sz) {
> + ibdev_dbg(&dev->ib_dev, "Failed to find page size.\n");
> + return -EINVAL;

<...>

> + }
> +
> + return mana_ib_gd_create_dma_region(dev, umem, gdma_region, page_sz);
> +}
> +
> +int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> + mana_handle_t *gdma_region)
> +{
> + unsigned long page_sz;
> +
> + /* Hardware requires dma region to align to chosen page size */
> + page_sz = ib_umem_find_best_pgoff(umem, PAGE_SZ_BM, 0);
> + if (!page_sz) {
> + ibdev_dbg(&dev->ib_dev, "Failed to find page size.\n");
> + return -ENOMEM;
> + }

A trivial problem.

The function ib_umem_find_best_pgoff is a wraper of the function
ib_umem_find_best_pgsz. In the above function mana_ib_create_dma_region,
when the function ib_umem_find_best_pgsz fails, the error is -EINVAL. In
this function mana_ib_create_zero_offset_dma_region, when
ib_umem_find_best_pgoff fails (actually ib_umem_find_best_pgsz fails),
error should also be -EINVAL?

This function ib_umem_find_best_pgsz is intended for HW that support
multiple page sizes but can do only a single page size in an MR.

Returns 0 if the umem requires page sizes not supported by the driver to
be mapped. Drivers always supporting PAGE_SIZE or smaller will never see
a 0 result.

So the above "return -ENOMEM;" should be "return -EINVAL;"?

Zhu Yanjun

> +
> + return mana_ib_gd_create_dma_region(dev, umem, gdma_region, page_sz);
> +}
> +
> int mana_ib_gd_destroy_dma_region(struct mana_ib_dev *dev, u64 gdma_region)
> {
> struct gdma_context *gc = mdev_to_gc(dev);
> diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
> index 6a03ae645..f83390eeb 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -160,8 +160,11 @@ static inline struct net_device *mana_ib_get_netdev(struct ib_device *ibdev, u32
>
> int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq);
>
> -int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> - mana_handle_t *gdma_region);
> +int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> + mana_handle_t *gdma_region);
> +
> +int mana_ib_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> + mana_handle_t *gdma_region, u64 virt);
>
> int mana_ib_gd_destroy_dma_region(struct mana_ib_dev *dev,
> mana_handle_t gdma_region);
> diff --git a/drivers/infiniband/hw/mana/mr.c b/drivers/infiniband/hw/mana/mr.c
> index ee4d4f834..b70b13484 100644
> --- a/drivers/infiniband/hw/mana/mr.c
> +++ b/drivers/infiniband/hw/mana/mr.c
> @@ -127,7 +127,7 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
> goto err_free;
> }
>
> - err = mana_ib_gd_create_dma_region(dev, mr->umem, &dma_region_handle);
> + err = mana_ib_create_dma_region(dev, mr->umem, &dma_region_handle, iova);
> if (err) {
> ibdev_dbg(ibdev, "Failed create dma region for user-mr, %d\n",
> err);
> @@ -135,7 +135,7 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
> }
>
> ibdev_dbg(ibdev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region %llx\n", err,
> + "create_dma_region ret %d gdma_region %llx\n", err,
> dma_region_handle);
>
> mr_params.pd_handle = pd->pd_handle;
> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 5d4c05dcd..6e7627745 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -357,8 +357,8 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
> }
> qp->sq_umem = umem;
>
> - err = mana_ib_gd_create_dma_region(mdev, qp->sq_umem,
> - &qp->sq_gdma_region);
> + err = mana_ib_create_zero_offset_dma_region(mdev, qp->sq_umem,
> + &qp->sq_gdma_region);
> if (err) {
> ibdev_dbg(&mdev->ib_dev,
> "Failed to create dma region for create qp-raw, %d\n",
> @@ -367,7 +367,7 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
> }
>
> ibdev_dbg(&mdev->ib_dev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region 0x%llx\n",
> + "create_dma_region ret %d gdma_region 0x%llx\n",
> err, qp->sq_gdma_region);
>
> /* Create a WQ on the same port handle used by the Ethernet */
> diff --git a/drivers/infiniband/hw/mana/wq.c b/drivers/infiniband/hw/mana/wq.c
> index 372d36151..7c9c69962 100644
> --- a/drivers/infiniband/hw/mana/wq.c
> +++ b/drivers/infiniband/hw/mana/wq.c
> @@ -46,7 +46,7 @@ struct ib_wq *mana_ib_create_wq(struct ib_pd *pd,
> wq->wq_buf_size = ucmd.wq_buf_size;
> wq->rx_object = INVALID_MANA_HANDLE;
>
> - err = mana_ib_gd_create_dma_region(mdev, wq->umem, &wq->gdma_region);
> + err = mana_ib_create_zero_offset_dma_region(mdev, wq->umem, &wq->gdma_region);
> if (err) {
> ibdev_dbg(&mdev->ib_dev,
> "Failed to create dma region for create wq, %d\n",
> @@ -55,7 +55,7 @@ struct ib_wq *mana_ib_create_wq(struct ib_pd *pd,
> }
>
> ibdev_dbg(&mdev->ib_dev,
> - "mana_ib_gd_create_dma_region ret %d gdma_region 0x%llx\n",
> + "create_dma_region ret %d gdma_region 0x%llx\n",
> err, wq->gdma_region);
>
> /* WQ ID is returned at wq_create time, doesn't know the value yet */