Re: [PATCH for-next v3 08/12] RDMA/erdma: Add verbs implementation

[Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx>]

On 3/10/22 11:21 PM, Bernard Metzler wrote:
>
<...>
> > +
> > +int erdma_query_port(struct ib_device *ibdev, u32 port,
> > +		     struct ib_port_attr *attr)
> > +{
> > +	struct erdma_dev *dev = to_edev(ibdev);
> > +	int ret = 0;
>
> not needed. just return 0.

Will fix it.

> > +
> > +	memset(attr, 0, sizeof(*attr));
> > +
> > +	attr->state = dev->state;
> > +	if (dev->netdev) {
> > +		ret = ib_get_eth_speed(ibdev, port, &attr->active_speed,
> > +				       &attr->active_width);
> > +		attr->max_mtu = ib_mtu_int_to_enum(dev->netdev->mtu);
> > +		attr->active_mtu = ib_mtu_int_to_enum(dev->netdev->mtu);
> > +	}
> > +
> > +	attr->gid_tbl_len = 1;
> > +	attr->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_DEVICE_MGMT_SUP;
> > +	attr->max_msg_sz = -1;
> > +	if (dev->state == IB_PORT_ACTIVE)
> > +		attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> > +	else
> > +		attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> > +
> > +	return ret;
> > +}
> > +
> > +int erdma_get_port_immutable(struct ib_device *ibdev, u32 port,
> > +			     struct ib_port_immutable *port_immutable)
> > +{
> > +	port_immutable->gid_tbl_len = 1;
> > +	port_immutable->core_cap_flags = RDMA_CORE_PORT_IWARP;
> > +
> > +	return 0;
> > +}
> > +
> > +int erdma_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
> > +{
> > +	struct erdma_pd *pd = to_epd(ibpd);
> > +	struct erdma_dev *dev = to_edev(ibpd->device);
> > +	int pdn;
> > +
> > +	pdn = erdma_alloc_idx(&dev->res_cb[ERDMA_RES_TYPE_PD]);
> > +	if (pdn < 0)
> > +		return pdn;
> > +
> > +	pd->pdn = pdn;
> > +
> > +	return 0;
> > +}
> > +
> > +int erdma_dealloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
> > +{
> > +	struct erdma_pd *pd = to_epd(ibpd);
> > +	struct erdma_dev *dev = to_edev(ibpd->device);
> > +
> > +	erdma_free_idx(&dev->res_cb[ERDMA_RES_TYPE_PD], pd->pdn);
> > +
> > +	return 0;
> > +}
> > +
> > +static int erdma_qp_validate_cap(struct erdma_dev *dev,
> > +				 struct ib_qp_init_attr *attrs)
> > +{
> > +	if ((attrs->cap.max_send_wr > dev->attrs.max_send_wr) ||
> > +	    (attrs->cap.max_recv_wr > dev->attrs.max_recv_wr) ||
> > +	    (attrs->cap.max_send_sge > dev->attrs.max_send_sge) ||
> > +	    (attrs->cap.max_recv_sge > dev->attrs.max_recv_sge) ||
> > +	    (attrs->cap.max_inline_data > ERDMA_MAX_INLINE) ||
> > +	    !attrs->cap.max_send_wr || !attrs->cap.max_recv_wr) {
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int erdma_qp_validate_attr(struct erdma_dev *dev,
> > +				  struct ib_qp_init_attr *attrs)
> > +{
> > +	if (attrs->qp_type != IB_QPT_RC)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (attrs->srq)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!attrs->send_cq || !attrs->recv_cq)
> > +		return -EOPNOTSUPP;
> > +
> > +	return 0;
> > +}
> > +
> > +static void free_kernel_qp(struct erdma_qp *qp)
> > +{
> > +	struct erdma_dev *dev = qp->dev;
> > +
> > +	vfree(qp->kern_qp.swr_tbl);
> > +	vfree(qp->kern_qp.rwr_tbl);
> > +
> > +	if (qp->kern_qp.sq_buf)
> > +		dma_free_coherent(
> > +			&dev->pdev->dev,
> > +			WARPPED_BUFSIZE(qp->attrs.sq_size << SQEBB_SHIFT),
> > +			qp->kern_qp.sq_buf, qp->kern_qp.sq_buf_dma_addr);
> > +
> > +	if (qp->kern_qp.rq_buf)
> > +		dma_free_coherent(
> > +			&dev->pdev->dev,
> > +			WARPPED_BUFSIZE(qp->attrs.rq_size << RQE_SHIFT),
> > +			qp->kern_qp.rq_buf, qp->kern_qp.rq_buf_dma_addr);
> > +}
> > +
> > +static int init_kernel_qp(struct erdma_dev *dev, struct erdma_qp *qp,
> > +			  struct ib_qp_init_attr *attrs)
> > +{
> > +	struct erdma_kqp *kqp = &qp->kern_qp;
> > +	int ret = -ENOMEM;
>
> not needed. jut return -ENOMEM at the one possible
> place.

Sure, will fix it.

Thanks,
Cheng Xu

> > +	int size;
> > +
> > +	if (attrs->sq_sig_type == IB_SIGNAL_ALL_WR)
> > +		kqp->sig_all = 1;
> > +
> > +	kqp->sq_pi = 0;
> > +	kqp->sq_ci = 0;
> > +	kqp->rq_pi = 0;
> > +	kqp->rq_ci = 0;
> > +	kqp->hw_sq_db =
> > +		dev->func_bar + (ERDMA_SDB_SHARED_PAGE_INDEX << PAGE_SHIFT);
> > +	kqp->hw_rq_db = dev->func_bar + ERDMA_BAR_RQDB_SPACE_OFFSET;
> > +
> > +	kqp->swr_tbl = vmalloc(qp->attrs.sq_size * sizeof(u64));
> > +	kqp->rwr_tbl = vmalloc(qp->attrs.rq_size * sizeof(u64));
> > +
> > +	size = (qp->attrs.sq_size << SQEBB_SHIFT) + ERDMA_EXTRA_BUFFER_SIZE;
> > +	kqp->sq_buf = dma_alloc_coherent(&dev->pdev->dev, size,
> > +					 &kqp->sq_buf_dma_addr, GFP_KERNEL);
> > +	if (!kqp->sq_buf)
> > +		goto err_out;
> > +
> > +	size = (qp->attrs.rq_size << RQE_SHIFT) + ERDMA_EXTRA_BUFFER_SIZE;
> > +	kqp->rq_buf = dma_alloc_coherent(&dev->pdev->dev, size,
> > +					 &kqp->rq_buf_dma_addr, GFP_KERNEL);
> > +	if (!kqp->rq_buf)
> > +		goto err_out;
> > +
> > +	kqp->sq_db_info = kqp->sq_buf + (qp->attrs.sq_size << SQEBB_SHIFT);
> > +	kqp->rq_db_info = kqp->rq_buf + (qp->attrs.rq_size << RQE_SHIFT);
> > +
> > +	return 0;
> > +
> > +err_out:
> > +	free_kernel_qp(qp);
> > +	return ret;
> > +}
> > +

<...>