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

[Hal Rosenstock]

Hi Heiko,

On Fri, 2006-03-03 at 09:00, Heiko J Schick wrote:
> Hallo Hal,
> 
> we can only modify a QP when the port is active. The port goes
> active when you create an AQP1

You are referring to how the eHCA works here, right ?

>  and the communication with the
> SM and the firmware was completed. This can take a while.

Understood.

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

Is it just the modifications of the QP1 state at issue here ? If so,
perhaps that can be deferred but does add some extra complexity. I know
some of this has been discussed on the list before.

> If that happens ib_mad could not be used anymore
> by IPoIB to join a multicast group.

Amongst other things too (like SA PathRecords, CM, etc.).

-- Hal

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