From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 1668CA0096 for ; Wed, 5 Jun 2019 12:35:14 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AB12F1B9F4; Wed, 5 Jun 2019 12:35:14 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id BB0171B9B6 for ; Wed, 5 Jun 2019 12:35:12 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2FA55C18B2D6; Wed, 5 Jun 2019 10:35:12 +0000 (UTC) Received: from [10.36.112.53] (ovpn-112-53.ams2.redhat.com [10.36.112.53]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 95F1A601A5; Wed, 5 Jun 2019 10:35:10 +0000 (UTC) To: Leyi Rong , qi.z.zhang@intel.com Cc: dev@dpdk.org, Vignesh Sridhar , Paul M Stillwell Jr References: <20190604054248.68510-1-leyi.rong@intel.com> <20190604054248.68510-15-leyi.rong@intel.com> From: Maxime Coquelin Message-ID: <5320d82a-4f46-4e60-b753-1621e8e16d1d@redhat.com> Date: Wed, 5 Jun 2019 12:35:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190604054248.68510-15-leyi.rong@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 05 Jun 2019 10:35:12 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 14/49] net/ice/base: refactor HW table init function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 6/4/19 7:42 AM, Leyi Rong wrote: > 1. Separated the calls to initialize and allocate the HW XLT tables > from call to fill table. This is to allow the ice_init_hw_tbls call > to be made prior to package download so that all HW structures are > correctly initialized. This will avoid any invalid memory references > if package download fails on unloading the driver. > 2. Fill HW tables with package content after successful package download. > 3. Free HW table and flow profile allocations when unloading driver. > 4. Add flag in block structure to check if lists in block are > initialized. This is to avoid any NULL reference in releasing flow > profiles that may have been freed in previous calls to free tables. > > Signed-off-by: Vignesh Sridhar > Signed-off-by: Paul M Stillwell Jr > Signed-off-by: Leyi Rong > --- > drivers/net/ice/base/ice_common.c | 6 +- > drivers/net/ice/base/ice_flex_pipe.c | 284 ++++++++++++++------------- > drivers/net/ice/base/ice_flex_pipe.h | 1 + > drivers/net/ice/base/ice_flex_type.h | 1 + > 4 files changed, 151 insertions(+), 141 deletions(-) > > diff --git a/drivers/net/ice/base/ice_common.c b/drivers/net/ice/base/ice_common.c > index a0ab25aef..62c7fad0d 100644 > --- a/drivers/net/ice/base/ice_common.c > +++ b/drivers/net/ice/base/ice_common.c > @@ -916,12 +916,13 @@ enum ice_status ice_init_hw(struct ice_hw *hw) > > ice_init_flex_flds(hw, ICE_RXDID_FLEX_NIC); > ice_init_flex_flds(hw, ICE_RXDID_FLEX_NIC_2); > - > /* Obtain counter base index which would be used by flow director */ > status = ice_alloc_fd_res_cntr(hw, &hw->fd_ctr_base); > if (status) > goto err_unroll_fltr_mgmt_struct; > - > + status = ice_init_hw_tbls(hw); > + if (status) > + goto err_unroll_fltr_mgmt_struct; > return ICE_SUCCESS; > > err_unroll_fltr_mgmt_struct: > @@ -952,6 +953,7 @@ void ice_deinit_hw(struct ice_hw *hw) > ice_sched_cleanup_all(hw); > ice_sched_clear_agg(hw); > ice_free_seg(hw); > + ice_free_hw_tbls(hw); > > if (hw->port_info) { > ice_free(hw, hw->port_info); > diff --git a/drivers/net/ice/base/ice_flex_pipe.c b/drivers/net/ice/base/ice_flex_pipe.c > index 8f0b513f4..93e056853 100644 > --- a/drivers/net/ice/base/ice_flex_pipe.c > +++ b/drivers/net/ice/base/ice_flex_pipe.c > @@ -1375,10 +1375,12 @@ enum ice_status ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len) > > if (!status) { > hw->seg = seg; > - /* on successful package download, update other required > - * registers to support the package > + /* on successful package download update other required > + * registers to support the package and fill HW tables > + * with package content. > */ > ice_init_pkg_regs(hw); > + ice_fill_blk_tbls(hw); > } else { > ice_debug(hw, ICE_DBG_INIT, "package load failed, %d\n", > status); > @@ -2755,6 +2757,65 @@ static const u32 ice_blk_sids[ICE_BLK_COUNT][ICE_SID_OFF_COUNT] = { > } > }; > > +/** > + * ice_init_sw_xlt1_db - init software XLT1 database from HW tables > + * @hw: pointer to the hardware structure > + * @blk: the HW block to initialize > + */ > +static > +void ice_init_sw_xlt1_db(struct ice_hw *hw, enum ice_block blk) > +{ > + u16 pt; > + > + for (pt = 0; pt < hw->blk[blk].xlt1.count; pt++) { > + u8 ptg; > + > + ptg = hw->blk[blk].xlt1.t[pt]; > + if (ptg != ICE_DEFAULT_PTG) { > + ice_ptg_alloc_val(hw, blk, ptg); > + ice_ptg_add_mv_ptype(hw, blk, pt, ptg); ice_ptg_add_mv_ptype() can fail, error should be propagated. > + } > + } > +} > + > +/** > + * ice_init_sw_xlt2_db - init software XLT2 database from HW tables > + * @hw: pointer to the hardware structure > + * @blk: the HW block to initialize > + */ > +static void ice_init_sw_xlt2_db(struct ice_hw *hw, enum ice_block blk) > +{ > + u16 vsi; > + > + for (vsi = 0; vsi < hw->blk[blk].xlt2.count; vsi++) { > + u16 vsig; > + > + vsig = hw->blk[blk].xlt2.t[vsi]; > + if (vsig) { > + ice_vsig_alloc_val(hw, blk, vsig); > + ice_vsig_add_mv_vsi(hw, blk, vsi, vsig); Ditto > + /* no changes at this time, since this has been > + * initialized from the original package > + */ > + hw->blk[blk].xlt2.vsis[vsi].changed = 0; > + } > + } > +} > + > +/** > + * ice_init_sw_db - init software database from HW tables > + * @hw: pointer to the hardware structure > + */ > +static void ice_init_sw_db(struct ice_hw *hw) > +{ > + u16 i; > + > + for (i = 0; i < ICE_BLK_COUNT; i++) { > + ice_init_sw_xlt1_db(hw, (enum ice_block)i); > + ice_init_sw_xlt2_db(hw, (enum ice_block)i); > + } And so this function should also propagate the error. > +} > + > /** > * ice_fill_tbl - Reads content of a single table type into database > * @hw: pointer to the hardware structure > @@ -2853,12 +2914,12 @@ static void ice_fill_tbl(struct ice_hw *hw, enum ice_block block_id, u32 sid) > case ICE_SID_FLD_VEC_PE: > es = (struct ice_sw_fv_section *)sect; > src = (u8 *)es->fv; > - sect_len = LE16_TO_CPU(es->count) * > - hw->blk[block_id].es.fvw * > + sect_len = (u32)(LE16_TO_CPU(es->count) * > + hw->blk[block_id].es.fvw) * > sizeof(*hw->blk[block_id].es.t); > dst = (u8 *)hw->blk[block_id].es.t; > - dst_len = hw->blk[block_id].es.count * > - hw->blk[block_id].es.fvw * > + dst_len = (u32)(hw->blk[block_id].es.count * > + hw->blk[block_id].es.fvw) * > sizeof(*hw->blk[block_id].es.t); > break; > default: > @@ -2886,75 +2947,61 @@ static void ice_fill_tbl(struct ice_hw *hw, enum ice_block block_id, u32 sid) > } > > /** > - * ice_fill_blk_tbls - Read package content for tables of a block > + * ice_fill_blk_tbls - Read package context for tables > * @hw: pointer to the hardware structure > - * @block_id: The block ID which contains the tables to be copied > * > * Reads the current package contents and populates the driver > - * database with the data it contains to allow for advanced driver > - * features. > - */ > -static void ice_fill_blk_tbls(struct ice_hw *hw, enum ice_block block_id) > -{ > - ice_fill_tbl(hw, block_id, hw->blk[block_id].xlt1.sid); > - ice_fill_tbl(hw, block_id, hw->blk[block_id].xlt2.sid); > - ice_fill_tbl(hw, block_id, hw->blk[block_id].prof.sid); > - ice_fill_tbl(hw, block_id, hw->blk[block_id].prof_redir.sid); > - ice_fill_tbl(hw, block_id, hw->blk[block_id].es.sid); > -} > - > -/** > - * ice_free_flow_profs - free flow profile entries > - * @hw: pointer to the hardware structure > + * database with the data iteratively for all advanced feature > + * blocks. Assume that the Hw tables have been allocated. > */ > -static void ice_free_flow_profs(struct ice_hw *hw) > +void ice_fill_blk_tbls(struct ice_hw *hw) > { > u8 i; > > for (i = 0; i < ICE_BLK_COUNT; i++) { > - struct ice_flow_prof *p, *tmp; > - > - if (!&hw->fl_profs[i]) > - continue; > - > - /* This call is being made as part of resource deallocation > - * during unload. Lock acquire and release will not be > - * necessary here. > - */ > - LIST_FOR_EACH_ENTRY_SAFE(p, tmp, &hw->fl_profs[i], > - ice_flow_prof, l_entry) { > - struct ice_flow_entry *e, *t; > - > - LIST_FOR_EACH_ENTRY_SAFE(e, t, &p->entries, > - ice_flow_entry, l_entry) > - ice_flow_rem_entry(hw, ICE_FLOW_ENTRY_HNDL(e)); > - > - LIST_DEL(&p->l_entry); > - if (p->acts) > - ice_free(hw, p->acts); > - ice_free(hw, p); > - } > + enum ice_block blk_id = (enum ice_block)i; > > - ice_destroy_lock(&hw->fl_profs_locks[i]); > + ice_fill_tbl(hw, blk_id, hw->blk[blk_id].xlt1.sid); > + ice_fill_tbl(hw, blk_id, hw->blk[blk_id].xlt2.sid); > + ice_fill_tbl(hw, blk_id, hw->blk[blk_id].prof.sid); > + ice_fill_tbl(hw, blk_id, hw->blk[blk_id].prof_redir.sid); > + ice_fill_tbl(hw, blk_id, hw->blk[blk_id].es.sid); > } > + > + ice_init_sw_db(hw); Propagate error once above is fixed. > } >