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

[Cheng Xu <chengyou@linux.alibaba.com>]

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;
>> +}
>> +

<...>