On Mon, Dec 10, 2018 at 08:58:12PM +0800, liyangyang (M) wrote:
> On 2018/12/6 17:14, Leon Romanovsky wrote:
> > On Thu, Dec 06, 2018 at 04:53:53PM +0800, liyangyang (M) wrote:
> >> Hi, Leon:
> >>
> >> Thanks a lot for your reply.
> >>
> >> On 2018/12/5 22:43, Leon Romanovsky wrote:
> >>> On Wed, Dec 05, 2018 at 05:41:42PM +0800, Lijun Ou wrote:
> >>>> From: Yangyang Li <liyangyang20@xxxxxxxxxx>
> >>>>
> >>>> This patch adds SCC context allocation and initialization support
> >>>> for DCQCN in kernel space driver.
> >>>>
> >>>> Signed-off-by: Yangyang Li <liyangyang20@xxxxxxxxxx>
> >>>> ---
> >>>> drivers/infiniband/hw/hns/hns_roce_cmd.h | 4 ++++
> >>>> drivers/infiniband/hw/hns/hns_roce_device.h | 6 +++++
> >>>> drivers/infiniband/hw/hns/hns_roce_hem.c | 26 ++++++++++++++++++---
> >>>> drivers/infiniband/hw/hns/hns_roce_hem.h | 1 +
> >>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 35 ++++++++++++++++++++++++++++-
> >>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 23 +++++++++++++++++--
> >>>> drivers/infiniband/hw/hns/hns_roce_main.c | 18 +++++++++++++++
> >>>> drivers/infiniband/hw/hns/hns_roce_qp.c | 20 ++++++++++++++++-
> >>>> 8 files changed, 126 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.h b/drivers/infiniband/hw/hns/hns_roce_cmd.h
> >>>> index 927701d..5348282 100644
> >>>> --- a/drivers/infiniband/hw/hns/hns_roce_cmd.h
> >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_cmd.h
> >>>> @@ -89,6 +89,10 @@ enum {
> >>>> HNS_ROCE_CMD_DESTROY_SRQC_BT1 = 0x39,
> >>>> HNS_ROCE_CMD_DESTROY_SRQC_BT2 = 0x3a,
> >>>>
> >>>> + /* SCC CTX BT commands */
> >>>> + HNS_ROCE_CMD_READ_SCC_CTX_BT0 = 0xa4,
> >>>> + HNS_ROCE_CMD_WRITE_SCC_CTX_BT0 = 0xa5,
> >>>> +
> >>>> /* EQC commands */
> >>>> HNS_ROCE_CMD_CREATE_AEQC = 0x80,
> >>>> HNS_ROCE_CMD_MODIFY_AEQC = 0x81,
> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> >>>> index 779dd4c4..718b415 100644
> >>>> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> >>>> @@ -482,6 +482,7 @@ struct hns_roce_qp_table {
> >>>> struct hns_roce_hem_table qp_table;
> >>>> struct hns_roce_hem_table irrl_table;
> >>>> struct hns_roce_hem_table trrl_table;
> >>>> + struct hns_roce_hem_table scc_ctx_table;
> >>>> };
> >>>>
> >>>> struct hns_roce_cq_table {
> >>>> @@ -768,6 +769,7 @@ struct hns_roce_caps {
> >>>> int irrl_entry_sz;
> >>>> int trrl_entry_sz;
> >>>> int cqc_entry_sz;
> >>>> + int scc_ctx_entry_sz;
> >>>> int srqc_entry_sz;
> >>>> int idx_entry_sz;
> >>>> u32 pbl_ba_pg_sz;
> >>>> @@ -780,6 +782,7 @@ struct hns_roce_caps {
> >>>> u32 srqc_bt_num;
> >>>> u32 cqc_bt_num;
> >>>> u32 mpt_bt_num;
> >>>> + u32 scc_ctx_bt_num;
> >>>> u32 qpc_ba_pg_sz;
> >>>> u32 qpc_buf_pg_sz;
> >>>> u32 qpc_hop_num;
> >>>> @@ -795,6 +798,9 @@ struct hns_roce_caps {
> >>>> u32 mtt_ba_pg_sz;
> >>>> u32 mtt_buf_pg_sz;
> >>>> u32 mtt_hop_num;
> >>>> + u32 scc_ctx_ba_pg_sz;
> >>>> + u32 scc_ctx_buf_pg_sz;
> >>>> + u32 scc_ctx_hop_num;
> >>>> u32 cqe_ba_pg_sz;
> >>>> u32 cqe_buf_pg_sz;
> >>>> u32 cqe_hop_num;
> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
> >>>> index 4cdbcaf..9e4a2ba 100644
> >>>> --- a/drivers/infiniband/hw/hns/hns_roce_hem.c
> >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
> >>>> @@ -45,6 +45,7 @@ bool hns_roce_check_whether_mhop(struct hns_roce_dev *hr_dev, u32 type)
> >>>> (hr_dev->caps.mpt_hop_num && type == HEM_TYPE_MTPT) ||
> >>>> (hr_dev->caps.cqc_hop_num && type == HEM_TYPE_CQC) ||
> >>>> (hr_dev->caps.srqc_hop_num && type == HEM_TYPE_SRQC) ||
> >>>> + (hr_dev->caps.scc_ctx_hop_num && type == HEM_TYPE_SCC_CTX) ||
> >>>> (hr_dev->caps.cqe_hop_num && type == HEM_TYPE_CQE) ||
> >>>> (hr_dev->caps.mtt_hop_num && type == HEM_TYPE_MTT) ||
> >>>> (hr_dev->caps.srqwqe_hop_num && type == HEM_TYPE_SRQWQE) ||
> >>>> @@ -125,6 +126,14 @@ int hns_roce_calc_hem_mhop(struct hns_roce_dev *hr_dev,
> >>>> mhop->ba_l0_num = hr_dev->caps.cqc_bt_num;
> >>>> mhop->hop_num = hr_dev->caps.cqc_hop_num;
> >>>> break;
> >>>> + case HEM_TYPE_SCC_CTX:
> >>>> + mhop->buf_chunk_size = 1 << (hr_dev->caps.scc_ctx_buf_pg_sz
> >>>> + + PAGE_SHIFT);
> >>>> + mhop->bt_chunk_size = 1 << (hr_dev->caps.scc_ctx_ba_pg_sz
> >>>> + + PAGE_SHIFT);
> >>>> + mhop->ba_l0_num = hr_dev->caps.scc_ctx_bt_num;
> >>>> + mhop->hop_num = hr_dev->caps.scc_ctx_hop_num;
> >>>> + break;
> >>>> case HEM_TYPE_SRQC:
> >>>> mhop->buf_chunk_size = 1 << (hr_dev->caps.srqc_buf_pg_sz
> >>>> + PAGE_SHIFT);
> >>>> @@ -175,7 +184,7 @@ int hns_roce_calc_hem_mhop(struct hns_roce_dev *hr_dev,
> >>>> return 0;
> >>>>
> >>>> /*
> >>>> - * QPC/MTPT/CQC/SRQC alloc hem for buffer pages.
> >>>> + * QPC/MTPT/CQC/SRQC/SCC_CTX alloc hem for buffer pages.
> >>>> * MTT/CQE alloc hem for bt pages.
> >>>> */
> >>>> bt_num = hns_roce_get_bt_num(table->type, mhop->hop_num);
> >>>> @@ -486,7 +495,7 @@ static int hns_roce_table_mhop_get(struct hns_roce_dev *hr_dev,
> >>>> }
> >>>>
> >>>> /*
> >>>> - * alloc buffer space chunk for QPC/MTPT/CQC/SRQC.
> >>>> + * alloc buffer space chunk for QPC/MTPT/CQC/SRQC/SCC_CTX.
> >>>> * alloc bt space chunk for MTT/CQE.
> >>>> */
> >>>> size = table->type < HEM_TYPE_MTT ? buf_chunk_size : bt_chunk_size;
> >>>> @@ -658,7 +667,7 @@ static void hns_roce_table_mhop_put(struct hns_roce_dev *hr_dev,
> >>>> }
> >>>>
> >>>> /*
> >>>> - * free buffer space chunk for QPC/MTPT/CQC/SRQC.
> >>>> + * free buffer space chunk for QPC/MTPT/CQC/SRQC/SCC_CTX.
> >>>> * free bt space chunk for MTT/CQE.
> >>>> */
> >>>> hns_roce_free_hem(hr_dev, table->hem[hem_idx]);
> >>>> @@ -904,6 +913,14 @@ int hns_roce_init_hem_table(struct hns_roce_dev *hr_dev,
> >>>> num_bt_l0 = hr_dev->caps.cqc_bt_num;
> >>>> hop_num = hr_dev->caps.cqc_hop_num;
> >>>> break;
> >>>> + case HEM_TYPE_SCC_CTX:
> >>>> + buf_chunk_size = 1 << (hr_dev->caps.scc_ctx_buf_pg_sz
> >>>> + + PAGE_SHIFT);
> >>>> + bt_chunk_size = 1 << (hr_dev->caps.scc_ctx_ba_pg_sz
> >>>> + + PAGE_SHIFT);
> >>>> + num_bt_l0 = hr_dev->caps.scc_ctx_bt_num;
> >>>> + hop_num = hr_dev->caps.scc_ctx_hop_num;
> >>>> + break;
> >>>> case HEM_TYPE_SRQC:
> >>>> buf_chunk_size = 1 << (hr_dev->caps.srqc_buf_pg_sz
> >>>> + PAGE_SHIFT);
> >>>> @@ -1081,6 +1098,9 @@ void hns_roce_cleanup_hem(struct hns_roce_dev *hr_dev)
> >>>> hns_roce_cleanup_hem_table(hr_dev,
> >>>> &hr_dev->srq_table.table);
> >>>> hns_roce_cleanup_hem_table(hr_dev, &hr_dev->cq_table.table);
> >>>> + if (hr_dev->caps.scc_ctx_entry_sz)
> >>>> + hns_roce_cleanup_hem_table(hr_dev,
> >>>> + &hr_dev->qp_table.scc_ctx_table);
> >>>> if (hr_dev->caps.trrl_entry_sz)
> >>>> hns_roce_cleanup_hem_table(hr_dev,
> >>>> &hr_dev->qp_table.trrl_table);
> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.h b/drivers/infiniband/hw/hns/hns_roce_hem.h
> >>>> index a650278..6e4fe97 100644
> >>>> --- a/drivers/infiniband/hw/hns/hns_roce_hem.h
> >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hem.h
> >>>> @@ -44,6 +44,7 @@ enum {
> >>>> HEM_TYPE_MTPT,
> >>>> HEM_TYPE_CQC,
> >>>> HEM_TYPE_SRQC,
> >>>> + HEM_TYPE_SCC_CTX,
> >>>>
> >>>> /* UNMAP HEM */
> >>>> HEM_TYPE_MTT,
> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >>>> index 835b783..77cfd9b 100644
> >>>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >>>> @@ -1078,6 +1078,9 @@ static int hns_roce_query_pf_resource(struct hns_roce_dev *hr_dev)
> >>>> hr_dev->caps.sl_num = roce_get_field(req_b->qid_idx_sl_num,
> >>>> PF_RES_DATA_3_PF_SL_NUM_M,
> >>>> PF_RES_DATA_3_PF_SL_NUM_S);
> >>>> + hr_dev->caps.scc_ctx_bt_num = roce_get_field(req_b->scc_ctx_bt_idx_num,
> >>>> + PF_RES_DATA_4_PF_SCC_CTX_BT_NUM_M,
> >>>> + PF_RES_DATA_4_PF_SCC_CTX_BT_NUM_S);
> >>>>
> >>>> return 0;
> >>>> }
> >>>> @@ -1178,6 +1181,7 @@ static int hns_roce_v2_set_bt(struct hns_roce_dev *hr_dev)
> >>>> u8 qpc_hop_num = hr_dev->caps.qpc_hop_num;
> >>>> u8 cqc_hop_num = hr_dev->caps.cqc_hop_num;
> >>>> u8 mpt_hop_num = hr_dev->caps.mpt_hop_num;
> >>>> + u8 scc_ctx_hop_num = hr_dev->caps.scc_ctx_hop_num;
> >>>> struct hns_roce_cfg_bt_attr *req;
> >>>> struct hns_roce_cmq_desc desc;
> >>>>
> >>>> @@ -1225,6 +1229,20 @@ static int hns_roce_v2_set_bt(struct hns_roce_dev *hr_dev)
> >>>> CFG_BT_ATTR_DATA_3_VF_MPT_HOPNUM_S,
> >>>> mpt_hop_num == HNS_ROCE_HOP_NUM_0 ? 0 : mpt_hop_num);
> >>>>
> >>>> + roce_set_field(req->vf_scc_ctx_cfg,
> >>>> + CFG_BT_ATTR_DATA_4_VF_SCC_CTX_BA_PGSZ_M,
> >>>> + CFG_BT_ATTR_DATA_4_VF_SCC_CTX_BA_PGSZ_S,
> >>>> + hr_dev->caps.scc_ctx_ba_pg_sz + PG_SHIFT_OFFSET);
> >>>> + roce_set_field(req->vf_scc_ctx_cfg,
> >>>> + CFG_BT_ATTR_DATA_4_VF_SCC_CTX_BUF_PGSZ_M,
> >>>> + CFG_BT_ATTR_DATA_4_VF_SCC_CTX_BUF_PGSZ_S,
> >>>> + hr_dev->caps.scc_ctx_buf_pg_sz + PG_SHIFT_OFFSET);
> >>>> + roce_set_field(req->vf_scc_ctx_cfg,
> >>>> + CFG_BT_ATTR_DATA_4_VF_SCC_CTX_HOPNUM_M,
> >>>> + CFG_BT_ATTR_DATA_4_VF_SCC_CTX_HOPNUM_S,
> >>>> + scc_ctx_hop_num ==
> >>>> + HNS_ROCE_HOP_NUM_0 ? 0 : scc_ctx_hop_num);
> >>>> +
> >>>> return hns_roce_cmq_send(hr_dev, &desc, 1);
> >>>> }
> >>>>
> >>>> @@ -1372,9 +1390,14 @@ static int hns_roce_v2_profile(struct hns_roce_dev *hr_dev)
> >>>> caps->max_srq_wrs = HNS_ROCE_V2_MAX_SRQ_WR;
> >>>> caps->max_srq_sges = HNS_ROCE_V2_MAX_SRQ_SGE;
> >>>>
> >>>> - if (hr_dev->pci_dev->revision == 0x21)
> >>>> + if (hr_dev->pci_dev->revision == 0x21) {
> >>>> caps->flags |= HNS_ROCE_CAP_FLAG_ATOMIC |
> >>>> HNS_ROCE_CAP_FLAG_SRQ;
> >>>> + caps->scc_ctx_entry_sz = HNS_ROCE_V2_SCC_CTX_ENTRY_SZ;
> >>>> + caps->scc_ctx_ba_pg_sz = 0;
> >>>> + caps->scc_ctx_buf_pg_sz = 0;
> >>>> + caps->scc_ctx_hop_num = HNS_ROCE_SCC_CTX_HOP_NUM;
> >>>> + }
> >>>>
> >>>> ret = hns_roce_v2_set_bt(hr_dev);
> >>>> if (ret)
> >>>> @@ -2618,6 +2641,13 @@ static int hns_roce_v2_set_hem(struct hns_roce_dev *hr_dev,
> >>>> case HEM_TYPE_SRQC:
> >>>> op = HNS_ROCE_CMD_WRITE_SRQC_BT0;
> >>>> break;
> >>>> + case HEM_TYPE_SCC_CTX:
> >>>> + if (step_idx) {
> >>>> + /* No need to notify Hardware when step_idx is 1 or 2*/
> >>>> + return 0;
> >>>> + }
> >>>> + op = HNS_ROCE_CMD_WRITE_SCC_CTX_BT0;
> >>>> + break;
> >>>> default:
> >>>> dev_warn(dev, "Table %d not to be written by mailbox!\n",
> >>>> table->type);
> >>>> @@ -2677,6 +2707,9 @@ static int hns_roce_v2_clear_hem(struct hns_roce_dev *hr_dev,
> >>>> case HEM_TYPE_CQC:
> >>>> op = HNS_ROCE_CMD_DESTROY_CQC_BT0;
> >>>> break;
> >>>> + case HEM_TYPE_SCC_CTX:
> >>>> + /* there is no need to destroy scc ctx */
> >>>> + return 0;
> >>>
> >>> Combination of "break" and "return" constructions in the same
> >>> switch<->case is a great way to shot yourself into the head.
> >>>
> >>> Please don't mix them.
> >> Good suggestion.
> >> "return" will be replaced by "goto" in function hns_roce_v2_set_hem and
> >> function hns_roce_v2_clear_hem.
> >> The modified function is as follows, please help me to see if this is appropriate?
> >>
> >> @@ -2618,6 +2641,14 @@ static int hns_roce_v2_set_hem(struct hns_roce_dev *hr_dev,
> >> case HEM_TYPE_SRQC:
> >> op = HNS_ROCE_CMD_WRITE_SRQC_BT0;
> >> break;
> >> + case HEM_TYPE_SCC_CTX:
> >> + if (step_idx) {
> >> + /* No need to notify Hardware when step_idx is 1 or 2*/
> >> + ret = 0;
> >> + goto done;
> >
> > I have the same feeling about "goto" as for the "return".
> > Let's do this, prior to executing switch():
> >
> > if ( ... == HEM_TYPE_SCC_CTX && !step_idx)
> > return 0;
> As your suggestion, I adjusted the position, and it seems clean and readable.
Thanks, I agree with claim that it is now "clean and readable".
Attachment:
signature.asc
Description: PGP signature
