Add limit checking on rd_atomic and dest_rd_atomic attributes:
especially for max_dest_rd_atomic, a value that is larger than HCA
capability can cause RDB overflow and corruption of another QP.
Signed-off-by: Jack Morgenstein <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_qp.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
94361cf74a6fca1973d2fed5338d5fb4bcd902fa
diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c
index 7450550..c5c3d0e 100644
--- a/drivers/infiniband/hw/mthca/mthca_qp.c
+++ b/drivers/infiniband/hw/mthca/mthca_qp.c
@@ -591,6 +591,20 @@ int mthca_modify_qp(struct ib_qp *ibqp,
return -EINVAL;
}
+ if (attr_mask & IB_QP_MAX_QP_RD_ATOMIC &&
+ attr->max_rd_atomic > dev->limits.max_qp_init_rdma) {
+ mthca_dbg(dev, "Max rdma_atomic as initiator %u too large (max is %d)\n",
+ attr->max_rd_atomic, dev->limits.max_qp_init_rdma);
+ return -EINVAL;
+ }
+
+ if (attr_mask & IB_QP_MAX_DEST_RD_ATOMIC &&
+ attr->max_dest_rd_atomic > 1 << dev->qp_table.rdb_shift) {
+ mthca_dbg(dev, "Max rdma_atomic as responder %u too large (max %d)\n",
+ attr->max_dest_rd_atomic, 1 << dev->qp_table.rdb_shift);
+ return -EINVAL;
+ }
+
mailbox = mthca_alloc_mailbox(dev, GFP_KERNEL);
if (IS_ERR(mailbox))
return PTR_ERR(mailbox);
--
0.99.9n
Fix thinko in rd_atomic calculation: ffs(x) - 1 does not find the next
power of 2 -- it should be fls(x - 1).
Signed-off-by: Jack Morgenstein <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_qp.c | 17 ++++++-----------
1 files changed, 6 insertions(+), 11 deletions(-)
6aa2e4e8063114bd7cea8616dd5848d3c64b4c36
diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c
index c5c3d0e..84056a8 100644
--- a/drivers/infiniband/hw/mthca/mthca_qp.c
+++ b/drivers/infiniband/hw/mthca/mthca_qp.c
@@ -728,9 +728,9 @@ int mthca_modify_qp(struct ib_qp *ibqp,
}
if (attr_mask & IB_QP_MAX_QP_RD_ATOMIC) {
- qp_context->params1 |= cpu_to_be32(min(attr->max_rd_atomic ?
- ffs(attr->max_rd_atomic) - 1 : 0,
- 7) << 21);
+ if (attr->max_rd_atomic)
+ qp_context->params1 |=
+ cpu_to_be32(fls(attr->max_rd_atomic - 1) << 21);
qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_SRA_MAX);
}
@@ -769,8 +769,6 @@ int mthca_modify_qp(struct ib_qp *ibqp,
}
if (attr_mask & IB_QP_MAX_DEST_RD_ATOMIC) {
- u8 rra_max;
-
if (qp->resp_depth && !attr->max_dest_rd_atomic) {
/*
* Lowering our responder resources to zero.
@@ -798,13 +796,10 @@ int mthca_modify_qp(struct ib_qp *ibqp,
MTHCA_QP_OPTPAR_RAE);
}
- for (rra_max = 0;
- 1 << rra_max < attr->max_dest_rd_atomic &&
- rra_max < dev->qp_table.rdb_shift;
- ++rra_max)
- ; /* nothing */
+ if (attr->max_dest_rd_atomic)
+ qp_context->params2 |=
+ cpu_to_be32(fls(attr->max_dest_rd_atomic - 1) << 21);
- qp_context->params2 |= cpu_to_be32(rra_max << 21);
qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RRA_MAX);
qp->resp_depth = attr->max_dest_rd_atomic;
--
0.99.9n
Only change the driver's copy of the QP attributes in modify QP after
checking the modify QP command completed successfully.
Signed-off-by: Jack Morgenstein <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_qp.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
44b5b0303327cfb23f135b95b2fe5436c81ed27c
diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c
index 84056a8..3543299 100644
--- a/drivers/infiniband/hw/mthca/mthca_qp.c
+++ b/drivers/infiniband/hw/mthca/mthca_qp.c
@@ -764,8 +764,6 @@ int mthca_modify_qp(struct ib_qp *ibqp,
qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RWE |
MTHCA_QP_OPTPAR_RRE |
MTHCA_QP_OPTPAR_RAE);
-
- qp->atomic_rd_en = attr->qp_access_flags;
}
if (attr_mask & IB_QP_MAX_DEST_RD_ATOMIC) {
@@ -801,8 +799,6 @@ int mthca_modify_qp(struct ib_qp *ibqp,
cpu_to_be32(fls(attr->max_dest_rd_atomic - 1) << 21);
qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RRA_MAX);
-
- qp->resp_depth = attr->max_dest_rd_atomic;
}
qp_context->params2 |= cpu_to_be32(MTHCA_QP_BIT_RSC);
@@ -844,8 +840,13 @@ int mthca_modify_qp(struct ib_qp *ibqp,
err = -EINVAL;
}
- if (!err)
+ if (!err) {
qp->state = new_state;
+ if (attr_mask & IB_QP_ACCESS_FLAGS)
+ qp->atomic_rd_en = attr->qp_access_flags;
+ if (attr_mask & IB_QP_MAX_DEST_RD_ATOMIC)
+ qp->resp_depth = attr->max_dest_rd_atomic;
+ }
mthca_free_mailbox(dev, mailbox);
--
0.99.9n
When cleaning up a CQ for a QP attached to SRQ, need to free an SRQ
WQE only if the CQE is a receive completion.
Signed-off-by: Jack Morgenstein <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_cq.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
576d2e4e40315e8140c04be99cd057720d8a3817
diff --git a/drivers/infiniband/hw/mthca/mthca_cq.c b/drivers/infiniband/hw/mthca/mthca_cq.c
index 4a8adce..fcef8dc 100644
--- a/drivers/infiniband/hw/mthca/mthca_cq.c
+++ b/drivers/infiniband/hw/mthca/mthca_cq.c
@@ -253,6 +253,15 @@ void mthca_cq_event(struct mthca_dev *de
wake_up(&cq->wait);
}
+static inline int is_recv_cqe(struct mthca_cqe *cqe)
+{
+ if ((cqe->opcode & MTHCA_ERROR_CQE_OPCODE_MASK) ==
+ MTHCA_ERROR_CQE_OPCODE_MASK)
+ return !(cqe->opcode & 0x01);
+ else
+ return !(cqe->is_send & 0x80);
+}
+
void mthca_cq_clean(struct mthca_dev *dev, u32 cqn, u32 qpn,
struct mthca_srq *srq)
{
@@ -296,7 +305,7 @@ void mthca_cq_clean(struct mthca_dev *de
while ((int) --prod_index - (int) cq->cons_index >= 0) {
cqe = get_cqe(cq, prod_index & cq->ibcq.cqe);
if (cqe->my_qpn == cpu_to_be32(qpn)) {
- if (srq)
+ if (srq && is_recv_cqe(cqe))
mthca_free_srq_wqe(srq, be32_to_cpu(cqe->wqe));
++nfreed;
} else if (nfreed)
--
0.99.9n
break only escapes from the innermost loop, and we want to escape both
loops and return an answer. Noticed by Ishai Rabinovitch.
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_memfree.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
6c7d2a75b512c64c910b69adf32dbaddb461910b
diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
index 5798ed0..9fb985a 100644
--- a/drivers/infiniband/hw/mthca/mthca_memfree.c
+++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
@@ -233,7 +233,7 @@ void *mthca_table_find(struct mthca_icm_
for (i = 0; i < chunk->npages; ++i) {
if (chunk->mem[i].length >= offset) {
page = chunk->mem[i].page;
- break;
+ goto out;
}
offset -= chunk->mem[i].length;
}
--
0.99.9n
This patch corrects some corner cases in managing the RAE/RRE bits in
the mthca qp context. These bits need to be zero if the user requests
max_dest_rd_atomic of zero. The bits need to be restored to the value
implied by the qp access flags attribute in a previous (or the
current) modify-qp command if the dest_rd_atomic variable is changed
to non-zero.
In the current implementation, the following scenario will not work:
RESET-to-INIT set QP access flags to all disabled (zeroes)
INIT-to-RTR set max_dest_rd_atomic=10, AND
set qp_access_flags = IB_ACCESS_REMOTE_READ | IB_ACCESS_REMOTE_ATOMIC
The current code will incorrectly take the access-flags value set in
the RESET-to-INIT transition.
We can simplify, and correct, this IB_QP_ACCESS_FLAGS handling: it is
always safe to set qp access flags in the firmware command if either
of IB_QP_MAX_DEST_RD_ATOMIC or IB_QP_ACCESS_FLAGS is set, so let's
just set it to the correct value, always.
Signed-off-by: Jack Morgenstein <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_qp.c | 87 ++++++++++++++------------------
1 files changed, 37 insertions(+), 50 deletions(-)
d1646f86a2a05a956adbb163c81a81bd621f055e
diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c
index 3543299..e826c9f 100644
--- a/drivers/infiniband/hw/mthca/mthca_qp.c
+++ b/drivers/infiniband/hw/mthca/mthca_qp.c
@@ -522,6 +522,36 @@ static void init_port(struct mthca_dev *
mthca_warn(dev, "INIT_IB returned status %02x.\n", status);
}
+static __be32 get_hw_access_flags(struct mthca_qp *qp, struct ib_qp_attr *attr,
+ int attr_mask)
+{
+ u8 dest_rd_atomic;
+ u32 access_flags;
+ u32 hw_access_flags = 0;
+
+ if (attr_mask & IB_QP_MAX_DEST_RD_ATOMIC)
+ dest_rd_atomic = attr->max_dest_rd_atomic;
+ else
+ dest_rd_atomic = qp->resp_depth;
+
+ if (attr_mask & IB_QP_ACCESS_FLAGS)
+ access_flags = attr->qp_access_flags;
+ else
+ access_flags = qp->atomic_rd_en;
+
+ if (!dest_rd_atomic)
+ access_flags &= IB_ACCESS_REMOTE_WRITE;
+
+ if (access_flags & IB_ACCESS_REMOTE_READ)
+ hw_access_flags |= MTHCA_QP_BIT_RRE;
+ if (access_flags & IB_ACCESS_REMOTE_ATOMIC)
+ hw_access_flags |= MTHCA_QP_BIT_RAE;
+ if (access_flags & IB_ACCESS_REMOTE_WRITE)
+ hw_access_flags |= MTHCA_QP_BIT_RWE;
+
+ return cpu_to_be32(hw_access_flags);
+}
+
int mthca_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr, int attr_mask)
{
struct mthca_dev *dev = to_mdev(ibqp->device);
@@ -743,57 +773,7 @@ int mthca_modify_qp(struct ib_qp *ibqp,
qp_context->snd_db_index = cpu_to_be32(qp->sq.db_index);
}
- if (attr_mask & IB_QP_ACCESS_FLAGS) {
- qp_context->params2 |=
- cpu_to_be32(attr->qp_access_flags & IB_ACCESS_REMOTE_WRITE ?
- MTHCA_QP_BIT_RWE : 0);
-
- /*
- * Only enable RDMA reads and atomics if we have
- * responder resources set to a non-zero value.
- */
- if (qp->resp_depth) {
- qp_context->params2 |=
- cpu_to_be32(attr->qp_access_flags & IB_ACCESS_REMOTE_READ ?
- MTHCA_QP_BIT_RRE : 0);
- qp_context->params2 |=
- cpu_to_be32(attr->qp_access_flags & IB_ACCESS_REMOTE_ATOMIC ?
- MTHCA_QP_BIT_RAE : 0);
- }
-
- qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RWE |
- MTHCA_QP_OPTPAR_RRE |
- MTHCA_QP_OPTPAR_RAE);
- }
-
if (attr_mask & IB_QP_MAX_DEST_RD_ATOMIC) {
- if (qp->resp_depth && !attr->max_dest_rd_atomic) {
- /*
- * Lowering our responder resources to zero.
- * Turn off reads RDMA and atomics as responder.
- * (RRE/RAE in params2 already zero)
- */
- qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RRE |
- MTHCA_QP_OPTPAR_RAE);
- }
-
- if (!qp->resp_depth && attr->max_dest_rd_atomic) {
- /*
- * Increasing our responder resources from
- * zero. Turn on RDMA reads and atomics as
- * appropriate.
- */
- qp_context->params2 |=
- cpu_to_be32(qp->atomic_rd_en & IB_ACCESS_REMOTE_READ ?
- MTHCA_QP_BIT_RRE : 0);
- qp_context->params2 |=
- cpu_to_be32(qp->atomic_rd_en & IB_ACCESS_REMOTE_ATOMIC ?
- MTHCA_QP_BIT_RAE : 0);
-
- qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RRE |
- MTHCA_QP_OPTPAR_RAE);
- }
-
if (attr->max_dest_rd_atomic)
qp_context->params2 |=
cpu_to_be32(fls(attr->max_dest_rd_atomic - 1) << 21);
@@ -801,6 +781,13 @@ int mthca_modify_qp(struct ib_qp *ibqp,
qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RRA_MAX);
}
+ if (attr_mask & (IB_QP_ACCESS_FLAGS | IB_QP_MAX_DEST_RD_ATOMIC)) {
+ qp_context->params2 |= get_hw_access_flags(qp, attr, attr_mask);
+ qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_RWE |
+ MTHCA_QP_OPTPAR_RRE |
+ MTHCA_QP_OPTPAR_RAE);
+ }
+
qp_context->params2 |= cpu_to_be32(MTHCA_QP_BIT_RSC);
if (ibqp->srq)
--
0.99.9n
sae and sre bits should only be set when setting sra_max. Further, in
the old code, if the caller specifies max_rd_atomic = 0, the sre and
sae bits are still set, with the result that the QP ends up with
max_rd_atomic = 1 in effect.
Signed-off-by: Jack Morgenstein <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_qp.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
c4342d8a4d95e18b957b898dbf5bfce28fca2780
diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c
index e826c9f..d786ef4 100644
--- a/drivers/infiniband/hw/mthca/mthca_qp.c
+++ b/drivers/infiniband/hw/mthca/mthca_qp.c
@@ -747,9 +747,7 @@ int mthca_modify_qp(struct ib_qp *ibqp,
qp_context->wqe_lkey = cpu_to_be32(qp->mr.ibmr.lkey);
qp_context->params1 = cpu_to_be32((MTHCA_ACK_REQ_FREQ << 28) |
(MTHCA_FLIGHT_LIMIT << 24) |
- MTHCA_QP_BIT_SRE |
- MTHCA_QP_BIT_SWE |
- MTHCA_QP_BIT_SAE);
+ MTHCA_QP_BIT_SWE);
if (qp->sq_policy == IB_SIGNAL_ALL_WR)
qp_context->params1 |= cpu_to_be32(MTHCA_QP_BIT_SSC);
if (attr_mask & IB_QP_RETRY_CNT) {
@@ -758,9 +756,13 @@ int mthca_modify_qp(struct ib_qp *ibqp,
}
if (attr_mask & IB_QP_MAX_QP_RD_ATOMIC) {
- if (attr->max_rd_atomic)
+ if (attr->max_rd_atomic) {
+ qp_context->params1 |=
+ cpu_to_be32(MTHCA_QP_BIT_SRE |
+ MTHCA_QP_BIT_SAE);
qp_context->params1 |=
cpu_to_be32(fls(attr->max_rd_atomic - 1) << 21);
+ }
qp_param->opt_param_mask |= cpu_to_be32(MTHCA_QP_OPTPAR_SRA_MAX);
}
--
0.99.9n
Roland Dreier <[email protected]> wrote:
>
> Fix thinko in rd_atomic calculation: ffs(x) - 1 does not find the next
> power of 2 -- it should be fls(x - 1).
Please use round_up_pow_of_two().
Quoting r. Andrew Morton <[email protected]>:
> Subject: Re: [git patch review 2/7] IB/mthca: correct log2 calculation
>
> Roland Dreier <[email protected]> wrote:
> >
> > Fix thinko in rd_atomic calculation: ffs(x) - 1 does not find the next
> > power of 2 -- it should be fls(x - 1).
>
> Please use round_up_pow_of_two().
Yes, but we want the bit number. roundup_pow_of_two does a shift.
--
MST