Received: by 2002:a05:7412:3b8b:b0:fc:a2b0:25d7 with SMTP id nd11csp121358rdb; Thu, 8 Feb 2024 00:26:43 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCUCEVzV4ZiI3tYA5/GiE7cRTHQD8P89z2xo6d4ssVReFXkR8wi0dcmOR5nYu7dQVMZZXDp6ed1EaFV5p8Bs+KW9RaOOMPVukyd+en4uQg== X-Google-Smtp-Source: AGHT+IGNsrcFaLUjcDX4RobReqisqmuH6XiyXsCKpuaHpvtJtDsvbezCzwOcsrDLIfbRzoHsg1TU X-Received: by 2002:a05:620a:13d9:b0:783:cc18:995a with SMTP id g25-20020a05620a13d900b00783cc18995amr8005214qkl.68.1707380803010; Thu, 08 Feb 2024 00:26:43 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707380802; cv=pass; d=google.com; s=arc-20160816; b=uOG/ENAzxcORLhX9Qa6k2wA0E5M/Nf2zeKpygglSRwKlxQ13knie8wBLWQuGw1B1hN 408rZWwiyMeYc/YeFG+hQAMX/tJsdLCHrIhsz9za1R60LCjsPVZMiRuguE9FKcVm8lfX H0BPOy+zObQn7PRIxYf5y0gY57EeY3DrE2oOBiFwnbpYxC6aWBPuIzMPn2PaHTrtOs12 i99WIxFCS3hqGcP4DSFnYeQASiB9zUm8O9DjI+8yAKDAZEzgoQGbmdJHgDFqThLolJj3 H31KmEkRUaCBZHgTeoTszTssBFzW5IsLI3x6SORI+pLWoqARArdhHD4uu0o1wS2MvR+7 cMYQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date:dkim-signature; bh=ijcOwlswbWBtrmvs7EA8xHEIMCStsS93S0bsTH0yz5g=; fh=3iQ5/Y4a9GG9d+5SN3dBnW+oqNHtfOTK010u9YF8dp0=; b=BEs1po94RInigRAvH4BAmqa43pC8GI68eWVSfT9tntPWoxFS/oaKjloAqUGQOhBnLG gmjSfbJbcOe059vcZwRSQbxrUDDeiztFIO11RygFOpER7wEB2dRFxi3p1ngP/GMwDotM cYjOSw6ObbBFlZrabBeboeT0t200rOTgZW5AVeQvbIZdLENVf7YFk7MA3M6Q4FDxMTfG UlGbSVVLNHGMaHvfhZv1j6EJTSjhWFpfIDdIJP8VEEoep/Di5BplgWbr9VxwnCvlWZDd IxODPuJIXHIXxwT6PcijuI+Jh6goIczLvkXzbDQzz3aeadu1FOm9sYexUx+orC8SuTx7 ug8g==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=ujWyTSR6; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-57613-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-57613-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org X-Forwarded-Encrypted: i=2; AJvYcCUjGH+1Eik6CvCoDRuq73O1iAlOHiZODuaj9i+BtwCDJjP8ZviG4IPhkfx4/uhvxSpb9YN6C67RyyihNixuPT9h3dfDWdcC3wkjfVvgOA== Return-Path: Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [2604:1380:45d1:ec00::1]) by mx.google.com with ESMTPS id u24-20020a05620a085800b007859e49411bsi2585512qku.456.2024.02.08.00.26.42 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Feb 2024 00:26:42 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-57613-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=ujWyTSR6; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-57613-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-57613-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id A81EA1C20CAE for ; Thu, 8 Feb 2024 08:26:42 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 09A946A8A1; Thu, 8 Feb 2024 08:23:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ujWyTSR6" Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 006FF6A03F; Thu, 8 Feb 2024 08:23:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707380622; cv=none; b=hSfMFrEhAq3L7+5Te01OTdKpinAZkSJdEpAXH5MkUlTA0VwjUXvsaGQOgvrGYfTt7pRffuV5C8WTtUfYvjmpBh5+WsYIDNuWeh36gEkRad0l/upENa8845UXWPGofaDH0/hISHbRf2EGtSblR1A5t+8hN4teGqLib6QgsT89QOg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707380622; c=relaxed/simple; bh=yNox0Rvrke1TK9bvylXp7lj6RMB3YMY6EmLiHoovj74=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BOHOEHxRQwvsx3TcHeC3RQNSc/ujI86O/rHucYfhVdmURd+CU27mnQxMs+HdbHVg8zVOznLmHE73nMZNVFCQESE0gFZMqRX0AudtkGmwkdoOYAjOz/TQYkH7+qtCLUHrGTEJc2lXQ6/WwNIQgnS4ZV2M98JE1ZvCz/tDmvQe0v0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ujWyTSR6; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id D1FE9C433C7; Thu, 8 Feb 2024 08:23:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1707380621; bh=yNox0Rvrke1TK9bvylXp7lj6RMB3YMY6EmLiHoovj74=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ujWyTSR69cvBIEjMA6VF+800AGAB5hUvP7a/PQGvQVgh1lAWgWt22OYbaR87SABQt QnZn76mUoHGi7rfWsdwNGQHKPG0TRSGI9Zv3c1MfUx4wbzbT37GFPL3wmEDg5zLmv/ rXuLhIUILb8PD9EoqGIjnpsdhJi+UzEjSGzCTvESTq8jETnPvE6Gyt1SS9LvkHBYy7 8cTjNLBK14KyohT4mG3KqmaSTXHGxxy9xcBKiqYrcms0ScrXdKlob1JFmULgYKyOBm VOlI5NX+qAZh9JXMRdk5H4Tilt6XLMegvKbPxCD08R2fmQs+n/R7e+P/SVa9jQnT4y 4uhSZjn8SCyAQ== Date: Thu, 8 Feb 2024 10:23:36 +0200 From: Leon Romanovsky To: Konstantin Taranov Cc: kotaranov@microsoft.com, sharmaajay@microsoft.com, longli@microsoft.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH rdma-next v1 1/1] RDMA/mana_ib: Fix bug in creation of dma regions Message-ID: <20240208082336.GE56027@unreal> References: <1707318566-3141-1-git-send-email-kotaranov@linux.microsoft.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1707318566-3141-1-git-send-email-kotaranov@linux.microsoft.com> On Wed, Feb 07, 2024 at 07:09:26AM -0800, Konstantin Taranov wrote: > From: Konstantin Taranov > > 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 > --- > 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 >