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

[Hal Rosenstock]

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