[PATCH for-next 0/8] RDMA/hns: Support UD for HIP09

[PATCH for-next 0/8] RDMA/hns: Support UD for HIP09

[Weihang Li]

This series does cleanups on UD related code at first, including removing
dead code, adding necessary check and some refactors. Then the UD feature
is enabled on HIP09.

Jiaran Zhang (1):
  RDMA/hns: Add check for the validity of sl configuration in UD SQ WQE

Weihang Li (7):
  RDMA/hns: Only record vlan info for HIP08
  RDMA/hns: Fix missing fields in address vector
  RDMA/hns: Avoid setting loopback indicator when smac is same as dmac
  RDMA/hns: Remove the portn field in UD SQ WQE
  RDMA/hns: Simplify process of filling UD SQ WQE
  RDMA/hns: Add UD support for HIP09
  RDMA/hns: Add support for UD inline

 drivers/infiniband/hw/hns/hns_roce_ah.c     |  61 ++++----
 drivers/infiniband/hw/hns/hns_roce_device.h |   5 +-
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 206 ++++++++++++++++++++--------
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h  |  44 +++---
 drivers/infiniband/hw/hns/hns_roce_qp.c     |   9 +-
 5 files changed, 209 insertions(+), 116 deletions(-)

-- 
2.8.1

[PATCH for-next 1/8] RDMA/hns: Only record vlan info for HIP08

[Weihang Li]

Information about vlan is stored in GMV(GID/MAC/VLAN) table for HIP09, so
there is no need to copy it to address vector.

Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_ah.c     | 51 +++++++++++++----------------
 drivers/infiniband/hw/hns/hns_roce_device.h |  2 +-
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 13 +++++---
 3 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c
index 75b06db..3be80d4 100644
--- a/drivers/infiniband/hw/hns/hns_roce_ah.c
+++ b/drivers/infiniband/hw/hns/hns_roce_ah.c
@@ -31,13 +31,13 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/pci.h>
 #include <rdma/ib_addr.h>
 #include <rdma/ib_cache.h>
 #include "hns_roce_device.h"
 
-#define HNS_ROCE_PORT_NUM_SHIFT		24
-#define HNS_ROCE_VLAN_SL_BIT_MASK	7
-#define HNS_ROCE_VLAN_SL_SHIFT		13
+#define VLAN_SL_MASK 7
+#define VLAN_SL_SHIFT 13
 
 static inline u16 get_ah_udp_sport(const struct rdma_ah_attr *ah_attr)
 {
@@ -58,37 +58,16 @@ static inline u16 get_ah_udp_sport(const struct rdma_ah_attr *ah_attr)
 int hns_roce_create_ah(struct ib_ah *ibah, struct rdma_ah_init_attr *init_attr,
 		       struct ib_udata *udata)
 {
-	struct hns_roce_dev *hr_dev = to_hr_dev(ibah->device);
-	const struct ib_gid_attr *gid_attr;
-	struct device *dev = hr_dev->dev;
-	struct hns_roce_ah *ah = to_hr_ah(ibah);
 	struct rdma_ah_attr *ah_attr = init_attr->ah_attr;
 	const struct ib_global_route *grh = rdma_ah_read_grh(ah_attr);
-	u16 vlan_id = 0xffff;
-	bool vlan_en = false;
-	int ret;
-
-	gid_attr = ah_attr->grh.sgid_attr;
-	ret = rdma_read_gid_l2_fields(gid_attr, &vlan_id, NULL);
-	if (ret)
-		return ret;
+	struct hns_roce_dev *hr_dev = to_hr_dev(ibah->device);
+	struct hns_roce_ah *ah = to_hr_ah(ibah);
+	int ret = 0;
 
-	/* Get mac address */
 	memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN);
 
-	if (vlan_id < VLAN_N_VID) {
-		vlan_en = true;
-		vlan_id |= (rdma_ah_get_sl(ah_attr) &
-			     HNS_ROCE_VLAN_SL_BIT_MASK) <<
-			     HNS_ROCE_VLAN_SL_SHIFT;
-	}
-
 	ah->av.port = rdma_ah_get_port_num(ah_attr);
 	ah->av.gid_index = grh->sgid_index;
-	ah->av.vlan_id = vlan_id;
-	ah->av.vlan_en = vlan_en;
-	dev_dbg(dev, "gid_index = 0x%x,vlan_id = 0x%x\n", ah->av.gid_index,
-		ah->av.vlan_id);
 
 	if (rdma_ah_get_static_rate(ah_attr))
 		ah->av.stat_rate = IB_RATE_10_GBPS;
@@ -98,7 +77,23 @@ int hns_roce_create_ah(struct ib_ah *ibah, struct rdma_ah_init_attr *init_attr,
 	ah->av.flowlabel = grh->flow_label;
 	ah->av.udp_sport = get_ah_udp_sport(ah_attr);
 
-	return 0;
+	/* HIP08 needs to record vlan info in Address Vector */
+	if (hr_dev->pci_dev->revision <= PCI_REVISION_ID_HIP08) {
+		ah->av.vlan_en = 0;
+
+		ret = rdma_read_gid_l2_fields(ah_attr->grh.sgid_attr,
+					      &ah->av.vlan_id, NULL);
+		if (ret)
+			return ret;
+
+		if (ah->av.vlan_id < VLAN_N_VID) {
+			ah->av.vlan_en = 1;
+			ah->av.vlan_id |= (rdma_ah_get_sl(ah_attr) & VLAN_SL_MASK) <<
+					  VLAN_SL_SHIFT;
+		}
+	}
+
+	return ret;
 }
 
 int hns_roce_query_ah(struct ib_ah *ibah, struct rdma_ah_attr *ah_attr)
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 1d99022..9a032d0 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -549,7 +549,7 @@ struct hns_roce_av {
 	u8 dgid[HNS_ROCE_GID_SIZE];
 	u8 mac[ETH_ALEN];
 	u16 vlan_id;
-	bool vlan_en;
+	u8 vlan_en;
 };
 
 struct hns_roce_ah {
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 83842cd..499d72c 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -492,8 +492,6 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	roce_set_field(ud_sq_wqe->byte_32, V2_UD_SEND_WQE_BYTE_32_DQPN_M,
 		       V2_UD_SEND_WQE_BYTE_32_DQPN_S, ud_wr(wr)->remote_qpn);
 
-	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_VLAN_M,
-		       V2_UD_SEND_WQE_BYTE_36_VLAN_S, ah->av.vlan_id);
 	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_M,
 		       V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_S, ah->av.hop_limit);
 	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_TCLASS_M,
@@ -505,11 +503,18 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_PORTN_M,
 		       V2_UD_SEND_WQE_BYTE_40_PORTN_S, qp->port);
 
-	roce_set_bit(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S,
-		     ah->av.vlan_en ? 1 : 0);
 	roce_set_field(ud_sq_wqe->byte_48, V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M,
 		       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S, ah->av.gid_index);
 
+	if (hr_dev->pci_dev->revision <= PCI_REVISION_ID_HIP08) {
+		roce_set_bit(ud_sq_wqe->byte_40,
+			     V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S,
+			     ah->av.vlan_en);
+		roce_set_field(ud_sq_wqe->byte_36,
+			       V2_UD_SEND_WQE_BYTE_36_VLAN_M,
+			       V2_UD_SEND_WQE_BYTE_36_VLAN_S, ah->av.vlan_id);
+	}
+
 	memcpy(&ud_sq_wqe->dgid[0], &ah->av.dgid[0], GID_LEN_V2);
 
 	set_extend_sge(qp, wr, &curr_idx, valid_num_sge);
-- 
2.8.1

[PATCH for-next 2/8] RDMA/hns: Fix missing fields in address vector

[Weihang Li]

Traffic class and hop limit in address vector is not assigned from GRH, but
it will be filled into UD SQ WQE. So the hardware will get a wrong value.

Fixes: 82e620d9c3a0 ("RDMA/hns: Modify the data structure of hns_roce_av")
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_ah.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c
index 3be80d4..d65ff6a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_ah.c
+++ b/drivers/infiniband/hw/hns/hns_roce_ah.c
@@ -64,18 +64,20 @@ int hns_roce_create_ah(struct ib_ah *ibah, struct rdma_ah_init_attr *init_attr,
 	struct hns_roce_ah *ah = to_hr_ah(ibah);
 	int ret = 0;
 
-	memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN);
-
 	ah->av.port = rdma_ah_get_port_num(ah_attr);
 	ah->av.gid_index = grh->sgid_index;
 
 	if (rdma_ah_get_static_rate(ah_attr))
 		ah->av.stat_rate = IB_RATE_10_GBPS;
 
-	memcpy(ah->av.dgid, grh->dgid.raw, HNS_ROCE_GID_SIZE);
-	ah->av.sl = rdma_ah_get_sl(ah_attr);
+	ah->av.hop_limit = grh->hop_limit;
 	ah->av.flowlabel = grh->flow_label;
 	ah->av.udp_sport = get_ah_udp_sport(ah_attr);
+	ah->av.sl = rdma_ah_get_sl(ah_attr);
+	ah->av.tclass = grh->traffic_class;
+
+	memcpy(ah->av.dgid, grh->dgid.raw, HNS_ROCE_GID_SIZE);
+	memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN);
 
 	/* HIP08 needs to record vlan info in Address Vector */
 	if (hr_dev->pci_dev->revision <= PCI_REVISION_ID_HIP08) {
-- 
2.8.1

[PATCH for-next 3/8] RDMA/hns: Avoid setting loopback indicator when smac is same as dmac

[Weihang Li]

The loopback flag will be set to 1 by the hardware when the source mac
address is same as the destination mac address. So the driver don't need to
compare them.

Fixes: d6a3627e311c ("RDMA/hns: Optimize wqe buffer set flow for post send")
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 499d72c..7a1d30f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -433,8 +433,6 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	unsigned int curr_idx = *sge_idx;
 	int valid_num_sge;
 	u32 msg_len = 0;
-	bool loopback;
-	u8 *smac;
 	int ret;
 
 	valid_num_sge = calc_wr_sge_num(wr, &msg_len);
@@ -457,13 +455,6 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	roce_set_field(ud_sq_wqe->byte_48, V2_UD_SEND_WQE_BYTE_48_DMAC_5_M,
 		       V2_UD_SEND_WQE_BYTE_48_DMAC_5_S, ah->av.mac[5]);
 
-	/* MAC loopback */
-	smac = (u8 *)hr_dev->dev_addr[qp->port];
-	loopback = ether_addr_equal_unaligned(ah->av.mac, smac) ? 1 : 0;
-
-	roce_set_bit(ud_sq_wqe->byte_40,
-		     V2_UD_SEND_WQE_BYTE_40_LBI_S, loopback);
-
 	ud_sq_wqe->msg_len = cpu_to_le32(msg_len);
 
 	/* Set sig attr */
-- 
2.8.1

[PATCH for-next 4/8] RDMA/hns: Add check for the validity of sl configuration in UD SQ WQE

[Weihang Li]

From: Jiaran Zhang <zhangjiaran@huawei.com>

According to the RoCE v1 specification, the sl (service level) 0-7 are
mapped directly to priorities 0-7 respectively, sl 8-15 are reserved. The
driver should verify whether the value of sl is larger than 7, if so, an
exception should be returned.

Fixes: d6a3627e311c ("RDMA/hns: Optimize wqe buffer set flow for post send")
Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 7a1d30f..69386a5 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -427,9 +427,10 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 			     void *wqe, unsigned int *sge_idx,
 			     unsigned int owner_bit)
 {
-	struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device);
 	struct hns_roce_ah *ah = to_hr_ah(ud_wr(wr)->ah);
 	struct hns_roce_v2_ud_send_wqe *ud_sq_wqe = wqe;
+	struct ib_device *ib_dev = qp->ibqp.device;
+	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
 	unsigned int curr_idx = *sge_idx;
 	int valid_num_sge;
 	u32 msg_len = 0;
@@ -489,6 +490,13 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
 	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
 		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
+
+	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
+		ibdev_err(ib_dev,
+			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
+			  ah->av.sl, MAX_SERVICE_LEVEL);
+		return -EINVAL;
+	}
 	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_SL_M,
 		       V2_UD_SEND_WQE_BYTE_40_SL_S, ah->av.sl);
 	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_PORTN_M,
-- 
2.8.1

Re: [PATCH for-next 4/8] RDMA/hns: Add check for the validity of sl configuration in UD SQ WQE

[Jason Gunthorpe]

On Fri, Oct 30, 2020 at 07:39:31PM +0800, Weihang Li wrote:
> From: Jiaran Zhang <zhangjiaran@huawei.com>
> 
> According to the RoCE v1 specification, the sl (service level) 0-7 are
> mapped directly to priorities 0-7 respectively, sl 8-15 are reserved. The
> driver should verify whether the value of sl is larger than 7, if so, an
> exception should be returned.
> 
> Fixes: d6a3627e311c ("RDMA/hns: Optimize wqe buffer set flow for post send")
> Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com>
> Signed-off-by: Weihang Li <liweihang@huawei.com>
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> index 7a1d30f..69386a5 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> @@ -427,9 +427,10 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
>  			     void *wqe, unsigned int *sge_idx,
>  			     unsigned int owner_bit)
>  {
> -	struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device);
>  	struct hns_roce_ah *ah = to_hr_ah(ud_wr(wr)->ah);
>  	struct hns_roce_v2_ud_send_wqe *ud_sq_wqe = wqe;
> +	struct ib_device *ib_dev = qp->ibqp.device;
> +	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
>  	unsigned int curr_idx = *sge_idx;
>  	int valid_num_sge;
>  	u32 msg_len = 0;
> @@ -489,6 +490,13 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
>  		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
>  	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
>  		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
> +
> +	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
> +		ibdev_err(ib_dev,
> +			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
> +			  ah->av.sl, MAX_SERVICE_LEVEL);
> +		return -EINVAL;
> +	}

We should not print for things like this, IIRC userspace can cause the
ah's sl to become set out of bounds

Jason

Re: [PATCH for-next 4/8] RDMA/hns: Add check for the validity of sl configuration in UD SQ WQE

[liweihang]

On 2020/11/13 2:33, Jason Gunthorpe wrote:
> On Fri, Oct 30, 2020 at 07:39:31PM +0800, Weihang Li wrote:
>> From: Jiaran Zhang <zhangjiaran@huawei.com>
>>
>> According to the RoCE v1 specification, the sl (service level) 0-7 are
>> mapped directly to priorities 0-7 respectively, sl 8-15 are reserved. The
>> driver should verify whether the value of sl is larger than 7, if so, an
>> exception should be returned.
>>
>> Fixes: d6a3627e311c ("RDMA/hns: Optimize wqe buffer set flow for post send")
>> Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com>
>> Signed-off-by: Weihang Li <liweihang@huawei.com>
>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>> index 7a1d30f..69386a5 100644
>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>> @@ -427,9 +427,10 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
>>  			     void *wqe, unsigned int *sge_idx,
>>  			     unsigned int owner_bit)
>>  {
>> -	struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device);
>>  	struct hns_roce_ah *ah = to_hr_ah(ud_wr(wr)->ah);
>>  	struct hns_roce_v2_ud_send_wqe *ud_sq_wqe = wqe;
>> +	struct ib_device *ib_dev = qp->ibqp.device;
>> +	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
>>  	unsigned int curr_idx = *sge_idx;
>>  	int valid_num_sge;
>>  	u32 msg_len = 0;
>> @@ -489,6 +490,13 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
>>  		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
>>  	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
>>  		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
>> +
>> +	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
>> +		ibdev_err(ib_dev,
>> +			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
>> +			  ah->av.sl, MAX_SERVICE_LEVEL);
>> +		return -EINVAL;
>> +	}
> 
> We should not print for things like this, IIRC userspace can cause the
> ah's sl to become set out of bounds
> 
> Jason
> 

Hi Jason,

In "Annex A 16: RoCE", I found the following description:

	SL 0-7 are mapped directly to Priorities 0-7, respectively

	SL 8-15 are reserved.

	CA16-18: An attempt to use an Address Vector for a RoCE port containing
	a reserved SL value shall result in the Invalid Address Vector verb result.

So what should we do if the user wants to use the reserved sl? Should I just let it
do mask with 0x7 when creating AH?

Thanks
Weihang

Re: [PATCH for-next 4/8] RDMA/hns: Add check for the validity of sl configuration in UD SQ WQE

[Jason Gunthorpe]

On Sat, Nov 14, 2020 at 03:46:58AM +0000, liweihang wrote:
> On 2020/11/13 2:33, Jason Gunthorpe wrote:
> > On Fri, Oct 30, 2020 at 07:39:31PM +0800, Weihang Li wrote:
> >> From: Jiaran Zhang <zhangjiaran@huawei.com>
> >>
> >> According to the RoCE v1 specification, the sl (service level) 0-7 are
> >> mapped directly to priorities 0-7 respectively, sl 8-15 are reserved. The
> >> driver should verify whether the value of sl is larger than 7, if so, an
> >> exception should be returned.
> >>
> >> Fixes: d6a3627e311c ("RDMA/hns: Optimize wqe buffer set flow for post send")
> >> Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com>
> >> Signed-off-by: Weihang Li <liweihang@huawei.com>
> >>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >> index 7a1d30f..69386a5 100644
> >> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >> @@ -427,9 +427,10 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
> >>  			     void *wqe, unsigned int *sge_idx,
> >>  			     unsigned int owner_bit)
> >>  {
> >> -	struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device);
> >>  	struct hns_roce_ah *ah = to_hr_ah(ud_wr(wr)->ah);
> >>  	struct hns_roce_v2_ud_send_wqe *ud_sq_wqe = wqe;
> >> +	struct ib_device *ib_dev = qp->ibqp.device;
> >> +	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
> >>  	unsigned int curr_idx = *sge_idx;
> >>  	int valid_num_sge;
> >>  	u32 msg_len = 0;
> >> @@ -489,6 +490,13 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
> >>  		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
> >>  	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
> >>  		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
> >> +
> >> +	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
> >> +		ibdev_err(ib_dev,
> >> +			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
> >> +			  ah->av.sl, MAX_SERVICE_LEVEL);
> >> +		return -EINVAL;
> >> +	}
> > 
> > We should not print for things like this, IIRC userspace can cause the
> > ah's sl to become set out of bounds

 
> In "Annex A 16: RoCE", I found the following description:
> 
> 	SL 0-7 are mapped directly to Priorities 0-7, respectively
> 
> 	SL 8-15 are reserved.
> 
> 	CA16-18: An attempt to use an Address Vector for a RoCE port containing
> 	a reserved SL value shall result in the Invalid Address Vector verb result.
> 
> So what should we do if the user wants to use the reserved sl? Should I just let it
> do mask with 0x7 when creating AH?

Fail and don't print anything

Jason

Re: [PATCH for-next 4/8] RDMA/hns: Add check for the validity of sl configuration in UD SQ WQE

[liweihang]

On 2020/11/17 0:09, Jason Gunthorpe wrote:
> On Sat, Nov 14, 2020 at 03:46:58AM +0000, liweihang wrote:
>> On 2020/11/13 2:33, Jason Gunthorpe wrote:
>>> On Fri, Oct 30, 2020 at 07:39:31PM +0800, Weihang Li wrote:
>>>> From: Jiaran Zhang <zhangjiaran@huawei.com>
>>>>
>>>> According to the RoCE v1 specification, the sl (service level) 0-7 are
>>>> mapped directly to priorities 0-7 respectively, sl 8-15 are reserved. The
>>>> driver should verify whether the value of sl is larger than 7, if so, an
>>>> exception should be returned.
>>>>
>>>> Fixes: d6a3627e311c ("RDMA/hns: Optimize wqe buffer set flow for post send")
>>>> Signed-off-by: Jiaran Zhang <zhangjiaran@huawei.com>
>>>> Signed-off-by: Weihang Li <liweihang@huawei.com>
>>>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 10 +++++++++-
>>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>>>> index 7a1d30f..69386a5 100644
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
>>>> @@ -427,9 +427,10 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
>>>>  			     void *wqe, unsigned int *sge_idx,
>>>>  			     unsigned int owner_bit)
>>>>  {
>>>> -	struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device);
>>>>  	struct hns_roce_ah *ah = to_hr_ah(ud_wr(wr)->ah);
>>>>  	struct hns_roce_v2_ud_send_wqe *ud_sq_wqe = wqe;
>>>> +	struct ib_device *ib_dev = qp->ibqp.device;
>>>> +	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
>>>>  	unsigned int curr_idx = *sge_idx;
>>>>  	int valid_num_sge;
>>>>  	u32 msg_len = 0;
>>>> @@ -489,6 +490,13 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
>>>>  		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
>>>>  	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
>>>>  		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
>>>> +
>>>> +	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
>>>> +		ibdev_err(ib_dev,
>>>> +			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
>>>> +			  ah->av.sl, MAX_SERVICE_LEVEL);
>>>> +		return -EINVAL;
>>>> +	}
>>>
>>> We should not print for things like this, IIRC userspace can cause the
>>> ah's sl to become set out of bounds
> 
>  
>> In "Annex A 16: RoCE", I found the following description:
>>
>> 	SL 0-7 are mapped directly to Priorities 0-7, respectively
>>
>> 	SL 8-15 are reserved.
>>
>> 	CA16-18: An attempt to use an Address Vector for a RoCE port containing
>> 	a reserved SL value shall result in the Invalid Address Vector verb result.
>>
>> So what should we do if the user wants to use the reserved sl? Should I just let it
>> do mask with 0x7 when creating AH?
> 
> Fail and don't print anything
> 
> Jason
> 

OK, thank you.

Weihang

[PATCH for-next 5/8] RDMA/hns: Remove the portn field in UD SQ WQE

[Weihang Li]

This field in UD WQE in not used by hardware.

Fixes: 7bdee4158b37 ("RDMA/hns: Fill sq wqe context of ud type in hip08")
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 2 --
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 69386a5..99e4189 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -499,8 +499,6 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	}
 	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_SL_M,
 		       V2_UD_SEND_WQE_BYTE_40_SL_S, ah->av.sl);
-	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_PORTN_M,
-		       V2_UD_SEND_WQE_BYTE_40_PORTN_S, qp->port);
 
 	roce_set_field(ud_sq_wqe->byte_48, V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M,
 		       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S, ah->av.gid_index);
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
index 1409d05..1466888 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
@@ -1121,9 +1121,6 @@ struct hns_roce_v2_ud_send_wqe {
 #define	V2_UD_SEND_WQE_BYTE_40_SL_S 20
 #define V2_UD_SEND_WQE_BYTE_40_SL_M GENMASK(23, 20)
 
-#define	V2_UD_SEND_WQE_BYTE_40_PORTN_S 24
-#define V2_UD_SEND_WQE_BYTE_40_PORTN_M GENMASK(26, 24)
-
 #define V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S 30
 
 #define	V2_UD_SEND_WQE_BYTE_40_LBI_S 31
-- 
2.8.1

[PATCH for-next 6/8] RDMA/hns: Simplify process of filling UD SQ WQE

[Weihang Li]

There are some codes can be simplified or encapsulated in set_ud_wqe() to
make them easier to be understand.

Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 99 +++++++++++++++---------------
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 29 +--------
 2 files changed, 51 insertions(+), 77 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 99e4189..ab68e6b 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -422,6 +422,48 @@ static int set_ud_opcode(struct hns_roce_v2_ud_send_wqe *ud_sq_wqe,
 	return 0;
 }
 
+static int fill_ud_av(struct hns_roce_v2_ud_send_wqe *ud_sq_wqe,
+		      struct hns_roce_ah *ah)
+{
+	struct ib_device *ib_dev = ah->ibah.device;
+	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
+
+	roce_set_field(ud_sq_wqe->byte_24, V2_UD_SEND_WQE_BYTE_24_UDPSPN_M,
+		       V2_UD_SEND_WQE_BYTE_24_UDPSPN_S, ah->av.udp_sport);
+
+	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_M,
+		       V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_S, ah->av.hop_limit);
+	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_TCLASS_M,
+		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
+	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
+		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
+
+	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
+		ibdev_err(ib_dev,
+			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
+			  ah->av.sl, MAX_SERVICE_LEVEL);
+		return -EINVAL;
+	}
+
+	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_SL_M,
+		       V2_UD_SEND_WQE_BYTE_40_SL_S, ah->av.sl);
+
+	ud_sq_wqe->sgid_index = ah->av.gid_index;
+
+	memcpy(ud_sq_wqe->dmac, ah->av.mac, ETH_ALEN);
+	memcpy(ud_sq_wqe->dgid, ah->av.dgid, GID_LEN_V2);
+
+	if (hr_dev->pci_dev->revision >= PCI_REVISION_ID_HIP09)
+		return 0;
+
+	roce_set_bit(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S,
+		     ah->av.vlan_en);
+	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_VLAN_M,
+		       V2_UD_SEND_WQE_BYTE_36_VLAN_S, ah->av.vlan_id);
+
+	return 0;
+}
+
 static inline int set_ud_wqe(struct hns_roce_qp *qp,
 			     const struct ib_send_wr *wr,
 			     void *wqe, unsigned int *sge_idx,
@@ -429,10 +471,8 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 {
 	struct hns_roce_ah *ah = to_hr_ah(ud_wr(wr)->ah);
 	struct hns_roce_v2_ud_send_wqe *ud_sq_wqe = wqe;
-	struct ib_device *ib_dev = qp->ibqp.device;
-	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
 	unsigned int curr_idx = *sge_idx;
-	int valid_num_sge;
+	unsigned int valid_num_sge;
 	u32 msg_len = 0;
 	int ret;
 
@@ -443,28 +483,13 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	if (WARN_ON(ret))
 		return ret;
 
-	roce_set_field(ud_sq_wqe->dmac, V2_UD_SEND_WQE_DMAC_0_M,
-		       V2_UD_SEND_WQE_DMAC_0_S, ah->av.mac[0]);
-	roce_set_field(ud_sq_wqe->dmac, V2_UD_SEND_WQE_DMAC_1_M,
-		       V2_UD_SEND_WQE_DMAC_1_S, ah->av.mac[1]);
-	roce_set_field(ud_sq_wqe->dmac, V2_UD_SEND_WQE_DMAC_2_M,
-		       V2_UD_SEND_WQE_DMAC_2_S, ah->av.mac[2]);
-	roce_set_field(ud_sq_wqe->dmac, V2_UD_SEND_WQE_DMAC_3_M,
-		       V2_UD_SEND_WQE_DMAC_3_S, ah->av.mac[3]);
-	roce_set_field(ud_sq_wqe->byte_48, V2_UD_SEND_WQE_BYTE_48_DMAC_4_M,
-		       V2_UD_SEND_WQE_BYTE_48_DMAC_4_S, ah->av.mac[4]);
-	roce_set_field(ud_sq_wqe->byte_48, V2_UD_SEND_WQE_BYTE_48_DMAC_5_M,
-		       V2_UD_SEND_WQE_BYTE_48_DMAC_5_S, ah->av.mac[5]);
-
 	ud_sq_wqe->msg_len = cpu_to_le32(msg_len);
 
-	/* Set sig attr */
 	roce_set_bit(ud_sq_wqe->byte_4, V2_UD_SEND_WQE_BYTE_4_CQE_S,
-		     (wr->send_flags & IB_SEND_SIGNALED) ? 1 : 0);
+		     !!(wr->send_flags & IB_SEND_SIGNALED));
 
-	/* Set se attr */
 	roce_set_bit(ud_sq_wqe->byte_4, V2_UD_SEND_WQE_BYTE_4_SE_S,
-		     (wr->send_flags & IB_SEND_SOLICITED) ? 1 : 0);
+		     !!(wr->send_flags & IB_SEND_SOLICITED));
 
 	roce_set_field(ud_sq_wqe->byte_16, V2_UD_SEND_WQE_BYTE_16_PD_M,
 		       V2_UD_SEND_WQE_BYTE_16_PD_S, to_hr_pd(qp->ibqp.pd)->pdn);
@@ -477,42 +502,14 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 		       V2_UD_SEND_WQE_BYTE_20_MSG_START_SGE_IDX_S,
 		       curr_idx & (qp->sge.sge_cnt - 1));
 
-	roce_set_field(ud_sq_wqe->byte_24, V2_UD_SEND_WQE_BYTE_24_UDPSPN_M,
-		       V2_UD_SEND_WQE_BYTE_24_UDPSPN_S, ah->av.udp_sport);
 	ud_sq_wqe->qkey = cpu_to_le32(ud_wr(wr)->remote_qkey & 0x80000000 ?
 			  qp->qkey : ud_wr(wr)->remote_qkey);
 	roce_set_field(ud_sq_wqe->byte_32, V2_UD_SEND_WQE_BYTE_32_DQPN_M,
 		       V2_UD_SEND_WQE_BYTE_32_DQPN_S, ud_wr(wr)->remote_qpn);
 
-	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_M,
-		       V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_S, ah->av.hop_limit);
-	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_TCLASS_M,
-		       V2_UD_SEND_WQE_BYTE_36_TCLASS_S, ah->av.tclass);
-	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_M,
-		       V2_UD_SEND_WQE_BYTE_40_FLOW_LABEL_S, ah->av.flowlabel);
-
-	if (unlikely(ah->av.sl > MAX_SERVICE_LEVEL)) {
-		ibdev_err(ib_dev,
-			  "failed to fill ud av, ud sl (%d) shouldn't be larger than %d.\n",
-			  ah->av.sl, MAX_SERVICE_LEVEL);
-		return -EINVAL;
-	}
-	roce_set_field(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_SL_M,
-		       V2_UD_SEND_WQE_BYTE_40_SL_S, ah->av.sl);
-
-	roce_set_field(ud_sq_wqe->byte_48, V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M,
-		       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S, ah->av.gid_index);
-
-	if (hr_dev->pci_dev->revision <= PCI_REVISION_ID_HIP08) {
-		roce_set_bit(ud_sq_wqe->byte_40,
-			     V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S,
-			     ah->av.vlan_en);
-		roce_set_field(ud_sq_wqe->byte_36,
-			       V2_UD_SEND_WQE_BYTE_36_VLAN_M,
-			       V2_UD_SEND_WQE_BYTE_36_VLAN_S, ah->av.vlan_id);
-	}
-
-	memcpy(&ud_sq_wqe->dgid[0], &ah->av.dgid[0], GID_LEN_V2);
+	ret = fill_ud_av(ud_sq_wqe, ah);
+	if (ret)
+		return ret;
 
 	set_extend_sge(qp, wr, &curr_idx, valid_num_sge);
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
index 1466888..c068517 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
@@ -1077,8 +1077,9 @@ struct hns_roce_v2_ud_send_wqe {
 	__le32	byte_32;
 	__le32	byte_36;
 	__le32	byte_40;
-	__le32	dmac;
-	__le32	byte_48;
+	u8	dmac[ETH_ALEN];
+	u8	sgid_index;
+	u8	smac_index;
 	u8	dgid[GID_LEN_V2];
 
 };
@@ -1125,30 +1126,6 @@ struct hns_roce_v2_ud_send_wqe {
 
 #define	V2_UD_SEND_WQE_BYTE_40_LBI_S 31
 
-#define	V2_UD_SEND_WQE_DMAC_0_S 0
-#define V2_UD_SEND_WQE_DMAC_0_M GENMASK(7, 0)
-
-#define	V2_UD_SEND_WQE_DMAC_1_S 8
-#define V2_UD_SEND_WQE_DMAC_1_M GENMASK(15, 8)
-
-#define	V2_UD_SEND_WQE_DMAC_2_S 16
-#define V2_UD_SEND_WQE_DMAC_2_M GENMASK(23, 16)
-
-#define	V2_UD_SEND_WQE_DMAC_3_S 24
-#define V2_UD_SEND_WQE_DMAC_3_M GENMASK(31, 24)
-
-#define	V2_UD_SEND_WQE_BYTE_48_DMAC_4_S 0
-#define V2_UD_SEND_WQE_BYTE_48_DMAC_4_M GENMASK(7, 0)
-
-#define	V2_UD_SEND_WQE_BYTE_48_DMAC_5_S 8
-#define V2_UD_SEND_WQE_BYTE_48_DMAC_5_M GENMASK(15, 8)
-
-#define	V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S 16
-#define V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M GENMASK(23, 16)
-
-#define	V2_UD_SEND_WQE_BYTE_48_SMAC_INDX_S 24
-#define V2_UD_SEND_WQE_BYTE_48_SMAC_INDX_M GENMASK(31, 24)
-
 struct hns_roce_v2_rc_send_wqe {
 	__le32		byte_4;
 	__le32		msg_len;
-- 
2.8.1

[PATCH for-next 7/8] RDMA/hns: Add UD support for HIP09

[Weihang Li]

HIP09 supports service type of Unreliable Datagram, add necessary process
to enable this feature.

Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h | 2 ++
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 8 +++++---
 drivers/infiniband/hw/hns/hns_roce_qp.c     | 3 ++-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 9a032d0..23f8fe7 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -222,7 +222,9 @@ enum {
 	HNS_ROCE_CAP_FLAG_FRMR                  = BIT(8),
 	HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL		= BIT(9),
 	HNS_ROCE_CAP_FLAG_ATOMIC		= BIT(10),
+	HNS_ROCE_CAP_FLAG_UD			= BIT(11),
 	HNS_ROCE_CAP_FLAG_SDI_MODE		= BIT(14),
+
 };
 
 #define HNS_ROCE_DB_TYPE_COUNT			2
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index ab68e6b..29cbf9f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -701,7 +701,7 @@ static int hns_roce_v2_post_send(struct ib_qp *ibqp,
 		       ~(((qp->sq.head + nreq) >> ilog2(qp->sq.wqe_cnt)) & 0x1);
 
 		/* Corresponding to the QP type, wqe process separately */
-		if (ibqp->qp_type == IB_QPT_GSI)
+		if (ibqp->qp_type == IB_QPT_GSI || ibqp->qp_type == IB_QPT_UD)
 			ret = set_ud_wqe(qp, wr, wqe, &sge_idx, owner_bit);
 		else if (ibqp->qp_type == IB_QPT_RC)
 			ret = set_rc_wqe(qp, wr, wqe, &sge_idx, owner_bit);
@@ -1892,7 +1892,7 @@ static void set_default_caps(struct hns_roce_dev *hr_dev)
 
 	caps->flags |= HNS_ROCE_CAP_FLAG_ATOMIC | HNS_ROCE_CAP_FLAG_MW |
 		       HNS_ROCE_CAP_FLAG_SRQ | HNS_ROCE_CAP_FLAG_FRMR |
-		       HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL;
+		       HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL | HNS_ROCE_CAP_FLAG_UD;
 
 	caps->num_qpc_timer	  = HNS_ROCE_V2_MAX_QPC_TIMER_NUM;
 	caps->qpc_timer_entry_sz  = HNS_ROCE_V2_QPC_TIMER_ENTRY_SZ;
@@ -5157,7 +5157,9 @@ static int hns_roce_v2_destroy_qp_common(struct hns_roce_dev *hr_dev,
 	unsigned long flags;
 	int ret = 0;
 
-	if (hr_qp->ibqp.qp_type == IB_QPT_RC && hr_qp->state != IB_QPS_RESET) {
+	if ((hr_qp->ibqp.qp_type == IB_QPT_RC ||
+	     hr_qp->ibqp.qp_type == IB_QPT_UD) &&
+	   hr_qp->state != IB_QPS_RESET) {
 		/* Modify qp to reset before destroying qp */
 		ret = hns_roce_v2_modify_qp(&hr_qp->ibqp, NULL, 0,
 					    hr_qp->state, IB_QPS_RESET);
diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index e288946..9ffd92a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -1010,6 +1010,7 @@ struct ib_qp *hns_roce_create_qp(struct ib_pd *pd,
 	switch (init_attr->qp_type) {
 	case IB_QPT_RC:
 	case IB_QPT_GSI:
+	case IB_QPT_UD:
 		break;
 	default:
 		ibdev_err(ibdev, "not support QP type %d\n",
@@ -1030,7 +1031,7 @@ struct ib_qp *hns_roce_create_qp(struct ib_pd *pd,
 	if (ret) {
 		ibdev_err(ibdev, "Create QP type 0x%x failed(%d)\n",
 			  init_attr->qp_type, ret);
-		ibdev_err(ibdev, "Create GSI QP failed!\n");
+
 		kfree(hr_qp);
 		return ERR_PTR(ret);
 	}
-- 
2.8.1

Re: [PATCH for-next 7/8] RDMA/hns: Add UD support for HIP09

[Jason Gunthorpe]

On Fri, Oct 30, 2020 at 07:39:34PM +0800, Weihang Li wrote:
> HIP09 supports service type of Unreliable Datagram, add necessary process
> to enable this feature.
> 
> Signed-off-by: Weihang Li <liweihang@huawei.com>
>  drivers/infiniband/hw/hns/hns_roce_device.h | 2 ++
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 8 +++++---
>  drivers/infiniband/hw/hns/hns_roce_qp.c     | 3 ++-
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> index 9a032d0..23f8fe7 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> @@ -222,7 +222,9 @@ enum {
>  	HNS_ROCE_CAP_FLAG_FRMR                  = BIT(8),
>  	HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL		= BIT(9),
>  	HNS_ROCE_CAP_FLAG_ATOMIC		= BIT(10),
> +	HNS_ROCE_CAP_FLAG_UD			= BIT(11),

Why add this flag if nothing reads it?

>  	HNS_ROCE_CAP_FLAG_SDI_MODE		= BIT(14),
> +
>  };

Extra space


If I recall properly earlier chips did not have a GID table so could
not support UD because they could not secure the AH, or something like
that.

So, I would expect to see that only the new devices support UD, but
I can't quite see that in this patch??

Jason

Re: [PATCH for-next 7/8] RDMA/hns: Add UD support for HIP09

[liweihang]

On 2020/11/13 2:35, Jason Gunthorpe wrote:
> On Fri, Oct 30, 2020 at 07:39:34PM +0800, Weihang Li wrote:
>> HIP09 supports service type of Unreliable Datagram, add necessary process
>> to enable this feature.
>>
>> Signed-off-by: Weihang Li <liweihang@huawei.com>
>>  drivers/infiniband/hw/hns/hns_roce_device.h | 2 ++
>>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 8 +++++---
>>  drivers/infiniband/hw/hns/hns_roce_qp.c     | 3 ++-
>>  3 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
>> index 9a032d0..23f8fe7 100644
>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>> @@ -222,7 +222,9 @@ enum {
>>  	HNS_ROCE_CAP_FLAG_FRMR                  = BIT(8),
>>  	HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL		= BIT(9),
>>  	HNS_ROCE_CAP_FLAG_ATOMIC		= BIT(10),
>> +	HNS_ROCE_CAP_FLAG_UD			= BIT(11),
> 
> Why add this flag if nothing reads it?

Hi Jason,

I checked it to set IB_USER_VERBS_CMD_CREATE_AH and IB_USER_VERBS_CMD_DESTROY_AH
in uverbs_cmd_mask which is not needed recently, I forgot to remove it in this
patch :) Will drop it.

> 
>>  	HNS_ROCE_CAP_FLAG_SDI_MODE		= BIT(14),
>> +
>>  };
> 
> Extra space

OK, will remove it.

> 
> 
> If I recall properly earlier chips did not have a GID table so could
> not support UD because they could not secure the AH, or something like
> that.
> 
> So, I would expect to see that only the new devices support UD, but
> I can't quite see that in this patch??

You are right. I made a mistake, I thought it's enough to add judgment of hardware
version in rdma-core to prevent the HIP08's user from using UD in userspace and I
realize it's meaningless just now...

I think I can add a check about the hardware version when creating a user's UD QP.

Thanks again for the reminder.

Weihang

> 
> Jason
> 

[PATCH for-next 8/8] RDMA/hns: Add support for UD inline

[Weihang Li]

HIP09 supports UD inline up to size of 1024 Bytes. When data size is
smaller than 8 bytes, they will be stored in sqwqe. Otherwise, the data
will be filled into extended sges.

Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h |   3 +-
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 111 ++++++++++++++++++++++++++--
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h  |  14 ++++
 drivers/infiniband/hw/hns/hns_roce_qp.c     |   6 ++
 4 files changed, 126 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 23f8fe7..bd19bee 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -133,6 +133,7 @@ enum hns_roce_qp_caps {
 	HNS_ROCE_QP_CAP_RQ_RECORD_DB = BIT(0),
 	HNS_ROCE_QP_CAP_SQ_RECORD_DB = BIT(1),
 	HNS_ROCE_QP_CAP_OWNER_DB = BIT(2),
+	HNS_ROCE_QP_CAP_UD_SQ_INL = BIT(3),
 };
 
 enum hns_roce_cq_flags {
@@ -223,8 +224,8 @@ enum {
 	HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL		= BIT(9),
 	HNS_ROCE_CAP_FLAG_ATOMIC		= BIT(10),
 	HNS_ROCE_CAP_FLAG_UD			= BIT(11),
+	HNS_ROCE_CAP_FLAG_UD_SQ_INL		= BIT(13),
 	HNS_ROCE_CAP_FLAG_SDI_MODE		= BIT(14),
-
 };
 
 #define HNS_ROCE_DB_TYPE_COUNT			2
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 29cbf9f..2de9519 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -428,9 +428,6 @@ static int fill_ud_av(struct hns_roce_v2_ud_send_wqe *ud_sq_wqe,
 	struct ib_device *ib_dev = ah->ibah.device;
 	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
 
-	roce_set_field(ud_sq_wqe->byte_24, V2_UD_SEND_WQE_BYTE_24_UDPSPN_M,
-		       V2_UD_SEND_WQE_BYTE_24_UDPSPN_S, ah->av.udp_sport);
-
 	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_M,
 		       V2_UD_SEND_WQE_BYTE_36_HOPLIMIT_S, ah->av.hop_limit);
 	roce_set_field(ud_sq_wqe->byte_36, V2_UD_SEND_WQE_BYTE_36_TCLASS_M,
@@ -464,6 +461,90 @@ static int fill_ud_av(struct hns_roce_v2_ud_send_wqe *ud_sq_wqe,
 	return 0;
 }
 
+static void fill_ud_inn_inl_data(const struct ib_send_wr *wr,
+				 struct hns_roce_v2_ud_send_wqe *ud_sq_wqe)
+{
+	u8 data[HNS_ROCE_V2_MAX_UD_INL_INN_SZ] = {0};
+	u32 *loc = (u32 *)data;
+	void *tmp = data;
+	unsigned int i;
+	u32 tmp_data;
+
+	for (i = 0; i < wr->num_sge; i++) {
+		memcpy(tmp, ((void *)wr->sg_list[i].addr),
+		       wr->sg_list[i].length);
+		tmp += wr->sg_list[i].length;
+	}
+
+	roce_set_field(ud_sq_wqe->msg_len,
+		       V2_UD_SEND_WQE_BYTE_8_INL_DATA_15_0_M,
+		       V2_UD_SEND_WQE_BYTE_8_INL_DATA_15_0_S,
+		       *loc & 0xffff);
+
+	roce_set_field(ud_sq_wqe->byte_16,
+		       V2_UD_SEND_WQE_BYTE_16_INL_DATA_23_16_M,
+		       V2_UD_SEND_WQE_BYTE_16_INL_DATA_23_16_S,
+		       (*loc >> 16) & 0xff);
+
+	tmp_data = *loc >> 24;
+	loc++;
+	tmp_data |= ((*loc & 0xffff) << 8);
+
+	roce_set_field(ud_sq_wqe->byte_20,
+		       V2_UD_SEND_WQE_BYTE_20_INL_DATA_47_24_M,
+		       V2_UD_SEND_WQE_BYTE_20_INL_DATA_47_24_S,
+		       tmp_data);
+
+	roce_set_field(ud_sq_wqe->byte_24,
+		       V2_UD_SEND_WQE_BYTE_24_INL_DATA_63_48_M,
+		       V2_UD_SEND_WQE_BYTE_24_INL_DATA_63_48_S,
+		       *loc >> 16);
+}
+
+static int set_ud_inl(struct hns_roce_qp *qp, const struct ib_send_wr *wr,
+		      struct hns_roce_v2_ud_send_wqe *ud_sq_wqe,
+		      unsigned int *sge_idx)
+{
+	struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device);
+	u32 msg_len = le32_to_cpu(ud_sq_wqe->msg_len);
+	struct ib_device *ibdev = &hr_dev->ib_dev;
+	unsigned int curr_idx = *sge_idx;
+	int ret;
+
+	if (!(qp->en_flags & HNS_ROCE_QP_CAP_UD_SQ_INL)) {
+		ibdev_err(ibdev, "not support UD SQ inline!\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (!check_inl_data_len(qp, msg_len))
+		return -EINVAL;
+
+	roce_set_bit(ud_sq_wqe->byte_4, V2_UD_SEND_WQE_BYTE_4_INL_S, 1);
+
+	if (msg_len <= HNS_ROCE_V2_MAX_UD_INL_INN_SZ) {
+		roce_set_bit(ud_sq_wqe->byte_20,
+			     V2_UD_SEND_WQE_BYTE_20_INL_TYPE_S, 0);
+
+		fill_ud_inn_inl_data(wr, ud_sq_wqe);
+	} else {
+		roce_set_bit(ud_sq_wqe->byte_20,
+			     V2_UD_SEND_WQE_BYTE_20_INL_TYPE_S, 1);
+
+		ret = fill_ext_sge_inl_data(qp, wr, &curr_idx, msg_len);
+		if (ret)
+			return ret;
+
+		roce_set_field(ud_sq_wqe->byte_16,
+			       V2_UD_SEND_WQE_BYTE_16_SGE_NUM_M,
+			       V2_UD_SEND_WQE_BYTE_16_SGE_NUM_S,
+			       curr_idx - *sge_idx);
+	}
+
+	*sge_idx = curr_idx;
+
+	return 0;
+}
+
 static inline int set_ud_wqe(struct hns_roce_qp *qp,
 			     const struct ib_send_wr *wr,
 			     void *wqe, unsigned int *sge_idx,
@@ -494,9 +575,6 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	roce_set_field(ud_sq_wqe->byte_16, V2_UD_SEND_WQE_BYTE_16_PD_M,
 		       V2_UD_SEND_WQE_BYTE_16_PD_S, to_hr_pd(qp->ibqp.pd)->pdn);
 
-	roce_set_field(ud_sq_wqe->byte_16, V2_UD_SEND_WQE_BYTE_16_SGE_NUM_M,
-		       V2_UD_SEND_WQE_BYTE_16_SGE_NUM_S, valid_num_sge);
-
 	roce_set_field(ud_sq_wqe->byte_20,
 		       V2_UD_SEND_WQE_BYTE_20_MSG_START_SGE_IDX_M,
 		       V2_UD_SEND_WQE_BYTE_20_MSG_START_SGE_IDX_S,
@@ -511,7 +589,23 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp,
 	if (ret)
 		return ret;
 
-	set_extend_sge(qp, wr, &curr_idx, valid_num_sge);
+	if (wr->send_flags & IB_SEND_INLINE) {
+		ret = set_ud_inl(qp, wr, ud_sq_wqe, &curr_idx);
+		if (ret)
+			return ret;
+	} else {
+		roce_set_field(ud_sq_wqe->byte_16,
+			       V2_UD_SEND_WQE_BYTE_16_SGE_NUM_M,
+			       V2_UD_SEND_WQE_BYTE_16_SGE_NUM_S,
+			       valid_num_sge);
+
+		roce_set_field(ud_sq_wqe->byte_24,
+			       V2_UD_SEND_WQE_BYTE_24_UDPSPN_M,
+			       V2_UD_SEND_WQE_BYTE_24_UDPSPN_S,
+			       ah->av.udp_sport);
+
+		set_extend_sge(qp, wr, &curr_idx, valid_num_sge);
+	}
 
 	/*
 	 * The pipeline can sequentially post all valid WQEs into WQ buffer,
@@ -1924,6 +2018,8 @@ static void set_default_caps(struct hns_roce_dev *hr_dev)
 		caps->gmv_buf_pg_sz = 0;
 		caps->gid_table_len[0] = caps->gmv_bt_num * (HNS_HW_PAGE_SIZE /
 					 caps->gmv_entry_sz);
+		caps->flags |= HNS_ROCE_CAP_FLAG_UD_SQ_INL;
+		caps->max_sq_inline = HNS_ROCE_V2_MAX_SQ_INL_EXT;
 	}
 }
 
@@ -5131,6 +5227,7 @@ static int hns_roce_v2_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 	qp_attr->cur_qp_state = qp_attr->qp_state;
 	qp_attr->cap.max_recv_wr = hr_qp->rq.wqe_cnt;
 	qp_attr->cap.max_recv_sge = hr_qp->rq.max_gs;
+	qp_attr->cap.max_inline_data = hr_qp->max_inline_data;
 
 	if (!ibqp->uobject) {
 		qp_attr->cap.max_send_wr = hr_qp->sq.wqe_cnt;
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
index c068517..1c1a773 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
@@ -61,6 +61,8 @@
 #define HNS_ROCE_V2_MAX_SQ_SGE_NUM		64
 #define HNS_ROCE_V2_MAX_EXTEND_SGE_NUM		0x200000
 #define HNS_ROCE_V2_MAX_SQ_INLINE		0x20
+#define HNS_ROCE_V2_MAX_SQ_INL_EXT		0x400
+#define HNS_ROCE_V2_MAX_UD_INL_INN_SZ		8
 #define HNS_ROCE_V2_MAX_RC_INL_INN_SZ		32
 #define HNS_ROCE_V2_UAR_NUM			256
 #define HNS_ROCE_V2_PHY_UAR_NUM			1
@@ -1126,6 +1128,18 @@ struct hns_roce_v2_ud_send_wqe {
 
 #define	V2_UD_SEND_WQE_BYTE_40_LBI_S 31
 
+#define V2_UD_SEND_WQE_BYTE_4_INL_S 12
+#define V2_UD_SEND_WQE_BYTE_20_INL_TYPE_S 31
+
+#define V2_UD_SEND_WQE_BYTE_8_INL_DATA_15_0_S 16
+#define V2_UD_SEND_WQE_BYTE_8_INL_DATA_15_0_M GENMASK(31, 16)
+#define V2_UD_SEND_WQE_BYTE_16_INL_DATA_23_16_S 24
+#define V2_UD_SEND_WQE_BYTE_16_INL_DATA_23_16_M GENMASK(31, 24)
+#define V2_UD_SEND_WQE_BYTE_20_INL_DATA_47_24_S 0
+#define V2_UD_SEND_WQE_BYTE_20_INL_DATA_47_24_M GENMASK(23, 0)
+#define V2_UD_SEND_WQE_BYTE_24_INL_DATA_63_48_S 0
+#define V2_UD_SEND_WQE_BYTE_24_INL_DATA_63_48_M GENMASK(15, 0)
+
 struct hns_roce_v2_rc_send_wqe {
 	__le32		byte_4;
 	__le32		msg_len;
diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index 9ffd92a..2dd325c 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -862,6 +862,9 @@ static int set_qp_param(struct hns_roce_dev *hr_dev, struct hns_roce_qp *hr_qp,
 		return ret;
 	}
 
+	if (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_UD_SQ_INL)
+		hr_qp->en_flags |= HNS_ROCE_QP_CAP_UD_SQ_INL;
+
 	if (udata) {
 		if (ib_copy_from_udata(ucmd, udata, sizeof(*ucmd))) {
 			ibdev_err(ibdev, "Failed to copy QP ucmd\n");
@@ -946,6 +949,9 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
 	}
 
 	if (udata) {
+		if (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_UD_SQ_INL)
+			resp.cap_flags |= HNS_ROCE_QP_CAP_UD_SQ_INL;
+
 		ret = ib_copy_to_udata(udata, &resp,
 				       min(udata->outlen, sizeof(resp)));
 		if (ret) {
-- 
2.8.1