IB/ehca: Reject send WRs only for RESET, INIT and RTR state

PATCH Applied Joachim Fenkes 5 participants 22 messages Jun 6, 2008 – Sep 1, 2009 InfiniBand

Share & tools

Joachim Fenkes

author

2008-06-06 16:35 UTC #0 lore raw

Signed-off-by Joachim Fenkes <[email protected]>

---
 drivers/infiniband/hw/ehca/ehca_reqs.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

drivers/infiniband/hw/ehca/ehca_reqs.c +4 -2

index bbe0436..f093b00 100644
@@ -421,8 +421,10 @@ int ehca_post_send(struct ib_qp *qp,
 	int ret = 0;
 	unsigned long flags;
 
-	if (unlikely(my_qp->state != IB_QPS_RTS)) {
-		ehca_err(qp->device, "QP not in RTS state  qpn=%x", qp->qp_num);
+	/* Reject WR if QP is in RESET, INIT or RTR state */
+	if (unlikely(my_qp->state < IB_QPS_RTS)) {
+		ehca_err(qp->device, "Invalid QP state  qp_state=%d qpn=%x",
+			 my_qp->state, qp->qp_num);
 		return -EINVAL;
 	}

Signature · 1 line

1.5.5

Roland Dreier

Applied

2008-06-06 18:22 UTC #1 lore raw

thanks, applied.

Dotan Barak

2008-06-06 19:15 UTC #2 lore raw

In reply to Joachim Fenkes

Hi.

I checked the code in the ehca driver and noticed that
post RR to a QP is being accepted in any state (including the RESET state).

thanks
Dotan

Quoted 23 lines · Joachim Fenkes

Signed-off-by Joachim Fenkes <[email protected]>

---
 drivers/infiniband/hw/ehca/ehca_reqs.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

drivers/infiniband/hw/ehca/ehca_reqs.c +4 -2

index bbe0436..f093b00 100644
@@ -421,8 +421,10 @@ int ehca_post_send(struct ib_qp *qp,
 	int ret = 0;
 	unsigned long flags;
 
-	if (unlikely(my_qp->state != IB_QPS_RTS)) {
-		ehca_err(qp->device, "QP not in RTS state  qpn=%x", qp->qp_num);
+	/* Reject WR if QP is in RESET, INIT or RTR state */
+	if (unlikely(my_qp->state < IB_QPS_RTS)) {
+		ehca_err(qp->device, "Invalid QP state  qp_state=%d qpn=%x",
+			 my_qp->state, qp->qp_num);
 		return -EINVAL;
 	}

Joachim Fenkes

author

2008-06-09 11:25 UTC #3 lore raw

PATCH 1/1 IB/ehca: Reject recv WRs if QP is in RESET state

Signed-off-by Joachim Fenkes <[email protected]>

---

Quoted 2 lines · Dotan Barak

I checked the code in the ehca driver and noticed that
post RR to a QP is being accepted in any state (including the RESET state).

You're right, this is only consistent -- thanks for pointing it out!

Regards,
Joachim

 drivers/infiniband/hw/ehca/ehca_reqs.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

drivers/infiniband/hw/ehca/ehca_reqs.c +10 -2

index f093b00..ad197f4 100644
@@ -544,8 +544,16 @@ int ehca_post_recv(struct ib_qp *qp,
 		   struct ib_recv_wr *recv_wr,
 		   struct ib_recv_wr **bad_recv_wr)
 {
-	return internal_post_recv(container_of(qp, struct ehca_qp, ib_qp),
-				  qp->device, recv_wr, bad_recv_wr);
+	struct ehca_qp *my_qp = container_of(qp, struct ehca_qp, ib_qp);
+
+	/* Reject WR if QP is in RESET state */
+	if (unlikely(my_qp->state == IB_QPS_RESET)) {
+		ehca_err(qp->device, "Invalid QP state  qp_state=%d qpn=%x",
+			 my_qp->state, qp->qp_num);
+		return -EINVAL;
+	}
+
+	return internal_post_recv(my_qp, qp->device, recv_wr, bad_recv_wr);
 }
 
 int ehca_post_srq_recv(struct ib_srq *srq,

Signature · 1 line

1.5.5

Roland Dreier

Applied

2008-06-20 17:26 UTC #4 lore raw

Re: [PATCH] IB/ehca: Reject recv WRs if QP is in RESET state

thanks, applied.

Joachim Fenkes

author

2008-07-01 14:14 UTC #5 lore raw

PATCH 1/1 IB/ehca: Make device table externally visible

In reply to Dotan Barak

This gives ehca an autogenerated modalias and therefore enables automatic loading.

Signed-off-by Joachim Fenkes <[email protected]>

---
 drivers/infiniband/hw/ehca/ehca_main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/infiniband/hw/ehca/ehca_main.c +1 -0

index 482103e..598844d 100644
@@ -923,6 +923,7 @@ static struct of_device_id ehca_device_table[] =
 	},
 	{},
 };
+MODULE_DEVICE_TABLE(of, ehca_device_table);
 
 static struct of_platform_driver ehca_driver = {
 	.name        = "ehca",

Signature · 1 line

1.5.5

Roland Dreier

Applied

2008-07-01 17:55 UTC #6 lore raw

Re: [PATCH] IB/ehca: Make device table externally visible

thanks, applied

Joachim Fenkes

author

2008-11-07 16:43 UTC #7 lore raw

PATCH 1/1 IB/ehca: Fix suppression of port activation events

In reply to Dotan Barak

A previous fix introduced a regression where port activation events were dropped unconditionally if port autodetection was not enabled. Fixed.

Signed-off-by Joachim Fenkes <[email protected]>

---

Roland -- this patch is made against your for-linus branch. Please review and apply if you think it's okay. Hope it's not too late for the next kernel.

Joachim

 drivers/infiniband/hw/ehca/ehca_irq.c |   45 +++++++++++++++++++-------------
 1 files changed, 27 insertions(+), 18 deletions(-)

drivers/infiniband/hw/ehca/ehca_irq.c +27 ~~-18~~

index 9e43459..757035e 100644
@@ -359,34 +359,43 @@ static void notify_port_conf_change(struct ehca_shca *shca, int port_num)
 	*old_attr = new_attr;
 }
 
+/* replay modify_qp for sqps -- return 0 if all is well, 1 if AQP1 destroyed */
+static int replay_modify_qp(struct ehca_sport *sport)
+{
+	int aqp1_destroyed;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sport->mod_sqp_lock, flags);
+
+	aqp1_destroyed = !sport->ibqp_sqp[IB_QPT_GSI];
+
+	if (sport->ibqp_sqp[IB_QPT_SMI])
+		ehca_recover_sqp(sport->ibqp_sqp[IB_QPT_SMI]);
+	if (!aqp1_destroyed)
+		ehca_recover_sqp(sport->ibqp_sqp[IB_QPT_GSI]);
+
+	spin_unlock_irqrestore(&sport->mod_sqp_lock, flags);
+
+	return aqp1_destroyed;
+}
+
 static void parse_ec(struct ehca_shca *shca, u64 eqe)
 {
 	u8 ec   = EHCA_BMASK_GET(NEQE_EVENT_CODE, eqe);
 	u8 port = EHCA_BMASK_GET(NEQE_PORT_NUMBER, eqe);
 	u8 spec_event;
 	struct ehca_sport *sport = &shca->sport[port - 1];
-	unsigned long flags;
 
 	switch (ec) {
 	case 0x30: /* port availability change */
 		if (EHCA_BMASK_GET(NEQE_PORT_AVAILABILITY, eqe)) {
-			/* only for autodetect mode important */
-			if (ehca_nr_ports >= 0)
-				break;
-
-			int suppress_event;
-			/* replay modify_qp for sqps */
-			spin_lock_irqsave(&sport->mod_sqp_lock, flags);
-			suppress_event = !sport->ibqp_sqp[IB_QPT_GSI];
-			if (sport->ibqp_sqp[IB_QPT_SMI])
-				ehca_recover_sqp(sport->ibqp_sqp[IB_QPT_SMI]);
-			if (!suppress_event)
-				ehca_recover_sqp(sport->ibqp_sqp[IB_QPT_GSI]);
-			spin_unlock_irqrestore(&sport->mod_sqp_lock, flags);
-
-			/* AQP1 was destroyed, ignore this event */
-			if (suppress_event)
-				break;
+			/* only replay modify_qp calls in autodetect mode;
+			 * if AQP1 was destroyed, the port is already down
+			 * again and we can drop the event.
+			 */
+			if (ehca_nr_ports < 0)
+				if (replay_modify_qp(sport))
+					break;
 
 			sport->port_state = IB_PORT_ACTIVE;
 			dispatch_port_event(shca, port, IB_EVENT_PORT_ACTIVE,

Signature · 1 line

1.5.5

Roland Dreier

2008-11-10 20:36 UTC #8 lore raw

Re: [PATCH] IB/ehca: Fix suppression of port activation events

Quoted 2 lines

A previous fix introduced a regression where port activation events were dropped unconditionally if port autodetection was not enabled. Fixed.

Is this a fix to "IB/ehca: Remove reference to special QP in case of port activation failure"? Because if so I can roll it into that patch, since Linus hasn't pulled it yet.

- R.

Joachim Fenkes

author

2008-11-11 09:04 UTC #9 lore raw

Re: [PATCH] IB/ehca: Fix suppression of port activation events

Roland Dreier <[email protected]> wrote on 10.11.2008 21:36:23:

Quoted 1 line

Quoted 1 line
A previous fix introduced a regression where port activation events

were

Quoted 5 lines

Quoted 1 line
dropped unconditionally if port autodetection was not enabled. Fixed.
Is this a fix to "IB/ehca: Remove reference to special QP in case of port activation failure"? Because if so I can roll it into that patch, since Linus hasn't pulled it yet.

Yes, that would be splendid, thank you!

Cheers,
Joachim

Joachim Fenkes

author

2008-11-21 15:37 UTC #10 lore raw

PATCH 1/1 IB/ehca: Fix lockdep failures for shca_list_lock

In reply to Dotan Barak

From: Michael Ellerman <[email protected]>

shca_list_lock is taken from softirq context in ehca_poll_eqs, so we need to lock IRQ safe elsewhere.

Signed-off-by Michael Ellerman <[email protected]>

Acked-by Joachim Fenkes <[email protected]>

---
 drivers/infiniband/hw/ehca/ehca_main.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

drivers/infiniband/hw/ehca/ehca_main.c +10 -7

index bb02a86..021c454 100644
@@ -717,6 +717,7 @@ static int __devinit ehca_probe(struct of_device *dev,
 	const u64 *handle;
 	struct ib_pd *ibpd;
 	int ret, i, eq_size;
+	u64 flags;
 
 	handle = of_get_property(dev->node, "ibm,hca-handle", NULL);
 	if (!handle) {

@@ -830,9 +831,9 @@ static int __devinit ehca_probe(struct of_device *dev,
 		ehca_err(&shca->ib_device,
 			 "Cannot create device attributes  ret=%d", ret);
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);
 	list_add(&shca->shca_list, &shca_list);
-	spin_unlock(&shca_list_lock);
+	spin_unlock_irqrestore(&shca_list_lock, flags);
 
 	return 0;

@@ -878,6 +879,7 @@ probe1:
 static int __devexit ehca_remove(struct of_device *dev)
 {
 	struct ehca_shca *shca = dev->dev.driver_data;
+	u64 flags;
 	int ret;
 
 	sysfs_remove_group(&dev->dev.kobj, &ehca_dev_attr_grp);

@@ -915,9 +917,9 @@ static int __devexit ehca_remove(struct of_device *dev)
 
 	ib_dealloc_device(&shca->ib_device);
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);
 	list_del(&shca->shca_list);
-	spin_unlock(&shca_list_lock);
+	spin_unlock_irqrestore(&shca_list_lock, flags);
 
 	return ret;
 }

@@ -975,6 +977,7 @@ static int ehca_mem_notifier(struct notifier_block *nb,
 			     unsigned long action, void *data)
 {
 	static unsigned long ehca_dmem_warn_time;
+	unsigned long flags;
 
 	switch (action) {
 	case MEM_CANCEL_OFFLINE:

@@ -985,12 +988,12 @@ static int ehca_mem_notifier(struct notifier_block *nb,
 	case MEM_GOING_ONLINE:
 	case MEM_GOING_OFFLINE:
 		/* only ok if no hca is attached to the lpar */
-		spin_lock(&shca_list_lock);
+		spin_lock_irqsave(&shca_list_lock, flags);
 		if (list_empty(&shca_list)) {
-			spin_unlock(&shca_list_lock);
+			spin_unlock_irqrestore(&shca_list_lock, flags);
 			return NOTIFY_OK;
 		} else {
-			spin_unlock(&shca_list_lock);
+			spin_unlock_irqrestore(&shca_list_lock, flags);
 			if (printk_timed_ratelimit(&ehca_dmem_warn_time,
 						   30 * 1000))
 				ehca_gen_err("DMEM operations are not allowed"

Signature · 1 line

1.5.5

Johannes Berg

2008-11-21 16:02 UTC #11 lore raw

Re: [PATCH] IB/ehca: Fix lockdep failures for shca_list_lock

drivers/infiniband/hw/ehca/ehca_main.c static int __devinit ehca_probe(struct of_device *dev, · line 834

Quoted 4 lines · Joachim Fenkes

drivers/infiniband/hw/ehca/ehca_main.c +2 -1

+	u64 flags;
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);

That's wrong and I think will give a warning on all machines where u64 != unsigned long. Might not particularly matter in this case.

Also, generally it seems wrong to say "fix lockdep failure" when the patch really fixes a bug that lockdep happened to find.

johannes

Joachim Fenkes

author

2008-11-21 16:18 UTC #12 lore raw

PATCH 1/1 IB/ehca: Fix locking for shca_list_lock

shca_list_lock is taken from softirq context in ehca_poll_eqs, so we need to lock IRQ safe elsewhere.

Signed-off-by Michael Ellerman <[email protected]>

Signed-off-by Joachim Fenkes <[email protected]>

---

Quoted 9 lines · Johannes Berg

Review
Quoted 4 lines · Joachim Fenkes
(quoted diff) +2 -1
+	u64 flags;
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);
That's wrong and I think will give a warning on all machines where u64 != unsigned long. Might not particularly matter in this case.

Doesn't matter for a ppc64 only driver, but you're right nonetheless. Thanks.

Quoted 2 lines

Also, generally it seems wrong to say "fix lockdep failure" when the patch really fixes a bug that lockdep happened to find.

Whatever -- changed.

Here's the updated patch.

Regards,
Joachim

 drivers/infiniband/hw/ehca/ehca_main.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

drivers/infiniband/hw/ehca/ehca_main.c +10 -7

index bb02a86..169aa1a 100644
@@ -717,6 +717,7 @@ static int __devinit ehca_probe(struct of_device *dev,
 	const u64 *handle;
 	struct ib_pd *ibpd;
 	int ret, i, eq_size;
+	unsigned long flags;
 
 	handle = of_get_property(dev->node, "ibm,hca-handle", NULL);
 	if (!handle) {

@@ -830,9 +831,9 @@ static int __devinit ehca_probe(struct of_device *dev,
 		ehca_err(&shca->ib_device,
 			 "Cannot create device attributes  ret=%d", ret);
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);
 	list_add(&shca->shca_list, &shca_list);
-	spin_unlock(&shca_list_lock);
+	spin_unlock_irqrestore(&shca_list_lock, flags);
 
 	return 0;

@@ -878,6 +879,7 @@ probe1:
 static int __devexit ehca_remove(struct of_device *dev)
 {
 	struct ehca_shca *shca = dev->dev.driver_data;
+	unsigned long flags;
 	int ret;
 
 	sysfs_remove_group(&dev->dev.kobj, &ehca_dev_attr_grp);

@@ -915,9 +917,9 @@ static int __devexit ehca_remove(struct of_device *dev)
 
 	ib_dealloc_device(&shca->ib_device);
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);
 	list_del(&shca->shca_list);
-	spin_unlock(&shca_list_lock);
+	spin_unlock_irqrestore(&shca_list_lock, flags);
 
 	return ret;
 }

@@ -975,6 +977,7 @@ static int ehca_mem_notifier(struct notifier_block *nb,
 			     unsigned long action, void *data)
 {
 	static unsigned long ehca_dmem_warn_time;
+	unsigned long flags;
 
 	switch (action) {
 	case MEM_CANCEL_OFFLINE:

@@ -985,12 +988,12 @@ static int ehca_mem_notifier(struct notifier_block *nb,
 	case MEM_GOING_ONLINE:
 	case MEM_GOING_OFFLINE:
 		/* only ok if no hca is attached to the lpar */
-		spin_lock(&shca_list_lock);
+		spin_lock_irqsave(&shca_list_lock, flags);
 		if (list_empty(&shca_list)) {
-			spin_unlock(&shca_list_lock);
+			spin_unlock_irqrestore(&shca_list_lock, flags);
 			return NOTIFY_OK;
 		} else {
-			spin_unlock(&shca_list_lock);
+			spin_unlock_irqrestore(&shca_list_lock, flags);
 			if (printk_timed_ratelimit(&ehca_dmem_warn_time,
 						   30 * 1000))
 				ehca_gen_err("DMEM operations are not allowed"

Signature · 1 line

1.5.5

Roland Dreier

2008-11-21 18:28 UTC #13 lore raw

Re: [PATCH] IB/ehca: Fix locking for shca_list_lock

Looks good... I'll add this for 2.6.29, since as far as I can tell this bug has been there approximately forever already.

Michael Ellerman

2008-11-22 03:41 UTC #14 lore raw

Re: [PATCH] IB/ehca: Fix lockdep failures for shca_list_lock

In reply to Johannes Berg

Quoted 9 lines · Johannes Berg

drivers/infiniband/hw/ehca/ehca_main.c static int __devinit ehca_probe(struct of_device *dev, · line 834
Quoted 4 lines · Joachim Fenkes
drivers/infiniband/hw/ehca/ehca_main.c +2 -1
+	u64 flags;
 
-	spin_lock(&shca_list_lock);
+	spin_lock_irqsave(&shca_list_lock, flags);
That's wrong and I think will give a warning on all machines where u64 != unsigned long. Might not particularly matter in this case.

Crud, sorry.

Quoted 2 lines

Also, generally it seems wrong to say "fix lockdep failure" when the patch really fixes a bug that lockdep happened to find.

True. I guess it should be "fix locking error found with lockdep", to make it clear no one has actually hit the bug.

cheers

Signature · 8 lines

Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

Joachim Fenkes

author

2008-11-25 12:58 UTC #15 lore raw

PATCH 1/1 IB/ehca: Change misleading error message

In reply to Dotan Barak

The error message printed when the eHCA driver prevents memory hotplug is misleading -- the user might think that hot-removing the lhca, hotplugging memory, then hot-adding the lhca again will work, but it doesn't.

Signed-off-by Joachim Fenkes <[email protected]>

---
 drivers/infiniband/hw/ehca/ehca_main.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

drivers/infiniband/hw/ehca/ehca_main.c +1 -2

index bb02a86..bec7e02 100644
@@ -994,8 +994,7 @@ static int ehca_mem_notifier(struct notifier_block *nb,
 			if (printk_timed_ratelimit(&ehca_dmem_warn_time,
 						   30 * 1000))
 				ehca_gen_err("DMEM operations are not allowed"
-					     "as long as an ehca adapter is"
-					     "attached to the LPAR");
+					     "in conjunction with eHCA");
 			return NOTIFY_BAD;
 		}
 	}

Signature · 1 line

1.5.5

Roland Dreier

2008-11-25 23:13 UTC #16 lore raw

Re: [PATCH] IB/ehca: Change misleading error message

Quoted 3 lines

The error message printed when the eHCA driver prevents memory hotplug is misleading -- the user might think that hot-removing the lhca, hotplugging memory, then hot-adding the lhca again will work, but it doesn't.

That's too bad... I applied this patch but out of curiousity, why doesn't the hot-remove/hot-add work? I would have thought that re-registering all of memory after the hot-add would do the right thing.

Joachim Fenkes

author

2008-11-26 13:45 UTC #17 lore raw

Re: [PATCH] IB/ehca: Change misleading error message

Roland Dreier <[email protected]> wrote on 26.11.2008 00:13:51:

Quoted 3 lines

That's too bad... I applied this patch but out of curiousity, why doesn't the hot-remove/hot-add work? I would have thought that re-registering all of memory after the hot-add would do the right thing.

That's right, but right now, we simply try to register all of memory from KERNELBASE to high_memory, which works right until we have memory holes in the middle; then the hypervisor will reject our page registrations. Same goes for huge (16GB) pages, by the way. We're working on a solution to this.

Cheers,
Joachim

Joachim Fenkes

author

2009-06-03 14:28 UTC #18 lore raw

PATCH 1/1 IB/ehca: Remove superfluous bitmasks from QP control block

In reply to Dotan Barak

All the fields in the control block are nicely right-aligned, so no masking is necessary.

Signed-off-by Joachim Fenkes <[email protected]>

---
 drivers/infiniband/hw/ehca/ehca_classes_pSeries.h |   28 ---------------------
 drivers/infiniband/hw/ehca/ehca_qp.c              |   18 +++---------
 2 files changed, 5 insertions(+), 41 deletions(-)

drivers/infiniband/hw/ehca/ehca_classes_pSeries.h +0 ~~-28~~

index 1798e64..689c357 100644
@@ -165,7 +165,6 @@ struct hcp_modify_qp_control_block {
 #define MQPCB_MASK_ALT_P_KEY_IDX                EHCA_BMASK_IBM( 7,  7)
 #define MQPCB_MASK_RDMA_ATOMIC_CTRL             EHCA_BMASK_IBM( 8,  8)
 #define MQPCB_MASK_QP_STATE                     EHCA_BMASK_IBM( 9,  9)
-#define MQPCB_QP_STATE                          EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_RDMA_NR_ATOMIC_RESP_RES      EHCA_BMASK_IBM(11, 11)
 #define MQPCB_MASK_PATH_MIGRATION_STATE         EHCA_BMASK_IBM(12, 12)
 #define MQPCB_MASK_RDMA_ATOMIC_OUTST_DEST_QP    EHCA_BMASK_IBM(13, 13)

@@ -176,60 +175,33 @@ struct hcp_modify_qp_control_block {
 #define MQPCB_MASK_RETRY_COUNT                  EHCA_BMASK_IBM(18, 18)
 #define MQPCB_MASK_TIMEOUT                      EHCA_BMASK_IBM(19, 19)
 #define MQPCB_MASK_PATH_MTU                     EHCA_BMASK_IBM(20, 20)
-#define MQPCB_PATH_MTU                          EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_MAX_STATIC_RATE              EHCA_BMASK_IBM(21, 21)
-#define MQPCB_MAX_STATIC_RATE                   EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_DLID                         EHCA_BMASK_IBM(22, 22)
-#define MQPCB_DLID                              EHCA_BMASK_IBM(16, 31)
 #define MQPCB_MASK_RNR_RETRY_COUNT              EHCA_BMASK_IBM(23, 23)
-#define MQPCB_RNR_RETRY_COUNT                   EHCA_BMASK_IBM(29, 31)
 #define MQPCB_MASK_SOURCE_PATH_BITS             EHCA_BMASK_IBM(24, 24)
-#define MQPCB_SOURCE_PATH_BITS                  EHCA_BMASK_IBM(25, 31)
 #define MQPCB_MASK_TRAFFIC_CLASS                EHCA_BMASK_IBM(25, 25)
-#define MQPCB_TRAFFIC_CLASS                     EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_HOP_LIMIT                    EHCA_BMASK_IBM(26, 26)
-#define MQPCB_HOP_LIMIT                         EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_SOURCE_GID_IDX               EHCA_BMASK_IBM(27, 27)
-#define MQPCB_SOURCE_GID_IDX                    EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_FLOW_LABEL                   EHCA_BMASK_IBM(28, 28)
-#define MQPCB_FLOW_LABEL                        EHCA_BMASK_IBM(12, 31)
 #define MQPCB_MASK_DEST_GID                     EHCA_BMASK_IBM(30, 30)
 #define MQPCB_MASK_SERVICE_LEVEL_AL             EHCA_BMASK_IBM(31, 31)
-#define MQPCB_SERVICE_LEVEL_AL                  EHCA_BMASK_IBM(28, 31)
 #define MQPCB_MASK_SEND_GRH_FLAG_AL             EHCA_BMASK_IBM(32, 32)
-#define MQPCB_SEND_GRH_FLAG_AL                  EHCA_BMASK_IBM(31, 31)
 #define MQPCB_MASK_RETRY_COUNT_AL               EHCA_BMASK_IBM(33, 33)
-#define MQPCB_RETRY_COUNT_AL                    EHCA_BMASK_IBM(29, 31)
 #define MQPCB_MASK_TIMEOUT_AL                   EHCA_BMASK_IBM(34, 34)
-#define MQPCB_TIMEOUT_AL                        EHCA_BMASK_IBM(27, 31)
 #define MQPCB_MASK_MAX_STATIC_RATE_AL           EHCA_BMASK_IBM(35, 35)
-#define MQPCB_MAX_STATIC_RATE_AL                EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_DLID_AL                      EHCA_BMASK_IBM(36, 36)
-#define MQPCB_DLID_AL                           EHCA_BMASK_IBM(16, 31)
 #define MQPCB_MASK_RNR_RETRY_COUNT_AL           EHCA_BMASK_IBM(37, 37)
-#define MQPCB_RNR_RETRY_COUNT_AL                EHCA_BMASK_IBM(29, 31)
 #define MQPCB_MASK_SOURCE_PATH_BITS_AL          EHCA_BMASK_IBM(38, 38)
-#define MQPCB_SOURCE_PATH_BITS_AL               EHCA_BMASK_IBM(25, 31)
 #define MQPCB_MASK_TRAFFIC_CLASS_AL             EHCA_BMASK_IBM(39, 39)
-#define MQPCB_TRAFFIC_CLASS_AL                  EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_HOP_LIMIT_AL                 EHCA_BMASK_IBM(40, 40)
-#define MQPCB_HOP_LIMIT_AL                      EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_SOURCE_GID_IDX_AL            EHCA_BMASK_IBM(41, 41)
-#define MQPCB_SOURCE_GID_IDX_AL                 EHCA_BMASK_IBM(24, 31)
 #define MQPCB_MASK_FLOW_LABEL_AL                EHCA_BMASK_IBM(42, 42)
-#define MQPCB_FLOW_LABEL_AL                     EHCA_BMASK_IBM(12, 31)
 #define MQPCB_MASK_DEST_GID_AL                  EHCA_BMASK_IBM(44, 44)
 #define MQPCB_MASK_MAX_NR_OUTST_SEND_WR         EHCA_BMASK_IBM(45, 45)
-#define MQPCB_MAX_NR_OUTST_SEND_WR              EHCA_BMASK_IBM(16, 31)
 #define MQPCB_MASK_MAX_NR_OUTST_RECV_WR         EHCA_BMASK_IBM(46, 46)
-#define MQPCB_MAX_NR_OUTST_RECV_WR              EHCA_BMASK_IBM(16, 31)
 #define MQPCB_MASK_DISABLE_ETE_CREDIT_CHECK     EHCA_BMASK_IBM(47, 47)
-#define MQPCB_DISABLE_ETE_CREDIT_CHECK          EHCA_BMASK_IBM(31, 31)
-#define MQPCB_QP_NUMBER                         EHCA_BMASK_IBM( 8, 31)
 #define MQPCB_MASK_QP_ENABLE                    EHCA_BMASK_IBM(48, 48)
-#define MQPCB_QP_ENABLE                         EHCA_BMASK_IBM(31, 31)
 #define MQPCB_MASK_CURR_SRQ_LIMIT               EHCA_BMASK_IBM(49, 49)
-#define MQPCB_CURR_SRQ_LIMIT                    EHCA_BMASK_IBM(16, 31)
 #define MQPCB_MASK_QP_AFF_ASYN_EV_LOG_REG       EHCA_BMASK_IBM(50, 50)
 #define MQPCB_MASK_SHARED_RQ_HNDL               EHCA_BMASK_IBM(51, 51)

drivers/infiniband/hw/ehca/ehca_qp.c +5 ~~-13~~

index ead4e71..0338f1f 100644
@@ -1962,19 +1962,13 @@ int ehca_query_qp(struct ib_qp *qp,
 	qp_attr->cap.max_inline_data = my_qp->sq_max_inline_data_size;
 	qp_attr->dest_qp_num = qpcb->dest_qp_nr;
 
-	qp_attr->pkey_index =
-		EHCA_BMASK_GET(MQPCB_PRIM_P_KEY_IDX, qpcb->prim_p_key_idx);
-
-	qp_attr->port_num =
-		EHCA_BMASK_GET(MQPCB_PRIM_PHYS_PORT, qpcb->prim_phys_port);
-
+	qp_attr->pkey_index = qpcb->prim_p_key_idx;
+	qp_attr->port_num = qpcb->prim_phys_port;
 	qp_attr->timeout = qpcb->timeout;
 	qp_attr->retry_cnt = qpcb->retry_count;
 	qp_attr->rnr_retry = qpcb->rnr_retry_count;
 
-	qp_attr->alt_pkey_index =
-		EHCA_BMASK_GET(MQPCB_PRIM_P_KEY_IDX, qpcb->alt_p_key_idx);
-
+	qp_attr->alt_pkey_index = qpcb->alt_p_key_idx;
 	qp_attr->alt_port_num = qpcb->alt_phys_port;
 	qp_attr->alt_timeout = qpcb->timeout_al;

@@ -2061,8 +2055,7 @@ int ehca_modify_srq(struct ib_srq *ibsrq, struct ib_srq_attr *attr,
 		update_mask |=
 			EHCA_BMASK_SET(MQPCB_MASK_CURR_SRQ_LIMIT, 1)
 			| EHCA_BMASK_SET(MQPCB_MASK_QP_AFF_ASYN_EV_LOG_REG, 1);
-		mqpcb->curr_srq_limit =
-			EHCA_BMASK_SET(MQPCB_CURR_SRQ_LIMIT, attr->srq_limit);
+		mqpcb->curr_srq_limit = attr->srq_limit;
 		mqpcb->qp_aff_asyn_ev_log_reg =
 			EHCA_BMASK_SET(QPX_AAELOG_RESET_SRQ_LIMIT, 1);
 	}

@@ -2125,8 +2118,7 @@ int ehca_query_srq(struct ib_srq *srq, struct ib_srq_attr *srq_attr)
 
 	srq_attr->max_wr = qpcb->max_nr_outst_recv_wr - 1;
 	srq_attr->max_sge = 3;
-	srq_attr->srq_limit = EHCA_BMASK_GET(
-		MQPCB_CURR_SRQ_LIMIT, qpcb->curr_srq_limit);
+	srq_attr->srq_limit = qpcb->curr_srq_limit;
 
 	if (ehca_debug_level >= 2)
 		ehca_dmp(qpcb, 4*70, "qp_num=%x", my_qp->real_qp_num);

Signature · 1 line

1.5.5

Roland Dreier

2009-06-03 20:26 UTC #19 lore raw

Re: [PATCH] IB/ehca: Remove superfluous bitmasks from QP control block

looks fine, applied for 2.6.31

Joachim Fenkes

author

2009-09-01 11:55 UTC #20 lore raw

PATCH 1/1 IB/ehca: Fix CQE flags reporting

In reply to Dotan Barak

Was reporting CQE flags in the wrong bit positions, causing consumers to miss incoming immediate data.

Signed-off-by Joachim Fenkes <[email protected]>

---

Please review and queue for 2.6.32 if you think it's okay. Thanks!
Joachim

 drivers/infiniband/hw/ehca/ehca_reqs.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

drivers/infiniband/hw/ehca/ehca_reqs.c +5 -1

index 5a3d96f..8fd88cd 100644
@@ -786,7 +786,11 @@ repoll:
 	wc->slid = cqe->rlid;
 	wc->dlid_path_bits = cqe->dlid;
 	wc->src_qp = cqe->remote_qp_number;
-	wc->wc_flags = cqe->w_completion_flags;
+	/*
+	 * HW has "Immed data present" and "GRH present" in bits 6 and 5.
+	 * SW defines those in bits 1 and 0, so we can just shift and mask.
+	 */
+	wc->wc_flags = (cqe->w_completion_flags >> 5) & 3;
 	wc->ex.imm_data = cpu_to_be32(cqe->immediate_data);
 	wc->sl = cqe->service_level;

Signature · 1 line

1.6.0.4

Roland Dreier

Applied

2009-09-01 19:55 UTC #21 lore raw

Re: [PATCH] IB/ehca: Fix CQE flags reporting

applied, thanks