Re: [PATCH for-next] RDMA/efa: Add EFA query MR support

[Leon Romanovsky <leon@kernel.org>]

On Wed, Dec 06, 2023 at 08:30:37PM +0200, Margolin, Michael wrote:
> Hey Gal,
> 
> Thanks for your comments.
> 
> On 12/6/2023 4:52 PM, Gal Pressman wrote:
> > Hi Michael,
> >
> > On 06/12/2023 0:16, Michael Margolin wrote:
> >> @@ -432,6 +435,9 @@ static int efa_ib_device_add(struct efa_dev *dev)
> >>
> >>       ib_set_device_ops(&dev->ibdev, &efa_dev_ops);
> >>
> >> +     if (IS_ENABLED(CONFIG_INFINIBAND_USER_ACCESS))
> > EFA depends on CONFIG_INFINIBAND_USER_ACCESS:
> >
> >       7 config INFINIBAND_EFA
> >       8         tristate "Amazon Elastic Fabric Adapter (EFA) support"
> >       9         depends on PCI_MSI && 64BIT && !CPU_BIG_ENDIAN
> >      10         depends on INFINIBAND_USER_ACCESS
> 
> I'll remove this if statement.
> 
> >> +             dev->ibdev.driver_def = efa_uapi_defs;
> >> +
> >>       err = ib_register_device(&dev->ibdev, "efa_%d", &pdev->dev);
> >>       if (err)
> >>               goto err_destroy_eqs;
> >> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> >> index 0f8ca99d0827..d81904f4b876 100644
> >> --- a/drivers/infiniband/hw/efa/efa_verbs.c
> >> +++ b/drivers/infiniband/hw/efa/efa_verbs.c
> >> @@ -13,6 +13,9 @@
> >>  #include <rdma/ib_user_verbs.h>
> >>  #include <rdma/ib_verbs.h>
> >>  #include <rdma/uverbs_ioctl.h>
> >> +#define UVERBS_MODULE_NAME efa_ib
> >> +#include <rdma/uverbs_named_ioctl.h>
> >> +#include <rdma/ib_user_ioctl_cmds.h>
> >>
> >>  #include "efa.h"
> >>  #include "efa_io_defs.h"
> >> @@ -1653,6 +1656,9 @@ static int efa_register_mr(struct ib_pd *ibpd, struct efa_mr *mr, u64 start,
> >>       mr->ibmr.lkey = result.l_key;
> >>       mr->ibmr.rkey = result.r_key;
> >>       mr->ibmr.length = length;
> >> +     mr->recv_pci_bus_id = result.recv_pci_bus_id;
> >> +     mr->rdma_read_pci_bus_id = result.rdma_read_pci_bus_id;
> >> +     mr->rdma_recv_pci_bus_id = result.rdma_recv_pci_bus_id;
> > Why is a query_mr ioctl better than returning this data through udata on
> > MR creation?
> 
> We need this for both reg_user_mr and reg_user_mr_dmabuf and it doesn't
> make sense to implement it twice. In addition, those two verbs are using
> different mechanisms (write and ioctl) and extending dmabuf reg_mr will
> require more extensive changes on rdma-core side.

Both of these callbacks use ioctl() interface. We keep write() interface
for legacy systems which won't and/or shouldn't use any new API.

Thanks

> 
> Michael
>