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. Michael
