| Message ID | 20240801015829.4011051-1-paul.greenwalt@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [iwl-next,v2] ice: add E830 HW VF mailbox message limit support | expand |
From: Paul Greenwalt <paul.greenwalt@intel.com> Date: Wed, 31 Jul 2024 21:58:29 -0400 > E830 adds hardware support to prevent the VF from overflowing the PF > mailbox with VIRTCHNL messages. E830 will use the hardware feature > (ICE_F_MBX_LIMIT) instead of the software solution ice_is_malicious_vf(). > > To prevent a VF from overflowing the PF, the PF sets the number of > messages per VF that can be in the PF's mailbox queue > (ICE_MBX_OVERFLOW_WATERMARK). When the PF process a message from a VF, > the PF decrements the per VF message count using the E830_MBX_VF_DEC_TRIG > register. Sorry seems like I didn't add netdev/IWL to my review, could you please read and address it? > > Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> Thanks, Olek
On 8/1/2024 8:56 AM, Alexander Lobakin wrote: > From: Paul Greenwalt <paul.greenwalt@intel.com> > Date: Wed, 31 Jul 2024 21:58:29 -0400 > >> E830 adds hardware support to prevent the VF from overflowing the PF >> mailbox with VIRTCHNL messages. E830 will use the hardware feature >> (ICE_F_MBX_LIMIT) instead of the software solution ice_is_malicious_vf(). >> >> To prevent a VF from overflowing the PF, the PF sets the number of >> messages per VF that can be in the PF's mailbox queue >> (ICE_MBX_OVERFLOW_WATERMARK). When the PF process a message from a VF, >> the PF decrements the per VF message count using the E830_MBX_VF_DEC_TRIG >> register. > > Sorry seems like I didn't add netdev/IWL to my review, could you please > read and address it? > Sorry Olek, but I don't seem to have your feedback. I will read and address any feedback you have. Thanks >> >> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> > > Thanks, > Olek
From: Paul Greenwalt <paul.greenwalt@intel.com> Date: Wed, 31 Jul 2024 21:58:29 -0400 > E830 adds hardware support to prevent the VF from overflowing the PF > mailbox with VIRTCHNL messages. E830 will use the hardware feature > (ICE_F_MBX_LIMIT) instead of the software solution ice_is_malicious_vf(). > > To prevent a VF from overflowing the PF, the PF sets the number of > messages per VF that can be in the PF's mailbox queue > (ICE_MBX_OVERFLOW_WATERMARK). When the PF process a message from a VF, > the PF decrements the per VF message count using the E830_MBX_VF_DEC_TRIG > register. > > Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> > --- > v1 -> v2: > - Update ice_mbx_vf_dec_trig_e830 and ice_mbx_vf_clear_cnt_e830 onstack > variables to const > --- > drivers/net/ethernet/intel/ice/ice.h | 1 + > .../net/ethernet/intel/ice/ice_hw_autogen.h | 3 ++ > drivers/net/ethernet/intel/ice/ice_lib.c | 12 +++++++ > drivers/net/ethernet/intel/ice/ice_main.c | 24 ++++++++++---- > drivers/net/ethernet/intel/ice/ice_sriov.c | 3 +- > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 26 +++++++++++++-- > drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 32 +++++++++++++++++++ > drivers/net/ethernet/intel/ice/ice_vf_mbx.h | 3 ++ > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 8 +++-- > 9 files changed, 99 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h > index 4c563b0d57ac..53c8edbfaede 100644 > --- a/drivers/net/ethernet/intel/ice/ice.h > +++ b/drivers/net/ethernet/intel/ice/ice.h > @@ -207,6 +207,7 @@ enum ice_feature { > ICE_F_GNSS, > ICE_F_ROCE_LAG, > ICE_F_SRIOV_LAG, > + ICE_F_MBX_LIMIT, > ICE_F_MAX > }; > > diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > index 91cbae1eec89..a306ea9b207c 100644 > --- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > +++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > @@ -539,5 +539,8 @@ > #define E830_PRTMAC_CL01_QNT_THR_CL0_M GENMASK(15, 0) > #define VFINT_DYN_CTLN(_i) (0x00003800 + ((_i) * 4)) > #define VFINT_DYN_CTLN_CLEARPBA_M BIT(1) > +#define E830_MBX_PF_IN_FLIGHT_VF_MSGS_THRESH 0x00234000 > +#define E830_MBX_VF_DEC_TRIG(_VF) (0x00233800 + ((_VF) * 4)) > +#define E830_MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT(_VF) (0x00233000 + ((_VF) * 4)) Still poor indentation, there must be tabs, no spaces. Also, parenthesis around `(_VF) * 4` are redundant. > > #endif /* _ICE_HW_AUTOGEN_H_ */ > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index fc5b87f51702..a6fa2ed6c4ab 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -3938,6 +3938,18 @@ void ice_init_feature_support(struct ice_pf *pf) > if (ice_gnss_is_gps_present(&pf->hw)) > ice_set_feature_support(pf, ICE_F_GNSS); > break; > + case ICE_DEV_ID_E830CC_BACKPLANE: > + case ICE_DEV_ID_E830CC_QSFP56: > + case ICE_DEV_ID_E830CC_SFP: > + case ICE_DEV_ID_E830CC_SFP_DD: > + case ICE_DEV_ID_E830C_BACKPLANE: > + case ICE_DEV_ID_E830C_QSFP: > + case ICE_DEV_ID_E830C_SFP: > + case ICE_DEV_ID_E830_XXV_BACKPLANE: > + case ICE_DEV_ID_E830_XXV_QSFP: > + case ICE_DEV_ID_E830_XXV_SFP: Can't this be somehow compressed via case A .. B There are no holes between at least some of these. > + ice_set_feature_support(pf, ICE_F_MBX_LIMIT); > + break; > default: > break; > } [...] > diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > index 40cb4ba0789c..65d9c41bed21 100644 > --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > @@ -210,6 +210,38 @@ ice_mbx_detect_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, > return 0; > } > > +/** > + * ice_mbx_vf_dec_trig_e830 - Decrements the VF mailbox queue counter > + * @hw: pointer to the HW struct > + * @event: pointer to the control queue receive event > + * > + * This function triggers to decrement the counter > + * MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT when the driver replenishes > + * the buffers at the PF mailbox queue. > + */ > +void ice_mbx_vf_dec_trig_e830(const struct ice_hw *hw, > + const struct ice_rq_event_info *event) > +{ > + const u16 vfid = le16_to_cpu(event->desc.retval); Oops, I may've confused you. You don't need to constify scalars. Mainly pointers and structures when possible. > + > + wr32(hw, E830_MBX_VF_DEC_TRIG(vfid), 1); > +} > + > +/** > + * ice_mbx_vf_clear_cnt_e830 - Clear the VF mailbox queue count > + * @hw: pointer to the HW struct > + * @vf_id: VF ID in the PF space > + * > + * This function clears the counter MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT, and should > + * be called when a VF is created and on VF reset. > + */ > +void ice_mbx_vf_clear_cnt_e830(const struct ice_hw *hw, const u16 vf_id) > +{ > + const u32 reg = rd32(hw, E830_MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT(vf_id)); Same here for @vf_id and @reg. > + > + wr32(hw, E830_MBX_VF_DEC_TRIG(vf_id), reg); > +} Thanks, Olek
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index 4c563b0d57ac..53c8edbfaede 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -207,6 +207,7 @@ enum ice_feature { ICE_F_GNSS, ICE_F_ROCE_LAG, ICE_F_SRIOV_LAG, + ICE_F_MBX_LIMIT, ICE_F_MAX }; diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h index 91cbae1eec89..a306ea9b207c 100644 --- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h +++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h @@ -539,5 +539,8 @@ #define E830_PRTMAC_CL01_QNT_THR_CL0_M GENMASK(15, 0) #define VFINT_DYN_CTLN(_i) (0x00003800 + ((_i) * 4)) #define VFINT_DYN_CTLN_CLEARPBA_M BIT(1) +#define E830_MBX_PF_IN_FLIGHT_VF_MSGS_THRESH 0x00234000 +#define E830_MBX_VF_DEC_TRIG(_VF) (0x00233800 + ((_VF) * 4)) +#define E830_MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT(_VF) (0x00233000 + ((_VF) * 4)) #endif /* _ICE_HW_AUTOGEN_H_ */ diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index fc5b87f51702..a6fa2ed6c4ab 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -3938,6 +3938,18 @@ void ice_init_feature_support(struct ice_pf *pf) if (ice_gnss_is_gps_present(&pf->hw)) ice_set_feature_support(pf, ICE_F_GNSS); break; + case ICE_DEV_ID_E830CC_BACKPLANE: + case ICE_DEV_ID_E830CC_QSFP56: + case ICE_DEV_ID_E830CC_SFP: + case ICE_DEV_ID_E830CC_SFP_DD: + case ICE_DEV_ID_E830C_BACKPLANE: + case ICE_DEV_ID_E830C_QSFP: + case ICE_DEV_ID_E830C_SFP: + case ICE_DEV_ID_E830_XXV_BACKPLANE: + case ICE_DEV_ID_E830_XXV_QSFP: + case ICE_DEV_ID_E830_XXV_SFP: + ice_set_feature_support(pf, ICE_F_MBX_LIMIT); + break; default: break; } diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 8ea52c4ff2b3..dcfc4d957b99 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -1559,12 +1559,20 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type) ice_vf_lan_overflow_event(pf, &event); break; case ice_mbx_opc_send_msg_to_pf: - data.num_msg_proc = i; - data.num_pending_arq = pending; - data.max_num_msgs_mbx = hw->mailboxq.num_rq_entries; - data.async_watermark_val = ICE_MBX_OVERFLOW_WATERMARK; + if (ice_is_feature_supported(pf, ICE_F_MBX_LIMIT)) { + ice_vc_process_vf_msg(pf, &event, NULL); + ice_mbx_vf_dec_trig_e830(hw, &event); + } else { + u16 val = hw->mailboxq.num_rq_entries; + + data.max_num_msgs_mbx = val; + val = ICE_MBX_OVERFLOW_WATERMARK; + data.async_watermark_val = val; + data.num_msg_proc = i; + data.num_pending_arq = pending; - ice_vc_process_vf_msg(pf, &event, &data); + ice_vc_process_vf_msg(pf, &event, &data); + } break; case ice_aqc_opc_fw_logs_event: ice_get_fwlog_data(pf, &event); @@ -4084,7 +4092,11 @@ static int ice_init_pf(struct ice_pf *pf) mutex_init(&pf->vfs.table_lock); hash_init(pf->vfs.table); - ice_mbx_init_snapshot(&pf->hw); + if (ice_is_feature_supported(pf, ICE_F_MBX_LIMIT)) + wr32(&pf->hw, E830_MBX_PF_IN_FLIGHT_VF_MSGS_THRESH, + ICE_MBX_OVERFLOW_WATERMARK); + else + ice_mbx_init_snapshot(&pf->hw); xa_init(&pf->dyn_ports); xa_init(&pf->sf_nums); diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index e34fe2516ccc..e7b5fe553d1f 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -194,7 +194,8 @@ void ice_free_vfs(struct ice_pf *pf) } /* clear malicious info since the VF is getting released */ - list_del(&vf->mbx_info.list_entry); + if (!ice_is_feature_supported(pf, ICE_F_MBX_LIMIT)) + list_del(&vf->mbx_info.list_entry); mutex_unlock(&vf->cfg_lock); } diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index a69e91f88d81..d618292dfe27 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -709,6 +709,23 @@ ice_vf_clear_vsi_promisc(struct ice_vf *vf, struct ice_vsi *vsi, u8 promisc_m) return 0; } +/** + * ice_reset_vf_mbx_cnt - reset VF mailbox message count + * @vf: pointer to the VF structure + * + * This function clears the VF mailbox message count, and should be called on + * VF reset. + */ +static void ice_reset_vf_mbx_cnt(struct ice_vf *vf) +{ + struct ice_pf *pf = vf->pf; + + if (ice_is_feature_supported(pf, ICE_F_MBX_LIMIT)) + ice_mbx_vf_clear_cnt_e830(&pf->hw, vf->vf_id); + else + ice_mbx_clear_malvf(&vf->mbx_info); +} + /** * ice_reset_all_vfs - reset all allocated VFs in one go * @pf: pointer to the PF structure @@ -735,7 +752,7 @@ void ice_reset_all_vfs(struct ice_pf *pf) /* clear all malicious info if the VFs are getting reset */ ice_for_each_vf(pf, bkt, vf) - ice_mbx_clear_malvf(&vf->mbx_info); + ice_reset_vf_mbx_cnt(vf); /* If VFs have been disabled, there is no need to reset */ if (test_and_set_bit(ICE_VF_DIS, pf->state)) { @@ -951,7 +968,7 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) ice_eswitch_update_repr(&vf->repr_id, vsi); /* if the VF has been reset allow it to come up again */ - ice_mbx_clear_malvf(&vf->mbx_info); + ice_reset_vf_mbx_cnt(vf); out_unlock: if (lag && lag->bonded && lag->primary && @@ -1004,7 +1021,10 @@ void ice_initialize_vf_entry(struct ice_vf *vf) ice_vf_fdir_init(vf); /* Initialize mailbox info for this VF */ - ice_mbx_init_vf_info(&pf->hw, &vf->mbx_info); + if (ice_is_feature_supported(pf, ICE_F_MBX_LIMIT)) + ice_mbx_vf_clear_cnt_e830(&pf->hw, vf->vf_id); + else + ice_mbx_init_vf_info(&pf->hw, &vf->mbx_info); mutex_init(&vf->cfg_lock); } diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c index 40cb4ba0789c..65d9c41bed21 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c @@ -210,6 +210,38 @@ ice_mbx_detect_malvf(struct ice_hw *hw, struct ice_mbx_vf_info *vf_info, return 0; } +/** + * ice_mbx_vf_dec_trig_e830 - Decrements the VF mailbox queue counter + * @hw: pointer to the HW struct + * @event: pointer to the control queue receive event + * + * This function triggers to decrement the counter + * MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT when the driver replenishes + * the buffers at the PF mailbox queue. + */ +void ice_mbx_vf_dec_trig_e830(const struct ice_hw *hw, + const struct ice_rq_event_info *event) +{ + const u16 vfid = le16_to_cpu(event->desc.retval); + + wr32(hw, E830_MBX_VF_DEC_TRIG(vfid), 1); +} + +/** + * ice_mbx_vf_clear_cnt_e830 - Clear the VF mailbox queue count + * @hw: pointer to the HW struct + * @vf_id: VF ID in the PF space + * + * This function clears the counter MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT, and should + * be called when a VF is created and on VF reset. + */ +void ice_mbx_vf_clear_cnt_e830(const struct ice_hw *hw, const u16 vf_id) +{ + const u32 reg = rd32(hw, E830_MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT(vf_id)); + + wr32(hw, E830_MBX_VF_DEC_TRIG(vf_id), reg); +} + /** * ice_mbx_vf_state_handler - Handle states of the overflow algorithm * @hw: pointer to the HW struct diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.h b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h index 44bc030d17e0..409931623222 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.h @@ -19,6 +19,9 @@ ice_aq_send_msg_to_vf(struct ice_hw *hw, u16 vfid, u32 v_opcode, u32 v_retval, u8 *msg, u16 msglen, struct ice_sq_cd *cd); u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed); +void ice_mbx_vf_dec_trig_e830(const struct ice_hw *hw, + const struct ice_rq_event_info *event); +void ice_mbx_vf_clear_cnt_e830(const struct ice_hw *hw, const u16 vf_id); int ice_mbx_vf_state_handler(struct ice_hw *hw, struct ice_mbx_data *mbx_data, struct ice_mbx_vf_info *vf_info, bool *report_malvf); diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c index 59f62306b9cb..3c86d0c2fe1f 100644 --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c @@ -4009,8 +4009,10 @@ ice_is_malicious_vf(struct ice_vf *vf, struct ice_mbx_data *mbxdata) * @event: pointer to the AQ event * @mbxdata: information used to detect VF attempting mailbox overflow * - * called from the common asq/arq handler to - * process request from VF + * Called from the common asq/arq handler to process request from VF. When this + * flow is used for devices with hardware VF to PF message queue overflow + * support (ICE_F_MBX_LIMIT) mbxdata is set to NULL and ice_is_malicious_vf + * check is skipped. */ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event, struct ice_mbx_data *mbxdata) @@ -4036,7 +4038,7 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event, mutex_lock(&vf->cfg_lock); /* Check if the VF is trying to overflow the mailbox */ - if (ice_is_malicious_vf(vf, mbxdata)) + if (mbxdata && ice_is_malicious_vf(vf, mbxdata)) goto finish; /* Check if VF is disabled. */
E830 adds hardware support to prevent the VF from overflowing the PF mailbox with VIRTCHNL messages. E830 will use the hardware feature (ICE_F_MBX_LIMIT) instead of the software solution ice_is_malicious_vf(). To prevent a VF from overflowing the PF, the PF sets the number of messages per VF that can be in the PF's mailbox queue (ICE_MBX_OVERFLOW_WATERMARK). When the PF process a message from a VF, the PF decrements the per VF message count using the E830_MBX_VF_DEC_TRIG register. Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> --- v1 -> v2: - Update ice_mbx_vf_dec_trig_e830 and ice_mbx_vf_clear_cnt_e830 onstack variables to const --- drivers/net/ethernet/intel/ice/ice.h | 1 + .../net/ethernet/intel/ice/ice_hw_autogen.h | 3 ++ drivers/net/ethernet/intel/ice/ice_lib.c | 12 +++++++ drivers/net/ethernet/intel/ice/ice_main.c | 24 ++++++++++---- drivers/net/ethernet/intel/ice/ice_sriov.c | 3 +- drivers/net/ethernet/intel/ice/ice_vf_lib.c | 26 +++++++++++++-- drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 32 +++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_vf_mbx.h | 3 ++ drivers/net/ethernet/intel/ice/ice_virtchnl.c | 8 +++-- 9 files changed, 99 insertions(+), 13 deletions(-)