Re: [Intel-wired-lan] [PATCH net-next v2 11/13] ice: introduce clear_reset_state operation

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Jacob,


Thank you for your patch.

Am 19.01.23 um 02:16 schrieb Jacob Keller:
> When hardware is reset, the VF relies on the VFGEN_RSTAT register to detect
> when the VF is finished resetting. This is a tri-state register where 0
> indicates a reset is in progress, 1 indicates the hardware is done
> resetting, and 2 indicates that the software is done resetting.
> 
> Currently the PF driver relies on the device hardware resetting VFGEN_RSTAT
> when a global reset occurs. This works ok, but it does mean that the VF
> might not immediately notice a reset when the driver first detects that the
> global reset is occurring.
> 
> This is also problematic for Scalable IOV, because there is no read/write
> equivalent VFGEN_RSTAT register for the Scalable VSI type. Instead, the
> Scalable IOV VFs will need to emulate this register.
> 
> To support this, introduce a new VF operation, clear_reset_state, which is
> called when the PF driver first detects a global reset. The Single Root IOV
> implementation can just write to VFGEN_RSTAT to ensure its cleared

it’s

> immediately, without waiting for the actual hardware reset to begin. The
> Scalable IOV implementation will use this as part of its tracking of the
> reset status to allow properly reporting the emulated VFGEN_RSTAT to the VF
> driver.

Can this be tested somehow? If yes, it’d be great if you added the recipe.

> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Changes since v1:
> * Add no-op version of  ice_set_vf_state_dis for when CONFIG_PCI_IOV is
>    disabled.
> * Make ice_set_state_qs_dis static since there are no callers outsice

outside

>    ice_vf_lib.c
> 
>   drivers/net/ethernet/intel/ice/ice_main.c   |  2 +-
>   drivers/net/ethernet/intel/ice/ice_sriov.c  | 16 ++++++++++++++++
>   drivers/net/ethernet/intel/ice/ice_vf_lib.c | 12 +++++++++++-
>   drivers/net/ethernet/intel/ice/ice_vf_lib.h |  5 +++--
>   4 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 3b98721fd9d8..5d890b6aa9d2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -537,7 +537,7 @@ ice_prepare_for_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
>   	/* Disable VFs until reset is completed */
>   	mutex_lock(&pf->vfs.table_lock);
>   	ice_for_each_vf(pf, bkt, vf)
> -		ice_set_vf_state_qs_dis(vf);
> +		ice_set_vf_state_dis(vf);
>   	mutex_unlock(&pf->vfs.table_lock);
>   
>   	if (ice_is_eswitch_mode_switchdev(pf)) {
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 46088c05d485..4d8930b83b35 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -654,6 +654,21 @@ static void ice_sriov_free_vf(struct ice_vf *vf)
>   	kfree_rcu(vf, rcu);
>   }
>   
> +/**
> + * ice_sriov_clear_reset_state - clears VF Reset status register
> + * @vf: the vf to configure
> + */
> +static void ice_sriov_clear_reset_state(struct ice_vf *vf)
> +{
> +	struct ice_hw *hw = &vf->pf->hw;
> +
> +	/* Clear the reset status register so that VF immediately sees that
> +	 * the device is resetting, even if hardware hasn't yet gotten around
> +	 * to clearing VFGEN_RSTAT for us.
> +	 */
> +	wr32(hw, VFGEN_RSTAT(vf->vf_id), VIRTCHNL_VFR_INPROGRESS);
> +}
> +
>   /**
>    * ice_sriov_clear_mbx_register - clears SRIOV VF's mailbox registers
>    * @vf: the vf to configure
> @@ -787,6 +802,7 @@ static void ice_sriov_post_vsi_rebuild(struct ice_vf *vf)
>   static const struct ice_vf_ops ice_sriov_vf_ops = {
>   	.reset_type = ICE_VF_RESET,
>   	.free = ice_sriov_free_vf,
> +	.clear_reset_state = ice_sriov_clear_reset_state,
>   	.clear_mbx_register = ice_sriov_clear_mbx_register,
>   	.trigger_reset_register = ice_sriov_trigger_reset_register,
>   	.poll_reset_status = ice_sriov_poll_reset_status,
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> index 1a5d64454f99..2ea801ebb2ac 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> @@ -717,7 +717,7 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags)
>    * ice_set_vf_state_qs_dis - Set VF queues state to disabled
>    * @vf: pointer to the VF structure
>    */
> -void ice_set_vf_state_qs_dis(struct ice_vf *vf)
> +static void ice_set_vf_state_qs_dis(struct ice_vf *vf)
>   {
>   	/* Clear Rx/Tx enabled queues flag */
>   	bitmap_zero(vf->txq_ena, ICE_MAX_RSS_QS_PER_VF);
> @@ -725,6 +725,16 @@ void ice_set_vf_state_qs_dis(struct ice_vf *vf)
>   	clear_bit(ICE_VF_STATE_QS_ENA, vf->vf_states);
>   }
>   
> +/**
> + * ice_set_vf_state_dis - Set VF state to disabled
> + * @vf: pointer to the VF structure
> + */
> +void ice_set_vf_state_dis(struct ice_vf *vf)
> +{
> +	ice_set_vf_state_qs_dis(vf);
> +	vf->vf_ops->clear_reset_state(vf);
> +}
> +
>   /* Private functions only accessed from other virtualization files */
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> index e3d94f3ca40d..5bb75edb6cef 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
> @@ -56,6 +56,7 @@ struct ice_mdd_vf_events {
>   struct ice_vf_ops {
>   	enum ice_disq_rst_src reset_type;
>   	void (*free)(struct ice_vf *vf);
> +	void (*clear_reset_state)(struct ice_vf *vf);
>   	void (*clear_mbx_register)(struct ice_vf *vf);
>   	void (*trigger_reset_register)(struct ice_vf *vf, bool is_vflr);
>   	bool (*poll_reset_status)(struct ice_vf *vf);
> @@ -213,7 +214,7 @@ u16 ice_get_num_vfs(struct ice_pf *pf);
>   struct ice_vsi *ice_get_vf_vsi(struct ice_vf *vf);
>   bool ice_is_vf_disabled(struct ice_vf *vf);
>   int ice_check_vf_ready_for_cfg(struct ice_vf *vf);
> -void ice_set_vf_state_qs_dis(struct ice_vf *vf);
> +void ice_set_vf_state_dis(struct ice_vf *vf);
>   bool ice_is_any_vf_in_unicast_promisc(struct ice_pf *pf);
>   void
>   ice_vf_get_promisc_masks(struct ice_vf *vf, struct ice_vsi *vsi,
> @@ -259,7 +260,7 @@ static inline int ice_check_vf_ready_for_cfg(struct ice_vf *vf)
>   	return -EOPNOTSUPP;
>   }
>   
> -static inline void ice_set_vf_state_qs_dis(struct ice_vf *vf)
> +static inline void ice_set_vf_state_dis(struct ice_vf *vf)
>   {
>   }

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan