Re: [PATCH for-next 1/4] RDMA/hns: Fix the endian problem for hns
From: Doug Ledford <hidden>
Date: 2018-01-31 15:44:45
On Tue, 2018-01-30 at 20:20 +0800, Lijun Ou wrote:
quoted hunk
The hip06 and hip08 run on a little endian ARM, it needs to revise the annotations to indicate that the HW uses little endian data in the various DMA buffers, and flow the necessary swaps throughout. The imm_data use big endian mode. The cpu_to_le32/le32_to_cpu swaps are no-op for this, which makes the only substantive change the handling of imm_data which is now mandatory swapped. This also keep match with the userspace hns driver and resolve the warning by sparse. Signed-off-by: Lijun Ou <redacted> --- drivers/infiniband/hw/hns/hns_roce_common.h | 6 +- drivers/infiniband/hw/hns/hns_roce_device.h | 2 +- drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 57 ++++-- drivers/infiniband/hw/hns/hns_roce_hw_v1.h | 258 ++++++++++++------------- drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 51 +++-- drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 283 ++++++++++++++-------------- drivers/infiniband/hw/hns/hns_roce_main.c | 2 +- drivers/infiniband/hw/hns/hns_roce_qp.c | 18 +- 8 files changed, 357 insertions(+), 320 deletions(-)diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h b/drivers/infiniband/hw/hns/hns_roce_common.h index dd67faf..319cb74 100644 --- a/drivers/infiniband/hw/hns/hns_roce_common.h +++ b/drivers/infiniband/hw/hns/hns_roce_common.h@@ -43,15 +43,15 @@ __raw_writel((__force u32)cpu_to_le32(value), (addr)) #define roce_get_field(origin, mask, shift) \ - (((origin) & (mask)) >> (shift)) + (((le32_to_cpu(origin)) & (mask)) >> (shift)) #define roce_get_bit(origin, shift) \ roce_get_field((origin), (1ul << (shift)), (shift)) #define roce_set_field(origin, mask, shift, val) \ do { \ - (origin) &= (~(mask)); \ - (origin) |= (((u32)(val) << (shift)) & (mask)); \ + (origin) &= ~cpu_to_le32(mask); \ + (origin) |= cpu_to_le32(((u32)(val) << (shift)) & (mask)); \ } while (0) #define roce_set_bit(origin, shift, val) \diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h index 42c3b5a..2503d7f 100644 --- a/drivers/infiniband/hw/hns/hns_roce_device.h +++ b/drivers/infiniband/hw/hns/hns_roce_device.h@@ -466,7 +466,7 @@ struct hns_roce_qp { struct ib_qp ibqp; struct hns_roce_buf hr_buf; struct hns_roce_wq rq; - __le64 doorbell_qpn; + u32 doorbell_qpn;
Why the change in size here? Did you mean to go from 64bits down to 32bits?
} else if (ibqp->qp_type == IB_QPT_RC) {
ctrl = wqe;
memset(ctrl, 0, sizeof(struct hns_roce_wqe_ctrl_seg));
for (i = 0; i < wr->num_sge; i++)
- ctrl->msg_length += wr->sg_list[i].length;
+ ctrl->msg_length =
+ cpu_to_le32(le32_to_cpu(ctrl->msg_length) +
+ wr->sg_list[i].length);
Minor nit:
Doing le32_to_cpu and cpu_to_le32 over and over again in a loop is
horribly inefficient. It would be much better IMO if you had a local
variable to use for the length, used that in the loop, and then only at
the end of the loop do a single cpu_to_le32 of the local variable to
store in msg_length. Same comment applies to the other spot in this
patch that does the same loop.
--
Doug Ledford [off-list ref]
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD Attachments
- signature.asc [application/pgp-signature] 833 bytes