[PATCH for-next 0/3] RDMA/vmw_pvrdma: Add RoCEv2 support

[PATCH for-next 0/3] RDMA/vmw_pvrdma: Add RoCEv2 support

[Adit Ranadive]

Hi Doug,

These are a set of patches to add RoCEv2 support for the VMware PVRDMA
driver as well as exposing some device caps. Please add these to your
4.14 queue.

Thanks,
Adit
---
Adit Ranadive (2):
  RDMA/vmw_pvrdma: Update device query parameters
  RDMA/vmw_pvrdma: Add new port_cap flag

Bryan Tan (1):
  RDMA/vmw_pvrdma: Add RoCEv2 support

 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h         |  3 +++
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 12 +++++++--
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c    | 30 +++++++++++++----------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_misc.c    | 14 +++++++++++
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c   | 10 ++++++++
 5 files changed, 54 insertions(+), 15 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 1/3] RDMA/vmw_pvrdma: Add RoCEv2 support

[Adit Ranadive]

From: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>

The driver version is bumped for compatibility purposes. Also, send correct
GID type during register to device.

Acked-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma.h         |  2 ++
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h |  9 +++++++-
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c    | 25 ++++++++++++++---------
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_misc.c    | 14 +++++++++++++
 4 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
index 8e2f0a1..513a182 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
@@ -444,6 +444,8 @@ void pvrdma_ah_attr_to_rdma(struct rdma_ah_attr *dst,
 			    const struct pvrdma_ah_attr *src);
 void rdma_ah_attr_to_pvrdma(struct pvrdma_ah_attr *dst,
 			    const struct rdma_ah_attr *src);
+enum ib_gid_type pvrdma_gid_type_to_ib(u8 gid_type);
+u8 ib_gid_type_to_pvrdma(enum ib_gid_type gid_type);
 
 int pvrdma_uar_table_init(struct pvrdma_dev *dev);
 void pvrdma_uar_table_cleanup(struct pvrdma_dev *dev);
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
index 09078cc..71df5d1 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
@@ -50,7 +50,7 @@
 
 #include "pvrdma_verbs.h"
 
-#define PVRDMA_VERSION			17
+#define PVRDMA_VERSION			18
 #define PVRDMA_BOARD_ID			1
 #define PVRDMA_REV_ID			1
 
@@ -123,6 +123,13 @@
 #define PVRDMA_GID_TYPE_FLAG_ROCE_V1	BIT(0)
 #define PVRDMA_GID_TYPE_FLAG_ROCE_V2	BIT(1)
 
+/*
+ * 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
+
 enum pvrdma_pci_resource {
 	PVRDMA_PCI_RESOURCE_MSIX,	/* BAR0: MSI-X, MMIO. */
 	PVRDMA_PCI_RESOURCE_REG,	/* BAR1: Registers, MMIO. */
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index 7f29e4d..6fd5828 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -128,10 +128,14 @@ static int pvrdma_init_device(struct pvrdma_dev *dev)
 static int pvrdma_port_immutable(struct ib_device *ibdev, u8 port_num,
 				 struct ib_port_immutable *immutable)
 {
+	struct pvrdma_dev *dev = to_vdev(ibdev);
 	struct ib_port_attr attr;
 	int err;
 
-	immutable->core_cap_flags = RDMA_CORE_PORT_IBA_ROCE;
+	if (dev->dsr->caps.gid_types == PVRDMA_GID_TYPE_FLAG_ROCE_V1)
+		immutable->core_cap_flags |= RDMA_CORE_PORT_IBA_ROCE;
+	else if (dev->dsr->caps.gid_types == PVRDMA_GID_TYPE_FLAG_ROCE_V2)
+		immutable->core_cap_flags |= RDMA_CORE_PORT_IBA_ROCE_UDP_ENCAP;
 
 	err = ib_query_port(ibdev, port_num, &attr);
 	if (err)
@@ -569,6 +573,7 @@ static void pvrdma_free_slots(struct pvrdma_dev *dev)
 
 static int pvrdma_add_gid_at_index(struct pvrdma_dev *dev,
 				   const union ib_gid *gid,
+				   u8 gid_type,
 				   int index)
 {
 	int ret;
@@ -586,7 +591,7 @@ static int pvrdma_add_gid_at_index(struct pvrdma_dev *dev,
 	cmd_bind->mtu = ib_mtu_enum_to_int(IB_MTU_1024);
 	cmd_bind->vlan = 0xfff;
 	cmd_bind->index = index;
-	cmd_bind->gid_type = PVRDMA_GID_TYPE_FLAG_ROCE_V1;
+	cmd_bind->gid_type = gid_type;
 
 	ret = pvrdma_cmd_post(dev, &req, NULL, 0);
 	if (ret < 0) {
@@ -607,7 +612,9 @@ static int pvrdma_add_gid(struct ib_device *ibdev,
 {
 	struct pvrdma_dev *dev = to_vdev(ibdev);
 
-	return pvrdma_add_gid_at_index(dev, gid, index);
+	return pvrdma_add_gid_at_index(dev, gid,
+				       ib_gid_type_to_pvrdma(attr->gid_type),
+				       index);
 }
 
 static int pvrdma_del_gid_at_index(struct pvrdma_dev *dev, int index)
@@ -822,10 +829,6 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 	version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION);
 	dev_info(&pdev->dev, "device version %d, driver version %d\n",
 		 version, PVRDMA_VERSION);
-	if (version < PVRDMA_VERSION) {
-		dev_err(&pdev->dev, "incompatible device version\n");
-		goto err_uar_unmap;
-	}
 
 	dev->dsr = dma_alloc_coherent(&pdev->dev, sizeof(*dev->dsr),
 				      &dev->dsrbase, GFP_KERNEL);
@@ -904,9 +907,11 @@ static int pvrdma_pci_probe(struct pci_dev *pdev,
 		goto err_free_cq_ring;
 	}
 
-	/* Currently, the driver only supports RoCE V1. */
-	if (!(dev->dsr->caps.gid_types & PVRDMA_GID_TYPE_FLAG_ROCE_V1)) {
-		dev_err(&pdev->dev, "driver needs RoCE v1 support\n");
+	/* PVRDMA supports RoCE V1 or V2. */
+	if (version >= PVRDMA_VERSION_ROCEV2_SUPPORT &&
+	    !((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");
 		ret = -EFAULT;
 		goto err_free_cq_ring;
 	}
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_misc.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_misc.c
index ec6a4ca..0861e28 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_misc.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_misc.c
@@ -303,3 +303,17 @@ void rdma_ah_attr_to_pvrdma(struct pvrdma_ah_attr *dst,
 	dst->port_num = rdma_ah_get_port_num(src);
 	memcpy(&dst->dmac, src->roce.dmac, sizeof(dst->dmac));
 }
+
+enum ib_gid_type pvrdma_gid_type_to_ib(u8 gid_type)
+{
+	return (gid_type == PVRDMA_GID_TYPE_FLAG_ROCE_V2) ?
+		IB_GID_TYPE_ROCE_UDP_ENCAP :
+		IB_GID_TYPE_ROCE;
+}
+
+u8 ib_gid_type_to_pvrdma(enum ib_gid_type gid_type)
+{
+	return (gid_type == IB_GID_TYPE_ROCE_UDP_ENCAP) ?
+		PVRDMA_GID_TYPE_FLAG_ROCE_V2 :
+		PVRDMA_GID_TYPE_FLAG_ROCE_V1;
+}
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 2/3] RDMA/vmw_pvrdma: Update device query parameters

[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 <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 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(-)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
index 513a182..ea10155 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
@@ -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. */
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
index 71df5d1..d947557 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
@@ -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)
 
-/*
- * 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
-
 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 {
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index 6fd5828..ae536a4 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -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,
 	}
 
 	/* PVRDMA supports RoCE V1 or V2. */
-	if (version >= PVRDMA_VERSION_ROCEV2_SUPPORT &&
+	if (dev->dsr_version >= PVRDMA_ROCEV2_VERSION &&
 	    !((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");
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
index 2851704..0b19d1a 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 2/3] RDMA/vmw_pvrdma: Update device query parameters

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 6293 bytes --]

On Fri, Aug 18, 2017 at 09:44:48AM -0700, Adit Ranadive wrote:
> 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 <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> ---
>  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(-)
>
> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> index 513a182..ea10155 100644
> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> @@ -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. */
> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> index 71df5d1..d947557 100644
> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> @@ -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)
>
> -/*
> - * 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.


>  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 {
> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> index 6fd5828..ae536a4 100644
> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> @@ -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,
>  	}
>
>  	/* 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.

Thanks

>  	    !((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");
> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
> index 2851704..0b19d1a 100644
> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
> @@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 2/3] RDMA/vmw_pvrdma: Update device query parameters

[Adit Ranadive]

On Sat Aug 19 2017 23:11:22 GMT-0700 (PDT), Leon Romanovsky wrote:
> On Fri, Aug 18, 2017 at 09:44:48AM -0700, Adit Ranadive wrote:
> > 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 <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > ---
> >  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(-)
> >
> > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > index 513a182..ea10155 100644
> > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > @@ -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. */
> > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> > index 71df5d1..d947557 100644
> > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> > @@ -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)
> >
> > -/*
> > - * 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.

>  
> >  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 {
> > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> > index 6fd5828..ae536a4 100644
> > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> > @@ -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,
> >  	}
> >
> >  	/* 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.

>  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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 2/3] RDMA/vmw_pvrdma: Update device query parameters

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 6368 bytes --]

On Sun, Aug 20, 2017 at 11:56:26PM -0700, Adit Ranadive wrote:
> On Sat Aug 19 2017 23:11:22 GMT-0700 (PDT), Leon Romanovsky wrote:
> > On Fri, Aug 18, 2017 at 09:44:48AM -0700, Adit Ranadive wrote:
> > > 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 <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  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(-)
> > >
> > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > > index 513a182..ea10155 100644
> > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> > > @@ -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. */
> > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> > > index 71df5d1..d947557 100644
> > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
> > > @@ -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)
> > >
> > > -/*
> > > - * 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.
>
> >
> > >  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 {
> > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> > > index 6fd5828..ae536a4 100644
> > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
> > > @@ -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,
> > >  	}
> > >
> > >  	/* 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.

It is always good thing to remove unneeded checks.

>
> >  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,
It will allow you to put all your checks of versions in one place and
implement global_version-to-capability_bits translation table.

>
> Thanks,
> Adit
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH for-next 3/3] RDMA/vmw_pvrdma: Add new port_cap flag

[Adit Ranadive]

Set the IP_BASED_GIDS port cap flag for vmw_pvrdma.

Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
index 0b19d1a..1a367b7 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c
@@ -152,6 +152,7 @@ int pvrdma_query_port(struct ib_device *ibdev, u8 port,
 	props->gid_tbl_len = resp->attrs.gid_tbl_len;
 	props->port_cap_flags =
 		pvrdma_port_cap_flags_to_ib(resp->attrs.port_cap_flags);
+	props->port_cap_flags |= IB_PORT_CM_SUP | IB_PORT_IP_BASED_GIDS;
 	props->max_msg_sz = resp->attrs.max_msg_sz;
 	props->bad_pkey_cntr = resp->attrs.bad_pkey_cntr;
 	props->qkey_viol_cntr = resp->attrs.qkey_viol_cntr;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 3/3] RDMA/vmw_pvrdma: Add new port_cap flag

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

On Fri, Aug 18, 2017 at 09:44:49AM -0700, Adit Ranadive wrote:
> Set the IP_BASED_GIDS port cap flag for vmw_pvrdma.
>
> Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> ---


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.

Thanks,

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 3/3] RDMA/vmw_pvrdma: Add new port_cap flag

[Adit Ranadive]

On Sun, Aug 20, 2017 at 08:56:17AM +0300, Leon Romanovsky wrote:
> On Fri, Aug 18, 2017 at 09:44:49AM -0700, Adit Ranadive wrote:
> > Set the IP_BASED_GIDS port cap flag for vmw_pvrdma.
> >
> > Acked-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > ---
> 
> 
> 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.

Sure, thanks for the pointers.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html