[PATCH 0/3] Annotate imm_data as be32 in the uapi header

[PATCH 0/3] Annotate imm_data as be32 in the uapi header

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This matches what the user space version of this struct has had for some time
and it removed several __force casts from the tree.

hns and qedr are updated to match what their userspace providers are doing.

Jason Gunthorpe (3):
  RDMA: Mark imm_data as be32 in the verbs uapi header
  RDMA/hns: Fix endian problems around imm_data and rkey
  RDMA/qedr: Fix endian problems around imm_data

 drivers/infiniband/core/uverbs_cmd.c       | 2 +-
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 7 ++++---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 6 +++---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 5 ++++-
 drivers/infiniband/hw/qedr/verbs.c         | 4 ++--
 drivers/infiniband/sw/rdmavt/cq.c          | 3 +--
 drivers/infiniband/sw/rxe/rxe_resp.c       | 3 +--
 include/uapi/rdma/ib_user_verbs.h          | 4 ++--
 8 files changed, 18 insertions(+), 16 deletions(-)

-- 
2.15.1

--
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 1/3] RDMA: Mark imm_data as be32 in the verbs uapi header

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This matches what the userspace copy of this header has been doing
for a while. imm_data is an opaque 4 byte array carried over the network,
and invalidate_rkey is in CPU byte order.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 2 +-
 drivers/infiniband/sw/rdmavt/cq.c    | 3 +--
 drivers/infiniband/sw/rxe/rxe_resp.c | 3 +--
 include/uapi/rdma/ib_user_verbs.h    | 4 ++--
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index c216d98bb816a0..4ddd61d905075c 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1196,7 +1196,7 @@ static int copy_wc_to_user(struct ib_device *ib_dev, void __user *dest,
 	tmp.opcode		= wc->opcode;
 	tmp.vendor_err		= wc->vendor_err;
 	tmp.byte_len		= wc->byte_len;
-	tmp.ex.imm_data		= (__u32 __force) wc->ex.imm_data;
+	tmp.ex.imm_data		= wc->ex.imm_data;
 	tmp.qp_num		= wc->qp->qp_num;
 	tmp.src_qp		= wc->src_qp;
 	tmp.wc_flags		= wc->wc_flags;
diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
index 61fca14c2598be..fb52b669bfce5c 100644
--- a/drivers/infiniband/sw/rdmavt/cq.c
+++ b/drivers/infiniband/sw/rdmavt/cq.c
@@ -101,8 +101,7 @@ void rvt_cq_enter(struct rvt_cq *cq, struct ib_wc *entry, bool solicited)
 		wc->uqueue[head].opcode = entry->opcode;
 		wc->uqueue[head].vendor_err = entry->vendor_err;
 		wc->uqueue[head].byte_len = entry->byte_len;
-		wc->uqueue[head].ex.imm_data =
-			(__u32 __force)entry->ex.imm_data;
+		wc->uqueue[head].ex.imm_data = entry->ex.imm_data;
 		wc->uqueue[head].qp_num = entry->qp->qp_num;
 		wc->uqueue[head].src_qp = entry->src_qp;
 		wc->uqueue[head].wc_flags = entry->wc_flags;
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 4240866a533192..f07324f2cde207 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -863,8 +863,7 @@ static enum resp_states do_complete(struct rxe_qp *qp,
 
 			if (pkt->mask & RXE_IMMDT_MASK) {
 				uwc->wc_flags |= IB_WC_WITH_IMM;
-				uwc->ex.imm_data =
-					(__u32 __force)immdt_imm(pkt);
+				uwc->ex.imm_data = immdt_imm(pkt);
 			}
 
 			if (pkt->mask & RXE_IETH_MASK) {
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 7e11bb8651b641..fd035641cf41ce 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -449,7 +449,7 @@ struct ib_uverbs_wc {
 	__u32 vendor_err;
 	__u32 byte_len;
 	union {
-		__u32 imm_data;
+		__be32 imm_data;
 		__u32 invalidate_rkey;
 	} ex;
 	__u32 qp_num;
@@ -765,7 +765,7 @@ struct ib_uverbs_send_wr {
 	__u32 opcode;
 	__u32 send_flags;
 	union {
-		__u32 imm_data;
+		__be32 imm_data;
 		__u32 invalidate_rkey;
 	} ex;
 	union {
-- 
2.15.1

--
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 2/3] RDMA/hns: Fix endian problems around imm_data and rkey

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This matches the changes made recently to the userspace hns
driver when it was made sparse clean.

See rdma-core commit bffd380cfe56 ("libhns: Make the provider sparse
clean")

wc->imm_data is not used in the kernel so this change has no practical
impact.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 7 ++++---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 6 +++---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 5 ++++-
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 939355ede14ab7..833a305085efe1 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -2311,15 +2311,16 @@ static int hns_roce_v1_poll_one(struct hns_roce_cq *hr_cq,
 		case HNS_ROCE_OPCODE_RDMA_WITH_IMM_RECEIVE:
 			wc->opcode = IB_WC_RECV_RDMA_WITH_IMM;
 			wc->wc_flags = IB_WC_WITH_IMM;
-			wc->ex.imm_data = le32_to_cpu(cqe->immediate_data);
+			wc->ex.imm_data =
+				cpu_to_be32(le32_to_cpu(cqe->immediate_data));
 			break;
 		case HNS_ROCE_OPCODE_SEND_DATA_RECEIVE:
 			if (roce_get_bit(cqe->cqe_byte_4,
 					 CQE_BYTE_4_IMM_INDICATOR_S)) {
 				wc->opcode = IB_WC_RECV;
 				wc->wc_flags = IB_WC_WITH_IMM;
-				wc->ex.imm_data = le32_to_cpu(
-						  cqe->immediate_data);
+				wc->ex.imm_data = cpu_to_be32(
+					le32_to_cpu(cqe->immediate_data));
 			} else {
 				wc->opcode = IB_WC_RECV;
 				wc->wc_flags = 0;
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 2ca35e341d0912..34f13c3cb088c7 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -1709,7 +1709,7 @@ static int hns_roce_v2_poll_one(struct hns_roce_cq *hr_cq,
 		case HNS_ROCE_V2_OPCODE_RDMA_WRITE_IMM:
 			wc->opcode = IB_WC_RECV_RDMA_WITH_IMM;
 			wc->wc_flags = IB_WC_WITH_IMM;
-			wc->ex.imm_data = le32_to_cpu(cqe->rkey_immtdata);
+			wc->ex.imm_data = cqe->immtdata;
 			break;
 		case HNS_ROCE_V2_OPCODE_SEND:
 			wc->opcode = IB_WC_RECV;
@@ -1718,12 +1718,12 @@ static int hns_roce_v2_poll_one(struct hns_roce_cq *hr_cq,
 		case HNS_ROCE_V2_OPCODE_SEND_WITH_IMM:
 			wc->opcode = IB_WC_RECV;
 			wc->wc_flags = IB_WC_WITH_IMM;
-			wc->ex.imm_data = le32_to_cpu(cqe->rkey_immtdata);
+			wc->ex.imm_data = cqe->immtdata;
 			break;
 		case HNS_ROCE_V2_OPCODE_SEND_WITH_INV:
 			wc->opcode = IB_WC_RECV;
 			wc->wc_flags = IB_WC_WITH_INVALIDATE;
-			wc->ex.invalidate_rkey = cqe->rkey_immtdata;
+			wc->ex.invalidate_rkey = le32_to_cpu(cqe->rkey);
 			break;
 		default:
 			wc->status = IB_WC_GENERAL_ERR;
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
index 463edab9b7193f..cbf61ad2539c78 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
@@ -762,7 +762,10 @@ struct hns_roce_v2_qp_context {
 
 struct hns_roce_v2_cqe {
 	u32	byte_4;
-	u32	rkey_immtdata;
+	union {
+		__le32 rkey;
+		__be32 immtdata;
+	};
 	u32	byte_12;
 	u32	byte_16;
 	u32	byte_cnt;
-- 
2.15.1

--
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 3/3] RDMA/qedr: Fix endian problems around imm_data

[Jason Gunthorpe]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The double swap matches what user space rdma-core does to imm_data.

wc->imm_data is not used in the kernel so this change has no practical
impact.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/qedr/verbs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 3b9c89848d6698..5551120ac6ea52 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -3039,7 +3039,7 @@ static int __qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 		swqe->wqe_size = 2;
 		swqe2 = qed_chain_produce(&qp->sq.pbl);
 
-		swqe->inv_key_or_imm_data = cpu_to_le32(wr->ex.imm_data);
+		swqe->inv_key_or_imm_data = cpu_to_le32(be32_to_cpu(wr->ex.imm_data));
 		length = qedr_prepare_sq_send_data(dev, qp, swqe, swqe2,
 						   wr, bad_wr);
 		swqe->length = cpu_to_le32(length);
@@ -3590,7 +3590,7 @@ static inline int qedr_set_ok_cqe_resp_wc(struct rdma_cqe_responder *resp,
 	wc->byte_len = le32_to_cpu(resp->length);
 
 	if (resp->flags & QEDR_RESP_IMM) {
-		wc->ex.imm_data = le32_to_cpu(resp->imm_data_or_inv_r_Key);
+		wc->ex.imm_data = cpu_to_be32(le32_to_cpu(resp->imm_data_or_inv_r_Key));
 		wc->wc_flags |= IB_WC_WITH_IMM;
 
 		if (resp->flags & QEDR_RESP_RDMA)
-- 
2.15.1

--
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 2/3] RDMA/hns: Fix endian problems around imm_data and rkey

[oulijun]

在 2018/1/12 5:43, Jason Gunthorpe 写道:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> This matches the changes made recently to the userspace hns
> driver when it was made sparse clean.
> 
> See rdma-core commit bffd380cfe56 ("libhns: Make the provider sparse
> clean")
> 
> wc->imm_data is not used in the kernel so this change has no practical
> impact.
> 
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 7 ++++---
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 6 +++---
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 5 ++++-
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 939355ede14ab7..833a305085efe1 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -2311,15 +2311,16 @@ static int hns_roce_v1_poll_one(struct hns_roce_cq *hr_cq,
>  		case HNS_ROCE_OPCODE_RDMA_WITH_IMM_RECEIVE:
>  			wc->opcode = IB_WC_RECV_RDMA_WITH_IMM;
>  			wc->wc_flags = IB_WC_WITH_IMM;
> -			wc->ex.imm_data = le32_to_cpu(cqe->immediate_data);
> +			wc->ex.imm_data =
> +				cpu_to_be32(le32_to_cpu(cqe->immediate_data));
>  			break;
>  		case HNS_ROCE_OPCODE_SEND_DATA_RECEIVE:
>  			if (roce_get_bit(cqe->cqe_byte_4,
>  					 CQE_BYTE_4_IMM_INDICATOR_S)) {
>  				wc->opcode = IB_WC_RECV;
>  				wc->wc_flags = IB_WC_WITH_IMM;
> -				wc->ex.imm_data = le32_to_cpu(
> -						  cqe->immediate_data);
> +				wc->ex.imm_data = cpu_to_be32(
> +					le32_to_cpu(cqe->immediate_data));
>  			} else {
>  				wc->opcode = IB_WC_RECV;
>  				wc->wc_flags = 0;
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> index 2ca35e341d0912..34f13c3cb088c7 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> @@ -1709,7 +1709,7 @@ static int hns_roce_v2_poll_one(struct hns_roce_cq *hr_cq,
>  		case HNS_ROCE_V2_OPCODE_RDMA_WRITE_IMM:
>  			wc->opcode = IB_WC_RECV_RDMA_WITH_IMM;
>  			wc->wc_flags = IB_WC_WITH_IMM;
> -			wc->ex.imm_data = le32_to_cpu(cqe->rkey_immtdata);
> +			wc->ex.imm_data = cqe->immtdata;
>  			break;
>  		case HNS_ROCE_V2_OPCODE_SEND:
>  			wc->opcode = IB_WC_RECV;
> @@ -1718,12 +1718,12 @@ static int hns_roce_v2_poll_one(struct hns_roce_cq *hr_cq,
>  		case HNS_ROCE_V2_OPCODE_SEND_WITH_IMM:
>  			wc->opcode = IB_WC_RECV;
>  			wc->wc_flags = IB_WC_WITH_IMM;
> -			wc->ex.imm_data = le32_to_cpu(cqe->rkey_immtdata);
> +			wc->ex.imm_data = cqe->immtdata;
>  			break;
>  		case HNS_ROCE_V2_OPCODE_SEND_WITH_INV:
>  			wc->opcode = IB_WC_RECV;
>  			wc->wc_flags = IB_WC_WITH_INVALIDATE;
> -			wc->ex.invalidate_rkey = cqe->rkey_immtdata;
> +			wc->ex.invalidate_rkey = le32_to_cpu(cqe->rkey);
>  			break;
>  		default:
>  			wc->status = IB_WC_GENERAL_ERR;
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
> index 463edab9b7193f..cbf61ad2539c78 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
> @@ -762,7 +762,10 @@ struct hns_roce_v2_qp_context {
>  
>  struct hns_roce_v2_cqe {
>  	u32	byte_4;
> -	u32	rkey_immtdata;
> +	union {
> +		__le32 rkey;
> +		__be32 immtdata;
> +	};
>  	u32	byte_12;
>  	u32	byte_16;
>  	u32	byte_cnt;
> 
 I am awared the question with endian format in kernel when fix the code in user mode.
And I plan to fixed it systemic in next future refer to the patch bffd380cfe56 .

Thanks. It is good to me.

Lijun Ou

--
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 3/3] RDMA/qedr: Fix endian problems around imm_data

[Kalderon, Michal]

> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Thursday, January 11, 2018 11:43 PM
> 
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The double swap matches what user space rdma-core does to imm_data.
> 
> wc->imm_data is not used in the kernel so this change has no practical
> impact.
> 
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/qedr/verbs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/qedr/verbs.c
> b/drivers/infiniband/hw/qedr/verbs.c
> index 3b9c89848d6698..5551120ac6ea52 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -3039,7 +3039,7 @@ static int __qedr_post_send(struct ib_qp *ibqp,
> struct ib_send_wr *wr,
>  		swqe->wqe_size = 2;
>  		swqe2 = qed_chain_produce(&qp->sq.pbl);
> 
> -		swqe->inv_key_or_imm_data = cpu_to_le32(wr-
> >ex.imm_data);
> +		swqe->inv_key_or_imm_data =
> cpu_to_le32(be32_to_cpu(wr->ex.imm_data));
>  		length = qedr_prepare_sq_send_data(dev, qp, swqe,
> swqe2,
>  						   wr, bad_wr);
>  		swqe->length = cpu_to_le32(length);
> @@ -3590,7 +3590,7 @@ static inline int qedr_set_ok_cqe_resp_wc(struct
> rdma_cqe_responder *resp,
>  	wc->byte_len = le32_to_cpu(resp->length);
> 
>  	if (resp->flags & QEDR_RESP_IMM) {
> -		wc->ex.imm_data = le32_to_cpu(resp-
> >imm_data_or_inv_r_Key);
> +		wc->ex.imm_data = cpu_to_be32(le32_to_cpu(resp-
> >imm_data_or_inv_r_Key));
>  		wc->wc_flags |= IB_WC_WITH_IMM;
> 
>  		if (resp->flags & QEDR_RESP_RDMA)
> --
> 2.15.1
Acked-by: Michal Kalderon <michal.kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Thanks Jason! 
--
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 0/3] Annotate imm_data as be32 in the uapi header

[Jason Gunthorpe]

On Thu, Jan 11, 2018 at 02:43:04PM -0700, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> This matches what the user space version of this struct has had for some time
> and it removed several __force casts from the tree.
> 
> hns and qedr are updated to match what their userspace providers are doing.
> 
> Jason Gunthorpe (3):
>   RDMA: Mark imm_data as be32 in the verbs uapi header
>   RDMA/hns: Fix endian problems around imm_data and rkey
>   RDMA/qedr: Fix endian problems around imm_data

With driver maintainer acks I applied all of these to for-next

Jason
--
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