| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240208082336.GE56027@unreal>
Date: Thu, 8 Feb 2024 10:23:36 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Konstantin Taranov <kotaranov@...ux.microsoft.com>
Cc: kotaranov@...rosoft.com, sharmaajay@...rosoft.com, longli@...rosoft.com,
jgg@...pe.ca, linux-rdma@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH rdma-next v1 1/1] RDMA/mana_ib: Fix bug in creation of
dma regions
On Wed, Feb 07, 2024 at 07:09:26AM -0800, Konstantin Taranov wrote:
> From: Konstantin Taranov <kotaranov@...rosoft.com>
>
> Dma registration was ignoring virtual addresses by setting it to 0.
> As a result, mana_ib could only register page-aligned memory.
> As well as, it could fail to produce dma regions with zero offset
> for WQs and CQs (e.g., page size is 8192 but address is only 4096
> bytes aligned), which is required by hardware.
>
> This patch takes into account the virtual address, allowing to create
> a dma region with any offset. For queues (e.g., WQs, CQs) that require
> dma regions with zero offset we add a flag to ensure zero offset.
>
> Signed-off-by: Konstantin Taranov <kotaranov@...rosoft.com>
> ---
> drivers/infiniband/hw/mana/cq.c | 3 ++-
> drivers/infiniband/hw/mana/main.c | 16 +++++++++++++---
> drivers/infiniband/hw/mana/mana_ib.h | 2 +-
> drivers/infiniband/hw/mana/mr.c | 2 +-
> drivers/infiniband/hw/mana/qp.c | 4 ++--
> drivers/infiniband/hw/mana/wq.c | 3 ++-
> 6 files changed, 21 insertions(+), 9 deletions(-)
You definitely advised to look at the Documentation/process/submitting-patches.rst guide.
1. First revision doesn't need to be v1.
2. One logical fix/change == one patch.
3. Fixes should have Fixes: tag in the commit message.
And I'm confident that the force_zero_offset change is not correct.
Thanks
>
> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> index 83d20c3f0..e35de6b92 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
> @@ -48,7 +48,8 @@ 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_gd_create_dma_region(mdev, cq->umem, &cq->gdma_region,
> + ucmd.buf_addr, true);
> if (err) {
> ibdev_dbg(ibdev,
> "Failed to create dma region for create cq, %d\n",
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index 29dd2438d..13a4d5ab4 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -302,7 +302,7 @@ mana_ib_gd_add_dma_region(struct mana_ib_dev *dev, struct gdma_context *gc,
> }
>
> int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> - mana_handle_t *gdma_region)
> + mana_handle_t *gdma_region, u64 virt, bool force_zero_offset)
> {
> struct gdma_dma_region_add_pages_req *add_req = NULL;
> size_t num_pages_processed = 0, num_pages_to_handle;
> @@ -324,11 +324,21 @@ int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
> 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);
> + 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 -ENOMEM;
> }
> +
> + if (force_zero_offset) {
> + while (ib_umem_dma_offset(umem, page_sz) && page_sz > PAGE_SIZE)
> + page_sz /= 2;
> + if (ib_umem_dma_offset(umem, page_sz) != 0) {
> + ibdev_dbg(&dev->ib_dev, "failed to find page size to force zero offset.\n");
> + return -ENOMEM;
> + }
> + }
> +
> num_pages_total = ib_umem_num_dma_blocks(umem, page_sz);
>
> max_pgs_create_cmd =
> @@ -348,7 +358,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;
>
> diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
> index 6a03ae645..0a5a8f3f8 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -161,7 +161,7 @@ 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);
> + mana_handle_t *gdma_region, u64 virt, bool force_zero_offset);
>
> 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..856d73ea2 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_gd_create_dma_region(dev, mr->umem, &dma_region_handle, iova, false);
> if (err) {
> ibdev_dbg(ibdev, "Failed create dma region for user-mr, %d\n",
> err);
> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 5d4c05dcd..02de90317 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_gd_create_dma_region(mdev, qp->sq_umem, &qp->sq_gdma_region,
> + ucmd.sq_buf_addr, true);
> if (err) {
> ibdev_dbg(&mdev->ib_dev,
> "Failed to create dma region for create qp-raw, %d\n",
> diff --git a/drivers/infiniband/hw/mana/wq.c b/drivers/infiniband/hw/mana/wq.c
> index 372d36151..d9c1a2d5d 100644
> --- a/drivers/infiniband/hw/mana/wq.c
> +++ b/drivers/infiniband/hw/mana/wq.c
> @@ -46,7 +46,8 @@ 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_gd_create_dma_region(mdev, wq->umem, &wq->gdma_region,
> + ucmd.wq_buf_addr, true);
> if (err) {
> ibdev_dbg(&mdev->ib_dev,
> "Failed to create dma region for create wq, %d\n",
>
> base-commit: aafe4cc5096996873817ff4981a3744e8caf7808
> --
> 2.43.0
>
Powered by blists - more mailing lists