RDMA/vmw_pvrdma: Add RoCEv2 support

Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len. Simplified some of the RoCEv2 versioning code and added 2 more device cap flags.

Acked-by Bryan Tan <[email protected]>

Acked-by Aditya Sarwade <[email protected]>

Signed-off-by Adit Ranadive <[email protected]>

---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h         |  1 +
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 19 ++++++++++---------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  7 +++----
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c   |  9 +++++++++
 4 files changed, 23 insertions(+), 13 deletions(-)

drivers/infiniband/hw/vmw_pvrdma/pvrdma.h +1 -0

index 513a182..ea10155 100644
@@ -194,6 +194,7 @@ struct pvrdma_dev {
 	void *resp_slot;
 	unsigned long flags;
 	struct list_head device_link;
+	unsigned int dsr_version;
 
 	/* Locking and interrupt information. */
 	spinlock_t cmd_lock; /* Command lock. */

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +9 -1

index 71df5d1..d947557 100644
@@ -50,7 +50,15 @@
 
 #include "pvrdma_verbs.h"
 
-#define PVRDMA_VERSION			18
+/*
+ * PVRDMA version macros. Some new features require updates to PVRDMA_VERSION.
+ * These macros allow us to check for different features if necessary.
+ */
+
+#define PVRDMA_ROCEV1_VERSION		17
+#define PVRDMA_ROCEV2_VERSION		18
+#define PVRDMA_VERSION			PVRDMA_ROCEV2_VERSION
+
 #define PVRDMA_BOARD_ID			1
 #define PVRDMA_REV_ID			1
 

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +1 -1

 enum pvrdma_pci_resource {
 	PVRDMA_PCI_RESOURCE_MSIX,	/* BAR0: MSI-X, MMIO. */
 	PVRDMA_PCI_RESOURCE_REG,	/* BAR1: Registers, MMIO. */

@@ -232,7 +233,7 @@ struct pvrdma_device_caps {
 	u8  atomic_ops;				/* PVRDMA_ATOMIC_OP_* bits */
 	u8  bmme_flags;				/* FRWR Mem Mgmt Extensions */
 	u8  gid_types;				/* PVRDMA_GID_TYPE_FLAG_ */
-	u8  reserved[4];
+	u32 max_fast_reg_page_list_len;
 };
 
 struct pvrdma_ring_page_info {

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +2 -3

index 6fd5828..ae536a4 100644
@@ -729,7 +729,6 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 	int ret;
 	unsigned long start;
 	unsigned long len;
-	unsigned int version;
 	dma_addr_t slot_dma = 0;
 
 	dev_dbg(&pdev->dev, "initializing driver %s\n", pci_name(pdev));

@@ -826,9 +825,9 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 		goto err_unmap_regs;
 	}
 
-	version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
+	dev->dsr_version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
 	dev_info(&pdev->dev, "device version %d, driver version %d\n",
-		 version, PVRDMA_VERSION);
+		 dev->dsr_version, PVRDMA_VERSION);
 
 	dev->dsr = dma_alloc_coherent(&pdev->dev, sizeof(*dev->dsr),
 				      &dev->dsrbase, GFP_KERNEL);

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +0 -0

 	    !((dev->dsr->caps.gid_types & PVRDMA_GID_TYPE_FLAG_ROCE_V1) ||
 	      (dev->dsr->caps.gid_types & PVRDMA_GID_TYPE_FLAG_ROCE_V2))) {
 		dev_err(&pdev->dev, "driver needs RoCE v1 or v2 support\n");

drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c +9 -1

index 2851704..0b19d1a 100644
@@ -83,6 +83,8 @@ int pvrdma_query_device(struct ib_device *ibdev,
 	props->max_qp_wr = dev->dsr->caps.max_qp_wr;
 	props->device_cap_flags = dev->dsr->caps.device_cap_flags;
 	props->max_sge = dev->dsr->caps.max_sge;
+	props->max_sge_rd = (dev->dsr_version >= PVRDMA_ROCEV2_VERSION) ?
+			     dev->dsr->caps.max_sge_rd : dev->dsr->caps.max_sge;
 	props->max_cq = dev->dsr->caps.max_cq;
 	props->max_cqe = dev->dsr->caps.max_cqe;
 	props->max_mr = dev->dsr->caps.max_mr;

@@ -100,9 +102,16 @@ int pvrdma_query_device(struct ib_device *ibdev,
 	if ((dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_LOCAL_INV) &&
 	    (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_REMOTE_INV) &&
 	    (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_FAST_REG_WR)) {
+		props->max_fast_reg_page_list_len =
+		(dev->dsr_version >= PVRDMA_ROCEV2_VERSION) ?
+		 dev->dsr->caps.max_fast_reg_page_list_len :
+		 PVRDMA_MAX_FAST_REG_PAGES;
 		props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS;
 	}
 
+	props->device_cap_flags |= IB_DEVICE_PORT_ACTIVE_EVENT |
+				   IB_DEVICE_RC_RNR_NAK_GEN;
+
 	return 0;
 }
 
--

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 51 lines · Adit Ranadive

Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len. Simplified some of the RoCEv2 versioning code and added 2 more device cap flags.

Acked-by Bryan Tan <[email protected]>

Acked-by Aditya Sarwade <[email protected]>

Signed-off-by Adit Ranadive <[email protected]>

---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h         |  1 +
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 19 ++++++++++---------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  7 +++----
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c   |  9 +++++++++
 4 files changed, 23 insertions(+), 13 deletions(-)

drivers/infiniband/hw/vmw_pvrdma/pvrdma.h +1 -0

index 513a182..ea10155 100644
@@ -194,6 +194,7 @@ struct pvrdma_dev {
 	void *resp_slot;
 	unsigned long flags;
 	struct list_head device_link;
+	unsigned int dsr_version;
 
 	/* Locking and interrupt information. */
 	spinlock_t cmd_lock; /* Command lock. */

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +9 -1

index 71df5d1..d947557 100644
@@ -50,7 +50,15 @@
 
 #include "pvrdma_verbs.h"
 
-#define PVRDMA_VERSION			18
+/*
+ * PVRDMA version macros. Some new features require updates to PVRDMA_VERSION.
+ * These macros allow us to check for different features if necessary.
+ */
+
+#define PVRDMA_ROCEV1_VERSION		17
+#define PVRDMA_ROCEV2_VERSION		18
+#define PVRDMA_VERSION			PVRDMA_ROCEV2_VERSION
+
 #define PVRDMA_BOARD_ID			1
 #define PVRDMA_REV_ID			1
 

@@ -123,13 +131,6 @@
 #define PVRDMA_GID_TYPE_FLAG_ROCE_V1	BIT(0)
 #define PVRDMA_GID_TYPE_FLAG_ROCE_V2	BIT(1)
 

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h line 132

Quoted 7 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +0 -7

-/*
- * PVRDMA version macros. Some new features require updates to PVRDMA_VERSION.
- * These macros allow us to check for different features if necessary.
- */
-
-#define PVRDMA_VERSION_ROCEV2_SUPPORT	18
-

You added it in previous patch of the same series. Please don't send patches with code which is going to be removed/rewritten immediately.

Quoted 40 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +1 -1

 enum pvrdma_pci_resource {
 	PVRDMA_PCI_RESOURCE_MSIX,	/* BAR0: MSI-X, MMIO. */
 	PVRDMA_PCI_RESOURCE_REG,	/* BAR1: Registers, MMIO. */

@@ -232,7 +233,7 @@ struct pvrdma_device_caps {
 	u8  atomic_ops;				/* PVRDMA_ATOMIC_OP_* bits */
 	u8  bmme_flags;				/* FRWR Mem Mgmt Extensions */
 	u8  gid_types;				/* PVRDMA_GID_TYPE_FLAG_ */
-	u8  reserved[4];
+	u32 max_fast_reg_page_list_len;
 };
 
 struct pvrdma_ring_page_info {

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +2 -3

index 6fd5828..ae536a4 100644
@@ -729,7 +729,6 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 	int ret;
 	unsigned long start;
 	unsigned long len;
-	unsigned int version;
 	dma_addr_t slot_dma = 0;
 
 	dev_dbg(&pdev->dev, "initializing driver %s\n", pci_name(pdev));

@@ -826,9 +825,9 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 		goto err_unmap_regs;
 	}
 
-	version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
+	dev->dsr_version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
 	dev_info(&pdev->dev, "device version %d, driver version %d\n",
-		 version, PVRDMA_VERSION);
+		 dev->dsr_version, PVRDMA_VERSION);
 
 	dev->dsr = dma_alloc_coherent(&pdev->dev, sizeof(*dev->dsr),
 				      &dev->dsrbase, GFP_KERNEL);

@@ -908,7 +907,7 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 	}
 

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c static int pvrdma_pci_probe(struct pci_dev *pdev, · line 910

Quoted 3 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +1 -1

 	/* PVRDMA supports RoCE V1 or V2. */
-	if (version >= PVRDMA_VERSION_ROCEV2_SUPPORT &&
+	if (dev->dsr_version >= PVRDMA_ROCEV2_VERSION &&

Your whole idea of version is shaky. You need to move to capability bits per feature model and not rely on global version. In the near future, your code will be full of checks like this.

There is a reason why you don't see many device driver here relies on FW version, which is similar to your versioning scheme.

Quoted 52 lines · Leon Romanovsky

Quoted 51 lines · Adit Ranadive

Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len. Simplified some of the RoCEv2 versioning code and added 2 more device cap flags.

Acked-by Bryan Tan <[email protected]>

Acked-by Aditya Sarwade <[email protected]>

Signed-off-by Adit Ranadive <[email protected]>

---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h         |  1 +
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 19 ++++++++++---------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  7 +++----
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c   |  9 +++++++++
 4 files changed, 23 insertions(+), 13 deletions(-)

drivers/infiniband/hw/vmw_pvrdma/pvrdma.h +1 -0

index 513a182..ea10155 100644
@@ -194,6 +194,7 @@ struct pvrdma_dev {
 	void *resp_slot;
 	unsigned long flags;
 	struct list_head device_link;
+	unsigned int dsr_version;
 
 	/* Locking and interrupt information. */
 	spinlock_t cmd_lock; /* Command lock. */

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +9 -1

index 71df5d1..d947557 100644
@@ -50,7 +50,15 @@
 
 #include "pvrdma_verbs.h"
 
-#define PVRDMA_VERSION			18
+/*
+ * PVRDMA version macros. Some new features require updates to PVRDMA_VERSION.
+ * These macros allow us to check for different features if necessary.
+ */
+
+#define PVRDMA_ROCEV1_VERSION		17
+#define PVRDMA_ROCEV2_VERSION		18
+#define PVRDMA_VERSION			PVRDMA_ROCEV2_VERSION
+
 #define PVRDMA_BOARD_ID			1
 #define PVRDMA_REV_ID			1
 

@@ -123,13 +131,6 @@
 #define PVRDMA_GID_TYPE_FLAG_ROCE_V1	BIT(0)
 #define PVRDMA_GID_TYPE_FLAG_ROCE_V2	BIT(1)
 

Quoted 10 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h line 132

Quoted 7 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +0 -7

-/*
- * PVRDMA version macros. Some new features require updates to PVRDMA_VERSION.
- * These macros allow us to check for different features if necessary.
- */
-
-#define PVRDMA_VERSION_ROCEV2_SUPPORT	18
-

You added it in previous patch of the same series. Please don't send patches with code which is going to be removed/rewritten immediately.

The previous patch was posted a while back internally. I had to do some cleanup (second patch) before sending it out. I'll send out a v1 with this merged into the first patch.

Quoted 40 lines

Quoted 40 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +1 -1

 enum pvrdma_pci_resource {
 	PVRDMA_PCI_RESOURCE_MSIX,	/* BAR0: MSI-X, MMIO. */
 	PVRDMA_PCI_RESOURCE_REG,	/* BAR1: Registers, MMIO. */

@@ -232,7 +233,7 @@ struct pvrdma_device_caps {
 	u8  atomic_ops;				/* PVRDMA_ATOMIC_OP_* bits */
 	u8  bmme_flags;				/* FRWR Mem Mgmt Extensions */
 	u8  gid_types;				/* PVRDMA_GID_TYPE_FLAG_ */
-	u8  reserved[4];
+	u32 max_fast_reg_page_list_len;
 };
 
 struct pvrdma_ring_page_info {

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +2 -3

index 6fd5828..ae536a4 100644
@@ -729,7 +729,6 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 	int ret;
 	unsigned long start;
 	unsigned long len;
-	unsigned int version;
 	dma_addr_t slot_dma = 0;
 
 	dev_dbg(&pdev->dev, "initializing driver %s\n", pci_name(pdev));

@@ -826,9 +825,9 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 		goto err_unmap_regs;
 	}
 
-	version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
+	dev->dsr_version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
 	dev_info(&pdev->dev, "device version %d, driver version %d\n",
-		 version, PVRDMA_VERSION);
+		 dev->dsr_version, PVRDMA_VERSION);
 
 	dev->dsr = dma_alloc_coherent(&pdev->dev, sizeof(*dev->dsr),
 				      &dev->dsrbase, GFP_KERNEL);

@@ -908,7 +907,7 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 	}
 

Quoted 7 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c static int pvrdma_pci_probe(struct pci_dev *pdev, · line 910

Quoted 3 lines

drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +1 -1

 	/* PVRDMA supports RoCE V1 or V2. */
-	if (version >= PVRDMA_VERSION_ROCEV2_SUPPORT &&
+	if (dev->dsr_version >= PVRDMA_ROCEV2_VERSION &&

Your whole idea of version is shaky. You need to move to capability bits per feature model and not rely on global version. In the near future, your code will be full of checks like this.

Why does the type of check matter? We would have to do a check anyway if there were capability bits exposed. In our case the global version is essentially the feature model for the device given that it isn't as feature rich as some of the other RDMA providers so doing a blanket check on the version seems the simplest way to go. I think the check here for the version is redundant anyways and can remove that in v1.

Quoted 3 lines

There is a reason why you don't see many device driver here relies on FW version, which is similar to your versioning scheme.

I agree but in our case we can't update the FW version without exposing any new capabilities (due to compatibility reasons) so is essentially tied to the feature caps. As a compromise I can add a macro that checks the caps so the various version checks are a bit less obnoxious.

Thanks,
Adit
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Set the IP_BASED_GIDS port cap flag for vmw_pvrdma.

Acked-by Aditya Sarwade <[email protected]>

Signed-off-by Adit Ranadive <[email protected]>

---

Quoted 5 lines · Adit Ranadive

Set the IP_BASED_GIDS port cap flag for vmw_pvrdma.

Acked-by Aditya Sarwade <[email protected]>

Signed-off-by Adit Ranadive <[email protected]>

---

Adit,

Please reread the Documentation/process/5.Posting.rst and the Documentation/process/submitting-patches.rst for the meaning of the tags ("12) When to use Acked-by: and Cc:").

I think that you wanted to write Reviewed-by instead.