[PATCH rdma-next 0/2] RDMA/mana_ib: Enable DMA-mapped memory regions

[PATCH rdma-next 0/2] RDMA/mana_ib: Enable DMA-mapped memory regions

[Konstantin Taranov]

From: Konstantin Taranov <kotaranov@microsoft.com>

This patch series enables creation of DMA-mapped memory regions.
It allows GPA creation in kernel-level PDs and implements get_dma_mr().
Note, mana_ib_get_dma_mr was already declared, but not implemented.

Konstantin Taranov (2):
  RDMA/mana_ib: Allow registration of DMA-mapped memory in PDs
  RDMA/mana_ib: Implement get_dma_mr

 drivers/infiniband/hw/mana/device.c |  1 +
 drivers/infiniband/hw/mana/main.c   |  3 +++
 drivers/infiniband/hw/mana/mr.c     | 36 +++++++++++++++++++++++++++++
 include/net/mana/gdma.h             |  6 +++++
 4 files changed, 46 insertions(+)


base-commit: dfcdb38b21e4fb92a49acdbdf6afa82c07c8eba0
-- 
2.43.0

[PATCH rdma-next 1/2] RDMA/mana_ib: Allow registration of DMA-mapped memory in PDs

[Konstantin Taranov]

From: Konstantin Taranov <kotaranov@microsoft.com>

Allow the HW to register DMA-mapped memory for kernel-level PDs.

Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
---
 drivers/infiniband/hw/mana/main.c | 3 +++
 include/net/mana/gdma.h           | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index b31dcff32699..820af42d1fe1 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -82,6 +82,9 @@ int mana_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
 	mana_gd_init_req_hdr(&req.hdr, GDMA_CREATE_PD, sizeof(req),
 			     sizeof(resp));
 
+	if (!udata)
+		flags |= GDMA_PD_FLAG_ALLOW_GPA_MR;
+
 	req.flags = flags;
 	err = mana_gd_send_request(gc, sizeof(req), &req,
 				   sizeof(resp), &resp);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 27684135bb4d..8d796a30ddde 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -762,6 +762,7 @@ struct gdma_destroy_dma_region_req {
 
 enum gdma_pd_flags {
 	GDMA_PD_FLAG_INVALID = 0,
+	GDMA_PD_FLAG_ALLOW_GPA_MR = 1,
 };
 
 struct gdma_create_pd_req {
-- 
2.43.0

[PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Konstantin Taranov]

From: Konstantin Taranov <kotaranov@microsoft.com>

Implement allocation of DMA-mapped memory regions.

Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
---
 drivers/infiniband/hw/mana/device.c |  1 +
 drivers/infiniband/hw/mana/mr.c     | 36 +++++++++++++++++++++++++++++
 include/net/mana/gdma.h             |  5 ++++
 3 files changed, 42 insertions(+)

diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
index 6fa902ee80a6..043cef09f1c2 100644
--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c
@@ -29,6 +29,7 @@ static const struct ib_device_ops mana_ib_dev_ops = {
 	.destroy_rwq_ind_table = mana_ib_destroy_rwq_ind_table,
 	.destroy_wq = mana_ib_destroy_wq,
 	.disassociate_ucontext = mana_ib_disassociate_ucontext,
+	.get_dma_mr = mana_ib_get_dma_mr,
 	.get_port_immutable = mana_ib_get_port_immutable,
 	.mmap = mana_ib_mmap,
 	.modify_qp = mana_ib_modify_qp,
diff --git a/drivers/infiniband/hw/mana/mr.c b/drivers/infiniband/hw/mana/mr.c
index 4f13423ecdbd..7c9394926a18 100644
--- a/drivers/infiniband/hw/mana/mr.c
+++ b/drivers/infiniband/hw/mana/mr.c
@@ -8,6 +8,8 @@
 #define VALID_MR_FLAGS                                                         \
 	(IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_WRITE | IB_ACCESS_REMOTE_READ)
 
+#define VALID_DMA_MR_FLAGS IB_ACCESS_LOCAL_WRITE
+
 static enum gdma_mr_access_flags
 mana_ib_verbs_to_gdma_access_flags(int access_flags)
 {
@@ -39,6 +41,8 @@ static int mana_ib_gd_create_mr(struct mana_ib_dev *dev, struct mana_ib_mr *mr,
 	req.mr_type = mr_params->mr_type;
 
 	switch (mr_params->mr_type) {
+	case GDMA_MR_TYPE_GPA:
+		break;
 	case GDMA_MR_TYPE_GVA:
 		req.gva.dma_region_handle = mr_params->gva.dma_region_handle;
 		req.gva.virtual_address = mr_params->gva.virtual_address;
@@ -168,6 +172,38 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
 	return ERR_PTR(err);
 }
 
+struct ib_mr *mana_ib_get_dma_mr(struct ib_pd *ibpd, int access_flags)
+{
+	struct mana_ib_pd *pd = container_of(ibpd, struct mana_ib_pd, ibpd);
+	struct gdma_create_mr_params mr_params = {};
+	struct ib_device *ibdev = ibpd->device;
+	struct mana_ib_dev *dev;
+	struct mana_ib_mr *mr;
+	int err;
+
+	dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
+
+	if (access_flags & ~VALID_DMA_MR_FLAGS)
+		return ERR_PTR(-EINVAL);
+
+	mr = kzalloc(sizeof(*mr), GFP_KERNEL);
+	if (!mr)
+		return ERR_PTR(-ENOMEM);
+
+	mr_params.pd_handle = pd->pd_handle;
+	mr_params.mr_type = GDMA_MR_TYPE_GPA;
+
+	err = mana_ib_gd_create_mr(dev, mr, &mr_params);
+	if (err)
+		goto err_free;
+
+	return &mr->ibmr;
+
+err_free:
+	kfree(mr);
+	return ERR_PTR(err);
+}
+
 int mana_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
 {
 	struct mana_ib_mr *mr = container_of(ibmr, struct mana_ib_mr, ibmr);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 8d796a30ddde..dc19b5cb33a6 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -788,6 +788,11 @@ struct gdma_destory_pd_resp {
 };/* HW DATA */
 
 enum gdma_mr_type {
+	/*
+	 * Guest Physical Address - MRs of this type allow access
+	 * to any DMA-mapped memory using bus-logical address
+	 */
+	GDMA_MR_TYPE_GPA = 1,
 	/* Guest Virtual Address - MRs of this type allow access
 	 * to memory mapped by PTEs associated with this MR using a virtual
 	 * address that is set up in the MST
-- 
2.43.0

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Jason Gunthorpe]

On Wed, Apr 17, 2024 at 07:20:59AM -0700, Konstantin Taranov wrote:
> From: Konstantin Taranov <kotaranov@microsoft.com>
> 
> Implement allocation of DMA-mapped memory regions.
> 
> Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> ---
>  drivers/infiniband/hw/mana/device.c |  1 +
>  drivers/infiniband/hw/mana/mr.c     | 36 +++++++++++++++++++++++++++++
>  include/net/mana/gdma.h             |  5 ++++
>  3 files changed, 42 insertions(+)

What is the point of doing this without supporting enough verbs to
allow a kernel ULP?

Jason

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Zhu Yanjun]

On 17.04.24 16:20, Konstantin Taranov wrote:
> From: Konstantin Taranov <kotaranov@microsoft.com>
> 
> Implement allocation of DMA-mapped memory regions.
> 
> Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> ---
>   drivers/infiniband/hw/mana/device.c |  1 +
>   drivers/infiniband/hw/mana/mr.c     | 36 +++++++++++++++++++++++++++++
>   include/net/mana/gdma.h             |  5 ++++
>   3 files changed, 42 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
> index 6fa902ee80a6..043cef09f1c2 100644
> --- a/drivers/infiniband/hw/mana/device.c
> +++ b/drivers/infiniband/hw/mana/device.c
> @@ -29,6 +29,7 @@ static const struct ib_device_ops mana_ib_dev_ops = {
>   	.destroy_rwq_ind_table = mana_ib_destroy_rwq_ind_table,
>   	.destroy_wq = mana_ib_destroy_wq,
>   	.disassociate_ucontext = mana_ib_disassociate_ucontext,
> +	.get_dma_mr = mana_ib_get_dma_mr,
>   	.get_port_immutable = mana_ib_get_port_immutable,
>   	.mmap = mana_ib_mmap,
>   	.modify_qp = mana_ib_modify_qp,
> diff --git a/drivers/infiniband/hw/mana/mr.c b/drivers/infiniband/hw/mana/mr.c
> index 4f13423ecdbd..7c9394926a18 100644
> --- a/drivers/infiniband/hw/mana/mr.c
> +++ b/drivers/infiniband/hw/mana/mr.c
> @@ -8,6 +8,8 @@
>   #define VALID_MR_FLAGS                                                         \
>   	(IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_WRITE | IB_ACCESS_REMOTE_READ)
>   
> +#define VALID_DMA_MR_FLAGS IB_ACCESS_LOCAL_WRITE
> +
>   static enum gdma_mr_access_flags
>   mana_ib_verbs_to_gdma_access_flags(int access_flags)
>   {
> @@ -39,6 +41,8 @@ static int mana_ib_gd_create_mr(struct mana_ib_dev *dev, struct mana_ib_mr *mr,
>   	req.mr_type = mr_params->mr_type;
>   
>   	switch (mr_params->mr_type) {
> +	case GDMA_MR_TYPE_GPA:
> +		break;
>   	case GDMA_MR_TYPE_GVA:
>   		req.gva.dma_region_handle = mr_params->gva.dma_region_handle;
>   		req.gva.virtual_address = mr_params->gva.virtual_address;
> @@ -168,6 +172,38 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd *ibpd, u64 start, u64 length,
>   	return ERR_PTR(err);
>   }
>   

Not sure if the following function needs comments or not.
If yes, the kernel doc 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/doc-guide/kernel-doc.rst?h=v6.9-rc4#n67 
can provide a good example.

Best Regards,
Zhu Yanjun

> +struct ib_mr *mana_ib_get_dma_mr(struct ib_pd *ibpd, int access_flags)
> +{
> +	struct mana_ib_pd *pd = container_of(ibpd, struct mana_ib_pd, ibpd);
> +	struct gdma_create_mr_params mr_params = {};
> +	struct ib_device *ibdev = ibpd->device;
> +	struct mana_ib_dev *dev;
> +	struct mana_ib_mr *mr;
> +	int err;
> +
> +	dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
> +
> +	if (access_flags & ~VALID_DMA_MR_FLAGS)
> +		return ERR_PTR(-EINVAL);
> +
> +	mr = kzalloc(sizeof(*mr), GFP_KERNEL);
> +	if (!mr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mr_params.pd_handle = pd->pd_handle;
> +	mr_params.mr_type = GDMA_MR_TYPE_GPA;
> +
> +	err = mana_ib_gd_create_mr(dev, mr, &mr_params);
> +	if (err)
> +		goto err_free;
> +
> +	return &mr->ibmr;
> +
> +err_free:
> +	kfree(mr);
> +	return ERR_PTR(err);
> +}
> +
>   int mana_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
>   {
>   	struct mana_ib_mr *mr = container_of(ibmr, struct mana_ib_mr, ibmr);
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index 8d796a30ddde..dc19b5cb33a6 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -788,6 +788,11 @@ struct gdma_destory_pd_resp {
>   };/* HW DATA */
>   
>   enum gdma_mr_type {
> +	/*
> +	 * Guest Physical Address - MRs of this type allow access
> +	 * to any DMA-mapped memory using bus-logical address
> +	 */
> +	GDMA_MR_TYPE_GPA = 1,
>   	/* Guest Virtual Address - MRs of this type allow access
>   	 * to memory mapped by PTEs associated with this MR using a virtual
>   	 * address that is set up in the MST

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Konstantin Taranov]

> From: Zhu Yanjun <zyjzyj2000@gmail.com>
> On 17.04.24 16:20, Konstantin Taranov wrote:
> > From: Konstantin Taranov <kotaranov@microsoft.com>
> >
> > Implement allocation of DMA-mapped memory regions.
> >
> > Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> > ---
> >   drivers/infiniband/hw/mana/device.c |  1 +
> >   drivers/infiniband/hw/mana/mr.c     | 36
> +++++++++++++++++++++++++++++
> >   include/net/mana/gdma.h             |  5 ++++
> >   3 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/mana/device.c
> > b/drivers/infiniband/hw/mana/device.c
> > index 6fa902ee80a6..043cef09f1c2 100644
> > --- a/drivers/infiniband/hw/mana/device.c
> > +++ b/drivers/infiniband/hw/mana/device.c
> > @@ -29,6 +29,7 @@ static const struct ib_device_ops mana_ib_dev_ops =
> {
> >     .destroy_rwq_ind_table = mana_ib_destroy_rwq_ind_table,
> >     .destroy_wq = mana_ib_destroy_wq,
> >     .disassociate_ucontext = mana_ib_disassociate_ucontext,
> > +   .get_dma_mr = mana_ib_get_dma_mr,
> >     .get_port_immutable = mana_ib_get_port_immutable,
> >     .mmap = mana_ib_mmap,
> >     .modify_qp = mana_ib_modify_qp,
> > diff --git a/drivers/infiniband/hw/mana/mr.c
> > b/drivers/infiniband/hw/mana/mr.c index 4f13423ecdbd..7c9394926a18
> > 100644
> > --- a/drivers/infiniband/hw/mana/mr.c
> > +++ b/drivers/infiniband/hw/mana/mr.c
> > @@ -8,6 +8,8 @@
> >   #define VALID_MR_FLAGS                                                         \
> >     (IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_WRITE |
> > IB_ACCESS_REMOTE_READ)
> >
> > +#define VALID_DMA_MR_FLAGS IB_ACCESS_LOCAL_WRITE
> > +
> >   static enum gdma_mr_access_flags
> >   mana_ib_verbs_to_gdma_access_flags(int access_flags)
> >   {
> > @@ -39,6 +41,8 @@ static int mana_ib_gd_create_mr(struct mana_ib_dev
> *dev, struct mana_ib_mr *mr,
> >     req.mr_type = mr_params->mr_type;
> >
> >     switch (mr_params->mr_type) {
> > +   case GDMA_MR_TYPE_GPA:
> > +           break;
> >     case GDMA_MR_TYPE_GVA:
> >             req.gva.dma_region_handle = mr_params-
> >gva.dma_region_handle;
> >             req.gva.virtual_address = mr_params->gva.virtual_address;
> @@
> > -168,6 +172,38 @@ struct ib_mr *mana_ib_reg_user_mr(struct ib_pd
> *ibpd, u64 start, u64 length,
> >     return ERR_PTR(err);
> >   }
> >
>
> Not sure if the following function needs comments or not.
> If yes, the kernel doc
> https://git.ke/
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2
> Ftree%2FDocumentation%2Fdoc-guide%2Fkernel-doc.rst%3Fh%3Dv6.9-
> rc4%23n67&data=05%7C02%7Ckotaranov%40microsoft.com%7C2816715935
> 85405f280e08dc5f925007%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C
> 0%7C638490329257001758%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C
> %7C&sdata=1Mzt0DKzty2jMJYm52gP%2FaloYnFGUTzN7gzAP05RdoQ%3D&res
> erved=0
> can provide a good example.
>

Thanks! I will have a look and see how I can improve comments.

> Best Regards,
> Zhu Yanjun
>
> > +struct ib_mr *mana_ib_get_dma_mr(struct ib_pd *ibpd, int
> > +access_flags) {
> > +   struct mana_ib_pd *pd = container_of(ibpd, struct mana_ib_pd,
> ibpd);
> > +   struct gdma_create_mr_params mr_params = {};
> > +   struct ib_device *ibdev = ibpd->device;
> > +   struct mana_ib_dev *dev;
> > +   struct mana_ib_mr *mr;
> > +   int err;
> > +
> > +   dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
> > +
> > +   if (access_flags & ~VALID_DMA_MR_FLAGS)
> > +           return ERR_PTR(-EINVAL);
> > +
> > +   mr = kzalloc(sizeof(*mr), GFP_KERNEL);
> > +   if (!mr)
> > +           return ERR_PTR(-ENOMEM);
> > +
> > +   mr_params.pd_handle = pd->pd_handle;
> > +   mr_params.mr_type = GDMA_MR_TYPE_GPA;
> > +
> > +   err = mana_ib_gd_create_mr(dev, mr, &mr_params);
> > +   if (err)
> > +           goto err_free;
> > +
> > +   return &mr->ibmr;
> > +
> > +err_free:
> > +   kfree(mr);
> > +   return ERR_PTR(err);
> > +}
> > +
> >   int mana_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> >   {
> >     struct mana_ib_mr *mr = container_of(ibmr, struct mana_ib_mr,
> > ibmr); diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> > index 8d796a30ddde..dc19b5cb33a6 100644
> > --- a/include/net/mana/gdma.h
> > +++ b/include/net/mana/gdma.h
> > @@ -788,6 +788,11 @@ struct gdma_destory_pd_resp {
> >   };/* HW DATA */
> >
> >   enum gdma_mr_type {
> > +   /*
> > +    * Guest Physical Address - MRs of this type allow access
> > +    * to any DMA-mapped memory using bus-logical address
> > +    */
> > +   GDMA_MR_TYPE_GPA = 1,
> >     /* Guest Virtual Address - MRs of this type allow access
> >      * to memory mapped by PTEs associated with this MR using a virtual
> >      * address that is set up in the MST

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Konstantin Taranov]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> On Wed, Apr 17, 2024 at 07:20:59AM -0700, Konstantin Taranov wrote:
> > From: Konstantin Taranov <kotaranov@microsoft.com>
> >
> > Implement allocation of DMA-mapped memory regions.
> >
> > Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> > ---
> >  drivers/infiniband/hw/mana/device.c |  1 +
> >  drivers/infiniband/hw/mana/mr.c     | 36
> +++++++++++++++++++++++++++++
> >  include/net/mana/gdma.h             |  5 ++++
> >  3 files changed, 42 insertions(+)
> 
> What is the point of doing this without supporting enough verbs to allow a
> kernel ULP?
> 

True, the proposed code is useless at this state.
Nevertheless, mana_ib team aims to send kernel ULP patches after we are done
with uverbs pathes (i.e., udata is not null). As this change does not conflict with the
current effort, I decided to send this patch now. I can extend the series to make
it more useful.

Jason, could  you suggest a minimal list of ib_device_ops methods, that includes
get_dma_mr, which can be approved?

Thanks,
Konstantin

> Jason

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Jason Gunthorpe]

On Fri, Apr 19, 2024 at 09:14:14AM +0000, Konstantin Taranov wrote:
> > From: Jason Gunthorpe <jgg@ziepe.ca>
> > On Wed, Apr 17, 2024 at 07:20:59AM -0700, Konstantin Taranov wrote:
> > > From: Konstantin Taranov <kotaranov@microsoft.com>
> > >
> > > Implement allocation of DMA-mapped memory regions.
> > >
> > > Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> > > ---
> > >  drivers/infiniband/hw/mana/device.c |  1 +
> > >  drivers/infiniband/hw/mana/mr.c     | 36
> > +++++++++++++++++++++++++++++
> > >  include/net/mana/gdma.h             |  5 ++++
> > >  3 files changed, 42 insertions(+)
> > 
> > What is the point of doing this without supporting enough verbs to allow a
> > kernel ULP?
> > 
> 
> True, the proposed code is useless at this state.
> Nevertheless, mana_ib team aims to send kernel ULP patches after we are done
> with uverbs pathes (i.e., udata is not null). As this change does not conflict with the
> current effort, I decided to send this patch now. I can extend the series to make
> it more useful.
> 
> Jason, could  you suggest a minimal list of ib_device_ops methods, that includes
> get_dma_mr, which can be approved?

Is there any chance you can send a single series to support a
ULP. NVMe or something like?

Jason

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Konstantin Taranov]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> On Fri, Apr 19, 2024 at 09:14:14AM +0000, Konstantin Taranov wrote:
> > > From: Jason Gunthorpe <jgg@ziepe.ca> On Wed, Apr 17, 2024 at
> > > 07:20:59AM -0700, Konstantin Taranov wrote:
> > > > From: Konstantin Taranov <kotaranov@microsoft.com>
> > > >
> > > > Implement allocation of DMA-mapped memory regions.
> > > >
> > > > Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> > > > ---
> > > >  drivers/infiniband/hw/mana/device.c |  1 +
> > > >  drivers/infiniband/hw/mana/mr.c     | 36
> > > +++++++++++++++++++++++++++++
> > > >  include/net/mana/gdma.h             |  5 ++++
> > > >  3 files changed, 42 insertions(+)
> > >
> > > What is the point of doing this without supporting enough verbs to
> > > allow a kernel ULP?
> > >
> >
> > True, the proposed code is useless at this state.
> > Nevertheless, mana_ib team aims to send kernel ULP patches after we
> > are done with uverbs pathes (i.e., udata is not null). As this change
> > does not conflict with the current effort, I decided to send this
> > patch now. I can extend the series to make it more useful.
> >
> > Jason, could  you suggest a minimal list of ib_device_ops methods,
> > that includes get_dma_mr, which can be approved?
> 
> Is there any chance you can send a single series to support a ULP. NVMe or
> something like?

Sure, I can. I will investigate the way to make get_dma_mr used with fewer changes. 

Generally, I am wondering what would be easier for reviewers.
Should I try to send short patch series enabling one feature, or should I actually try
to produce long patch series that enable a use-case consisting of several features?

Konstantin

Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Jason Gunthorpe]

On Mon, Apr 22, 2024 at 09:12:46AM +0000, Konstantin Taranov wrote:
> > From: Jason Gunthorpe <jgg@ziepe.ca>
> > On Fri, Apr 19, 2024 at 09:14:14AM +0000, Konstantin Taranov wrote:
> > > > From: Jason Gunthorpe <jgg@ziepe.ca> On Wed, Apr 17, 2024 at
> > > > 07:20:59AM -0700, Konstantin Taranov wrote:
> > > > > From: Konstantin Taranov <kotaranov@microsoft.com>
> > > > >
> > > > > Implement allocation of DMA-mapped memory regions.
> > > > >
> > > > > Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> > > > > ---
> > > > >  drivers/infiniband/hw/mana/device.c |  1 +
> > > > >  drivers/infiniband/hw/mana/mr.c     | 36
> > > > +++++++++++++++++++++++++++++
> > > > >  include/net/mana/gdma.h             |  5 ++++
> > > > >  3 files changed, 42 insertions(+)
> > > >
> > > > What is the point of doing this without supporting enough verbs to
> > > > allow a kernel ULP?
> > > >
> > >
> > > True, the proposed code is useless at this state.
> > > Nevertheless, mana_ib team aims to send kernel ULP patches after we
> > > are done with uverbs pathes (i.e., udata is not null). As this change
> > > does not conflict with the current effort, I decided to send this
> > > patch now. I can extend the series to make it more useful.
> > >
> > > Jason, could  you suggest a minimal list of ib_device_ops methods,
> > > that includes get_dma_mr, which can be approved?
> > 
> > Is there any chance you can send a single series to support a ULP. NVMe or
> > something like?
> 
> Sure, I can. I will investigate the way to make get_dma_mr used with fewer changes. 
> 
> Generally, I am wondering what would be easier for reviewers.
> Should I try to send short patch series enabling one feature, or should I actually try
> to produce long patch series that enable a use-case consisting of several features?

Generally the guideline is to avoid adding dead code, some exceptions
may be possible, but that should be the gold standard to try for,
IMHO.

If you want to support kernel ULPs then say witch kernel ULP you want
and send a series to make it work.

If that series is way too big then split it into two halfs and post
both at the start.

Jason