[openib-general] [PATCH 12/17] ehca: queue pair

[Heiko J Schick]

Hallo Hal,

we can only modify a QP when the port is active. The port goes
active when you create an AQP1 and the communication with the
SM and the firmware was completed. This can take a while.

We've included the wait, because ib_mad creates an AQP1. After
the AQP1 was created sucessfully, ib_mad tries to do ib_modify_qp
to set the QP in RTS. If we don't wait, until the port went active,
this modifies won't work and ib_mad frees all allocates ressources
for both ports. If that happens ib_mad could not be used anymore
by IPoIB to join a multicast group.

Regards,
Heiko

Hal Rosenstock wrote:
> On Thu, 2006-03-02 at 03:38, Heiko J Schick wrote:
> 
> 
>>-- linux-2.6.16-rc4-orig/drivers/infiniband/hw/ehca/ehca_sqp.c	1970-01-01 01:00:00.000000000 +0100
>>+++ linux-2.6.16-rc4/drivers/infiniband/hw/ehca/ehca_sqp.c	2006-03-01 09:23:12.000000000 +0100
> 
> 
>>+extern int ehca_port_act_time;
>>+
>>+/**
>>+ * ehca_define_sqp - defines special queue pair 1(GSI QP). When special queue
>>+ * pair is created successfully, the corresponding port gets active.
> 
> 
> The wait for active before creating QP1 seems problematic to me for a
> number of reasons. I know there is a module parameter to tune this to
> the subnet. Is there a reason that QP1 can't be created before this (the
> port becoming active) ? I think it may need to be created before the
> port gets to armed state if active trigger is supported (I don't know
> whether that's the case with eHCA).
> 
> 
>>+ * Define Special Queue pair 0 (SMI QP) is still not supported.
> 
> 
> The comment implies that the intent is to support this. Is that true ?
> I'm wondering if in the interim something needs to be done with the core
> to handle this case which I think is valid per the spec. I don't there
> is a requirement that QP0 be exposed and that a device be able to
> support an SM. This should all be handled in a graceful manner by the
> layers above.
> 
> 
>>+ *
>>+ * @qp_init_attr : Queue pair init attributes including port and queue pair type.
>>+ */
>>+
>>+u64 ehca_define_sqp(struct ehca_shca *shca,
>>+			       struct ehca_qp *ehca_qp,
>>+			       struct ib_qp_init_attr *qp_init_attr)
>>+{
>>+
>>+	u32 pma_qp_nr = 0;
>>+	u32 bma_qp_nr = 0;
>>+	u64 ret = H_Success;
>>+	u8 port = qp_init_attr->port_num;
>>+	int counter = 0;
>>+
>>+	EDEB_EN(7, "port=%x qp_type=%x",
>>+		port, qp_init_attr->qp_type);
>>+
>>+	shca->sport[port - 1].port_state = IB_PORT_DOWN;
>>+
>>+	switch (qp_init_attr->qp_type) {
>>+	case IB_QPT_SMI:
>>+		/* function not supported yet */
>>+		break;
> 
> 
> Should this be:
> 		ret = H_unsupported; (or something like this)
> 		goto hca_define_aqp1;
> while this is not supported ?
> 
> 
>>+	case IB_QPT_GSI:
>>+		ret = hipz_h_define_aqp1(shca->ipz_hca_handle,
>>+					 ehca_qp->ipz_qp_handle,
>>+					 ehca_qp->ehca_qp_core.galpas.kernel,
>>+					 (u32) qp_init_attr->port_num,
>>+					 &pma_qp_nr, &bma_qp_nr);
>>+
>>+		if (ret != H_Success) {
>>+			EDEB_ERR(4, "Can't define AQP1 for port %x. rc=%lx",
>>+				    port, ret);
>>+			goto ehca_define_aqp1;
>>+		}
>>+		break;
>>+	default:
>>+		ret = H_Parameter;
>>+		goto ehca_define_aqp1;
>>+	}
>>+
>>+	while ((shca->sport[port - 1].port_state != IB_PORT_ACTIVE) &&
>>+	       (counter < ehca_port_act_time)) {
>>+		EDEB(6, "... wait until port %x is active",
>>+			port);
>>+		msleep_interruptible(1000);
>>+		counter++;
>>+	}
>>+
>>+	if (counter == ehca_port_act_time) {
>>+		EDEB_ERR(4, "Port %x is not active.", port);
>>+		ret = H_Hardware;
>>+	}
>>+
>>+      ehca_define_aqp1:
>>+	EDEB_EX(7, "ret=%lx", ret);
>>+
>>+	return ret;
>>+}
> 
> 
> -- Hal
>