[Intel-wired-lan] [PATCH net-next v2 00/13] ice: various virtualization cleanups

[Intel-wired-lan] [PATCH net-next v2 00/13] ice: various virtualization cleanups

[Jacob Keller]

This series contains a variety of refactors and cleanups in the VF code for
the ice driver. Its primary focus is to cleanup a few areas of the driver
before we begin implementing Scalable IOV.

This includes some cleanup and simplification of the VF operations and
addition of a few new operations that will be required by Scalable IOV, as
well as some other refactors needed for the handling of VF subfunctions.

It also contains a patch for changing the way we handle the mapping of the
PCI BAR 0 registers to allow sparse maps. This is done to enable iRDMA to
map the RDMA registers with write combining. Today, the iRDMA driver tries
this, but since the ice driver already mapped them as uncachable, the write
combining does not take effect. This results in additional latency as the
write combining can't be used.

This is fixed by modifying the driver to perform its own iomaps to map the
region before the RDMA space and the region after the space separately. We
can't just reduce the size of the map because Scalable IOV will need the
registers in the region beyond the RDMA area.

Changes since v1
* drop "ice: add helper function for checking VSI VF requirements"
* add new patch "ice: refactor VSI setup to use parameter structure"
* Fix iRDMA use of hw_addr
* Fix printk format in WARN_ON
* Add Co-developed-by tag for "ice: add a function to initialize vf entry"
* Cleanup use of ice_set_vf_state_qs_dis
* Fix build failures reported by robot

Jacob Keller (13):
  ice: fix function comment referring to ice_vsi_alloc
  ice: drop unnecessary VF parameter from several VSI functions
  ice: refactor VSI setup to use parameter structure
  ice: move vsi_type assignment from ice_vsi_alloc to ice_vsi_cfg
  ice: Fix RDMA latency issue by allowing write-combining
  ice: move ice_vf_vsi_release into ice_vf_lib.c
  ice: Pull common tasks into ice_vf_post_vsi_rebuild
  ice: add a function to initialize vf entry
  ice: introduce ice_vf_init_host_cfg function
  ice: convert vf_ops .vsi_rebuild to .create_vsi
  ice: introduce clear_reset_state operation
  ice: introduce .irq_close VF operation
  ice: remove unnecessary virtchnl_ether_addr struct use

 drivers/infiniband/hw/irdma/main.c            |   2 +-
 drivers/net/ethernet/intel/ice/ice.h          |   4 +-
 drivers/net/ethernet/intel/ice/ice_base.c     |   5 +-
 drivers/net/ethernet/intel/ice/ice_eswitch.c  |  26 ++-
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |   3 +-
 drivers/net/ethernet/intel/ice/ice_lib.c      | 138 ++++++-----
 drivers/net/ethernet/intel/ice/ice_lib.h      |  52 ++++-
 drivers/net/ethernet/intel/ice/ice_main.c     | 219 ++++++++++++++++--
 drivers/net/ethernet/intel/ice/ice_osdep.h    |  48 +++-
 drivers/net/ethernet/intel/ice/ice_sriov.c    | 133 ++++-------
 drivers/net/ethernet/intel/ice/ice_txrx.h     |   2 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |   2 +-
 drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 181 ++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_vf_lib.h   |  12 +-
 .../ethernet/intel/ice/ice_vf_lib_private.h   |   3 +
 drivers/net/ethernet/intel/ice/ice_virtchnl.c |  24 +-
 16 files changed, 621 insertions(+), 233 deletions(-)


base-commit: dd2bcc3ced4c47bead56ad9b728d7d5c3941cae2
-- 
2.39.1.405.gd4c25cc71f83

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

[Intel-wired-lan] [PATCH net-next v2 01/13] ice: fix function comment referring to ice_vsi_alloc

[Jacob Keller]

Since commit 1d2e32275de7 ("ice: split ice_vsi_setup into smaller
functions") ice_vsi_alloc has not been responsible for all of the behavior
implied by the comment for ice_vsi_setup_vector_base.

Fix the comment to refer to the new function ice_vsi_alloc_def().

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
Changes since v1:
* Add Reviewed-by tag

 drivers/net/ethernet/intel/ice/ice_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 04f31a83e327..62d27e50f40e 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1527,7 +1527,7 @@ static int ice_get_vf_ctrl_res(struct ice_pf *pf, struct ice_vsi *vsi)
  * ice_vsi_setup_vector_base - Set up the base vector for the given VSI
  * @vsi: ptr to the VSI
  *
- * This should only be called after ice_vsi_alloc() which allocates the
+ * This should only be called after ice_vsi_alloc_def() which allocates the
  * corresponding SW VSI structure and initializes num_queue_pairs for the
  * newly allocated VSI.
  *
-- 
2.39.1.405.gd4c25cc71f83

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

[Intel-wired-lan] [PATCH net-next v2 02/13] ice: drop unnecessary VF parameter from several VSI functions

[Jacob Keller]

The vsi->vf pointer gets assigned early on during ice_vsi_alloc. Several
functions currently take a VF pointer, but they can just use the existing
vsi->vf pointer as needed. Modify these functions to drop the unnecessary
VF parameter.

Note that ice_vsi_cfg is not changed as a following change will refactor so
that the VF pointer is assigned during ice_vsi_cfg rather than
ice_vsi_alloc.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
Changes since v1:
* Add Reviewed-by tag
* Fix kdoc warning about non-existent function argument

 drivers/net/ethernet/intel/ice/ice_lib.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 62d27e50f40e..46af8b2707b6 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -166,14 +166,14 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi)
 /**
  * ice_vsi_set_num_qs - Set number of queues, descriptors and vectors for a VSI
  * @vsi: the VSI being configured
- * @vf: the VF associated with this VSI, if any
  *
  * Return 0 on success and a negative value on error
  */
-static void ice_vsi_set_num_qs(struct ice_vsi *vsi, struct ice_vf *vf)
+static void ice_vsi_set_num_qs(struct ice_vsi *vsi)
 {
 	enum ice_vsi_type vsi_type = vsi->type;
 	struct ice_pf *pf = vsi->back;
+	struct ice_vf *vf = vsi->vf;
 
 	if (WARN_ON(vsi_type == ICE_VSI_VF && !vf))
 		return;
@@ -594,15 +594,13 @@ static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
 /**
  * ice_vsi_alloc_def - set default values for already allocated VSI
  * @vsi: ptr to VSI
- * @vf: VF for ICE_VSI_VF and ICE_VSI_CTRL
  * @ch: ptr to channel
  */
 static int
-ice_vsi_alloc_def(struct ice_vsi *vsi, struct ice_vf *vf,
-		  struct ice_channel *ch)
+ice_vsi_alloc_def(struct ice_vsi *vsi, struct ice_channel *ch)
 {
 	if (vsi->type != ICE_VSI_CHNL) {
-		ice_vsi_set_num_qs(vsi, vf);
+		ice_vsi_set_num_qs(vsi);
 		if (ice_vsi_alloc_arrays(vsi))
 			return -ENOMEM;
 	}
@@ -2702,14 +2700,11 @@ static int ice_vsi_cfg_tc_lan(struct ice_pf *pf, struct ice_vsi *vsi)
 /**
  * ice_vsi_cfg_def - configure default VSI based on the type
  * @vsi: pointer to VSI
- * @vf: pointer to VF to which this VSI connects. This field is used primarily
- *      for the ICE_VSI_VF type. Other VSI types should pass NULL.
  * @ch: ptr to channel
  * @init_vsi: is this an initialization or a reconfigure of the VSI
  */
 static int
-ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_vf *vf, struct ice_channel *ch,
-		int init_vsi)
+ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_channel *ch, int init_vsi)
 {
 	struct device *dev = ice_pf_to_dev(vsi->back);
 	struct ice_pf *pf = vsi->back;
@@ -2717,7 +2712,7 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_vf *vf, struct ice_channel *ch,
 
 	vsi->vsw = pf->first_sw;
 
-	ret = ice_vsi_alloc_def(vsi, vf, ch);
+	ret = ice_vsi_alloc_def(vsi, ch);
 	if (ret)
 		return ret;
 
@@ -2875,7 +2870,7 @@ int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vf *vf, struct ice_channel *ch,
 {
 	int ret;
 
-	ret = ice_vsi_cfg_def(vsi, vf, ch, init_vsi);
+	ret = ice_vsi_cfg_def(vsi, ch, init_vsi);
 	if (ret)
 		return ret;
 
@@ -3504,7 +3499,7 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, int init_vsi)
 	prev_rxq = vsi->num_rxq;
 
 	ice_vsi_decfg(vsi);
-	ret = ice_vsi_cfg_def(vsi, vsi->vf, vsi->ch, init_vsi);
+	ret = ice_vsi_cfg_def(vsi, vsi->ch, init_vsi);
 	if (ret)
 		goto err_vsi_cfg;
 
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 02/13] ice: drop unnecessary VF parameter from several VSI functions

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 02/13] ice: drop unnecessary VF
> parameter from several VSI functions
> 
> The vsi->vf pointer gets assigned early on during ice_vsi_alloc. Several
> functions currently take a VF pointer, but they can just use the existing
> vsi->vf pointer as needed. Modify these functions to drop the
> vsi->unnecessary
> VF parameter.
> 
> Note that ice_vsi_cfg is not changed as a following change will refactor so
> that the VF pointer is assigned during ice_vsi_cfg rather than ice_vsi_alloc.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
> Changes since v1:
> * Add Reviewed-by tag
> * Fix kdoc warning about non-existent function argument
> 
>  drivers/net/ethernet/intel/ice/ice_lib.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 62d27e50f40e..46af8b2707b6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 03/13] ice: refactor VSI setup to use parameter structure

[Jacob Keller]

The ice_vsi_setup function, ice_vsi_alloc, and ice_vsi_cfg functions have
grown a large number of parameters. These parameters are used to initialize
a new VSI, as well as re-configure an existing VSI

Any time we want to add a new parameter to this function chain, even if it
will usually be unset, we have to change many call sites due to changing
the function signature.

A future change is going to refactor ice_vsi_alloc and ice_vsi_cfg to move
the VSI configuration and initialization all into ice_vsi_cfg.

Before this, refactor the VSI setup flow to use a new ice_vsi_cfg_params
structure. This will contain the configuration (mainly pointers) used to
initialize a VSI.

Pass this from ice_vsi_setup into the related functions such as
ice_vsi_alloc, ice_vsi_cfg, and ice_vsi_cfg_def.

Introduce a helper, ice_vsi_to_params to convert an existing VSI to the
parameters used to initialize it. This will aid in the flows where we
rebuild an existing VSI.

Since we also pass the ICE_VSI_FLAG_INIT to more functions which do not
need (or cannot yet have) the VSI parameters, lets make this clear by
renaming the function parameter to vsi_flags and using a u32 instead of a
signed integer. The name vsi_flags also makes it clear that we may extend
the flags in the future.

This change will make it easier to refactor the setup flow in the future,
and will reduce the complexity required to add a new parameter for
configuration in the future.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes since v1:
* New patch to introduce ice_vsi_cfg_params structure

 drivers/net/ethernet/intel/ice/ice_eswitch.c |  8 +-
 drivers/net/ethernet/intel/ice/ice_lib.c     | 92 ++++++++++----------
 drivers/net/ethernet/intel/ice/ice_lib.h     | 52 +++++++++--
 drivers/net/ethernet/intel/ice/ice_main.c    | 40 +++++++--
 drivers/net/ethernet/intel/ice/ice_sriov.c   |  9 +-
 drivers/net/ethernet/intel/ice/ice_vf_lib.c  |  9 +-
 6 files changed, 147 insertions(+), 63 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
index f9f15acae90a..b86d173a20af 100644
--- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
+++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
@@ -425,7 +425,13 @@ static void ice_eswitch_release_env(struct ice_pf *pf)
 static struct ice_vsi *
 ice_eswitch_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
 {
-	return ice_vsi_setup(pf, pi, ICE_VSI_SWITCHDEV_CTRL, NULL, NULL);
+	struct ice_vsi_cfg_params params = {};
+
+	params.type = ICE_VSI_SWITCHDEV_CTRL;
+	params.pi = pi;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	return ice_vsi_setup(pf, &params);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 46af8b2707b6..1a2af5a9cffe 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -639,10 +639,7 @@ ice_vsi_alloc_def(struct ice_vsi *vsi, struct ice_channel *ch)
 /**
  * ice_vsi_alloc - Allocates the next available struct VSI in the PF
  * @pf: board private structure
- * @pi: pointer to the port_info instance
- * @vsi_type: type of VSI
- * @ch: ptr to channel
- * @vf: VF for ICE_VSI_VF and ICE_VSI_CTRL
+ * @params: parameters to use when allocating the new VSI
  *
  * The VF pointer is used for ICE_VSI_VF and ICE_VSI_CTRL. For ICE_VSI_CTRL,
  * it may be NULL in the case there is no association with a VF. For
@@ -651,14 +648,12 @@ ice_vsi_alloc_def(struct ice_vsi *vsi, struct ice_channel *ch)
  * returns a pointer to a VSI on success, NULL on failure.
  */
 static struct ice_vsi *
-ice_vsi_alloc(struct ice_pf *pf, struct ice_port_info *pi,
-	      enum ice_vsi_type vsi_type, struct ice_channel *ch,
-	      struct ice_vf *vf)
+ice_vsi_alloc(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
 {
 	struct device *dev = ice_pf_to_dev(pf);
 	struct ice_vsi *vsi = NULL;
 
-	if (WARN_ON(vsi_type == ICE_VSI_VF && !vf))
+	if (WARN_ON(params->type == ICE_VSI_VF && !params->vf))
 		return NULL;
 
 	/* Need to protect the allocation of the VSIs at the PF level */
@@ -677,11 +672,11 @@ ice_vsi_alloc(struct ice_pf *pf, struct ice_port_info *pi,
 	if (!vsi)
 		goto unlock_pf;
 
-	vsi->type = vsi_type;
+	vsi->type = params->type;
 	vsi->back = pf;
-	vsi->port_info = pi;
+	vsi->port_info = params->pi;
 	/* For VSIs which don't have a connected VF, this will be NULL */
-	vsi->vf = vf;
+	vsi->vf = params->vf;
 	set_bit(ICE_VSI_DOWN, vsi->state);
 
 	/* fill slot and make note of the index */
@@ -693,8 +688,9 @@ ice_vsi_alloc(struct ice_pf *pf, struct ice_port_info *pi,
 					 pf->next_vsi);
 
 	if (vsi->type == ICE_VSI_CTRL) {
-		if (vf) {
-			vf->ctrl_vsi_idx = vsi->idx;
+		if (vsi->vf) {
+			WARN_ON(vsi->vf->ctrl_vsi_idx != ICE_NO_VSI);
+			vsi->vf->ctrl_vsi_idx = vsi->idx;
 		} else {
 			WARN_ON(pf->ctrl_vsi_idx != ICE_NO_VSI);
 			pf->ctrl_vsi_idx = vsi->idx;
@@ -1265,12 +1261,15 @@ ice_chnl_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt)
 /**
  * ice_vsi_init - Create and initialize a VSI
  * @vsi: the VSI being configured
- * @init_vsi: flag, tell if VSI need to be initialized
+ * @vsi_flags: VSI configuration flags
+ *
+ * Set ICE_FLAG_VSI_INIT to initialize a new VSI context, clear it to
+ * reconfigure an existing context.
  *
  * This initializes a VSI context depending on the VSI type to be added and
  * passes it down to the add_vsi aq command to create a new VSI.
  */
-static int ice_vsi_init(struct ice_vsi *vsi, int init_vsi)
+static int ice_vsi_init(struct ice_vsi *vsi, u32 vsi_flags)
 {
 	struct ice_pf *pf = vsi->back;
 	struct ice_hw *hw = &pf->hw;
@@ -1332,7 +1331,7 @@ static int ice_vsi_init(struct ice_vsi *vsi, int init_vsi)
 		/* if updating VSI context, make sure to set valid_section:
 		 * to indicate which section of VSI context being updated
 		 */
-		if (!(init_vsi & ICE_VSI_FLAG_INIT))
+		if (!(vsi_flags & ICE_VSI_FLAG_INIT))
 			ctxt->info.valid_sections |=
 				cpu_to_le16(ICE_AQ_VSI_PROP_Q_OPT_VALID);
 	}
@@ -1345,7 +1344,7 @@ static int ice_vsi_init(struct ice_vsi *vsi, int init_vsi)
 		if (ret)
 			goto out;
 
-		if (!(init_vsi & ICE_VSI_FLAG_INIT))
+		if (!(vsi_flags & ICE_VSI_FLAG_INIT))
 			/* means VSI being updated */
 			/* must to indicate which section of VSI context are
 			 * being modified
@@ -1361,7 +1360,7 @@ static int ice_vsi_init(struct ice_vsi *vsi, int init_vsi)
 			cpu_to_le16(ICE_AQ_VSI_PROP_SECURITY_VALID);
 	}
 
-	if (init_vsi & ICE_VSI_FLAG_INIT) {
+	if (vsi_flags & ICE_VSI_FLAG_INIT) {
 		ret = ice_add_vsi(hw, vsi->idx, ctxt, NULL);
 		if (ret) {
 			dev_err(dev, "Add VSI failed, err %d\n", ret);
@@ -2700,11 +2699,10 @@ static int ice_vsi_cfg_tc_lan(struct ice_pf *pf, struct ice_vsi *vsi)
 /**
  * ice_vsi_cfg_def - configure default VSI based on the type
  * @vsi: pointer to VSI
- * @ch: ptr to channel
- * @init_vsi: is this an initialization or a reconfigure of the VSI
+ * @params: the parameters to configure this VSI with
  */
 static int
-ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_channel *ch, int init_vsi)
+ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
 {
 	struct device *dev = ice_pf_to_dev(vsi->back);
 	struct ice_pf *pf = vsi->back;
@@ -2712,7 +2710,7 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_channel *ch, int init_vsi)
 
 	vsi->vsw = pf->first_sw;
 
-	ret = ice_vsi_alloc_def(vsi, ch);
+	ret = ice_vsi_alloc_def(vsi, params->ch);
 	if (ret)
 		return ret;
 
@@ -2735,7 +2733,7 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_channel *ch, int init_vsi)
 	ice_vsi_set_tc_cfg(vsi);
 
 	/* create the VSI */
-	ret = ice_vsi_init(vsi, init_vsi);
+	ret = ice_vsi_init(vsi, params->flags);
 	if (ret)
 		goto unroll_get_qs;
 
@@ -2860,17 +2858,13 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_channel *ch, int init_vsi)
 /**
  * ice_vsi_cfg - configure VSI and tc on it
  * @vsi: pointer to VSI
- * @vf: pointer to VF to which this VSI connects. This field is used primarily
- *      for the ICE_VSI_VF type. Other VSI types should pass NULL.
- * @ch: ptr to channel
- * @init_vsi: is this an initialization or a reconfigure of the VSI
+ * @params: parameters used to configure this VSI
  */
-int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vf *vf, struct ice_channel *ch,
-		int init_vsi)
+int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
 {
 	int ret;
 
-	ret = ice_vsi_cfg_def(vsi, ch, init_vsi);
+	ret = ice_vsi_cfg_def(vsi, params);
 	if (ret)
 		return ret;
 
@@ -2941,11 +2935,7 @@ void ice_vsi_decfg(struct ice_vsi *vsi)
 /**
  * ice_vsi_setup - Set up a VSI by a given type
  * @pf: board private structure
- * @pi: pointer to the port_info instance
- * @vsi_type: VSI type
- * @vf: pointer to VF to which this VSI connects. This field is used primarily
- *      for the ICE_VSI_VF type. Other VSI types should pass NULL.
- * @ch: ptr to channel
+ * @params: parameters to use when creating the VSI
  *
  * This allocates the sw VSI structure and its queue resources.
  *
@@ -2953,21 +2943,26 @@ void ice_vsi_decfg(struct ice_vsi *vsi)
  * success, NULL on failure.
  */
 struct ice_vsi *
-ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
-	      enum ice_vsi_type vsi_type, struct ice_vf *vf,
-	      struct ice_channel *ch)
+ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
 {
 	struct device *dev = ice_pf_to_dev(pf);
 	struct ice_vsi *vsi;
 	int ret;
 
-	vsi = ice_vsi_alloc(pf, pi, vsi_type, ch, vf);
+	/* ice_vsi_setup can only initialize a new VSI, and we must have
+	 * a port_info structure for it.
+	 */
+	if (WARN_ON(!(params->flags & ICE_VSI_FLAG_INIT)) ||
+	    WARN_ON(!params->pi))
+		return NULL;
+
+	vsi = ice_vsi_alloc(pf, params);
 	if (!vsi) {
 		dev_err(dev, "could not allocate VSI\n");
 		return NULL;
 	}
 
-	ret = ice_vsi_cfg(vsi, vf, ch, ICE_VSI_FLAG_INIT);
+	ret = ice_vsi_cfg(vsi, params);
 	if (ret)
 		goto err_vsi_cfg;
 
@@ -2992,7 +2987,7 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
 	return vsi;
 
 err_vsi_cfg:
-	if (vsi_type == ICE_VSI_VF)
+	if (params->type == ICE_VSI_VF)
 		ice_enable_lag(pf->lag);
 	ice_vsi_free(vsi);
 
@@ -3470,12 +3465,16 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq)
 /**
  * ice_vsi_rebuild - Rebuild VSI after reset
  * @vsi: VSI to be rebuild
- * @init_vsi: flag, tell if VSI need to be initialized
+ * @vsi_flags: flags used for VSI rebuild flow
+ *
+ * Set vsi_flags to ICE_VSI_FLAG_INIT to initialize a new VSI, or
+ * ICE_VSI_FLAG_NO_INIT to rebuild an existing VSI in hardware.
  *
  * Returns 0 on success and negative value on failure
  */
-int ice_vsi_rebuild(struct ice_vsi *vsi, int init_vsi)
+int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
 {
+	struct ice_vsi_cfg_params params = {};
 	struct ice_coalesce_stored *coalesce;
 	int ret, prev_txq, prev_rxq;
 	int prev_num_q_vectors = 0;
@@ -3484,6 +3483,9 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, int init_vsi)
 	if (!vsi)
 		return -EINVAL;
 
+	params = ice_vsi_to_params(vsi);
+	params.flags = vsi_flags;
+
 	pf = vsi->back;
 	if (WARN_ON(vsi->type == ICE_VSI_VF && !vsi->vf))
 		return -EINVAL;
@@ -3499,13 +3501,13 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, int init_vsi)
 	prev_rxq = vsi->num_rxq;
 
 	ice_vsi_decfg(vsi);
-	ret = ice_vsi_cfg_def(vsi, vsi->ch, init_vsi);
+	ret = ice_vsi_cfg_def(vsi, &params);
 	if (ret)
 		goto err_vsi_cfg;
 
 	ret = ice_vsi_cfg_tc_lan(pf, vsi);
 	if (ret) {
-		if (init_vsi & ICE_VSI_FLAG_INIT) {
+		if (vsi_flags & ICE_VSI_FLAG_INIT) {
 			ret = -EIO;
 			goto err_vsi_cfg_tc_lan;
 		} else {
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
index b76f05e1f8a3..75221478f2dc 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -7,6 +7,47 @@
 #include "ice.h"
 #include "ice_vlan.h"
 
+/* Flags used for VSI configuration and rebuild */
+#define ICE_VSI_FLAG_INIT	BIT(0)
+#define ICE_VSI_FLAG_NO_INIT	0
+
+/**
+ * struct ice_vsi_cfg_params - VSI configuration parameters
+ * @pi: pointer to the port_info instance for the VSI
+ * @ch: pointer to the channel structure for the VSI, may be NULL
+ * @vf: pointer to the VF associated with this VSI, may be NULL
+ * @type: the type of VSI to configure
+ * @flags: VSI flags used for rebuild and configuration
+ *
+ * Parameter structure used when configuring a new VSI.
+ */
+struct ice_vsi_cfg_params {
+	struct ice_port_info *pi;
+	struct ice_channel *ch;
+	struct ice_vf *vf;
+	enum ice_vsi_type type;
+	u32 flags;
+};
+
+/**
+ * ice_vsi_to_params - Get parameters for an existing VSI
+ * @vsi: the VSI to get parameters for
+ *
+ * Fill a parameter structure for reconfiguring a VSI with its current
+ * parameters, such as during a rebuild operation.
+ */
+static inline struct ice_vsi_cfg_params ice_vsi_to_params(struct ice_vsi *vsi)
+{
+	struct ice_vsi_cfg_params params = {};
+
+	params.pi = vsi->port_info;
+	params.ch = vsi->ch;
+	params.vf = vsi->vf;
+	params.type = vsi->type;
+
+	return params;
+}
+
 const char *ice_vsi_type_str(enum ice_vsi_type vsi_type);
 
 bool ice_pf_state_is_nominal(struct ice_pf *pf);
@@ -50,9 +91,7 @@ int ice_vsi_cfg_rss_lut_key(struct ice_vsi *vsi);
 void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
 
 struct ice_vsi *
-ice_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
-	      enum ice_vsi_type vsi_type, struct ice_vf *vf,
-	      struct ice_channel *ch);
+ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params);
 
 void ice_napi_del(struct ice_vsi *vsi);
 
@@ -70,11 +109,8 @@ int ice_free_res(struct ice_res_tracker *res, u16 index, u16 id);
 int
 ice_get_res(struct ice_pf *pf, struct ice_res_tracker *res, u16 needed, u16 id);
 
-#define ICE_VSI_FLAG_INIT	BIT(0)
-#define ICE_VSI_FLAG_NO_INIT	0
-int ice_vsi_rebuild(struct ice_vsi *vsi, int init_vsi);
-int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vf *vf,
-		struct ice_channel *ch, int init_vsi);
+int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags);
+int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params);
 
 bool ice_is_reset_in_progress(unsigned long *state);
 int ice_wait_for_reset(struct ice_pf *pf, unsigned long timeout);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 8fd9c87f30e2..4165fde0106d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3444,14 +3444,27 @@ void ice_fill_rss_lut(u8 *lut, u16 rss_table_size, u16 rss_size)
 static struct ice_vsi *
 ice_pf_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
 {
-	return ice_vsi_setup(pf, pi, ICE_VSI_PF, NULL, NULL);
+	struct ice_vsi_cfg_params params = {};
+
+	params.type = ICE_VSI_PF;
+	params.pi = pi;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	return ice_vsi_setup(pf, &params);
 }
 
 static struct ice_vsi *
 ice_chnl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
 		   struct ice_channel *ch)
 {
-	return ice_vsi_setup(pf, pi, ICE_VSI_CHNL, NULL, ch);
+	struct ice_vsi_cfg_params params = {};
+
+	params.type = ICE_VSI_CHNL;
+	params.pi = pi;
+	params.ch = ch;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	return ice_vsi_setup(pf, &params);
 }
 
 /**
@@ -3465,7 +3478,13 @@ ice_chnl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi,
 static struct ice_vsi *
 ice_ctrl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
 {
-	return ice_vsi_setup(pf, pi, ICE_VSI_CTRL, NULL, NULL);
+	struct ice_vsi_cfg_params params = {};
+
+	params.type = ICE_VSI_CTRL;
+	params.pi = pi;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	return ice_vsi_setup(pf, &params);
 }
 
 /**
@@ -3479,7 +3498,13 @@ ice_ctrl_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
 struct ice_vsi *
 ice_lb_vsi_setup(struct ice_pf *pf, struct ice_port_info *pi)
 {
-	return ice_vsi_setup(pf, pi, ICE_VSI_LB, NULL, NULL);
+	struct ice_vsi_cfg_params params = {};
+
+	params.type = ICE_VSI_LB;
+	params.pi = pi;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	return ice_vsi_setup(pf, &params);
 }
 
 /**
@@ -4998,6 +5023,7 @@ static void ice_deinit(struct ice_pf *pf)
  */
 int ice_load(struct ice_pf *pf)
 {
+	struct ice_vsi_cfg_params params = {};
 	struct ice_vsi *vsi;
 	int err;
 
@@ -5010,7 +5036,11 @@ int ice_load(struct ice_pf *pf)
 		return err;
 
 	vsi = ice_get_main_vsi(pf);
-	err = ice_vsi_cfg(vsi, NULL, NULL, ICE_VSI_FLAG_INIT);
+
+	params = ice_vsi_to_params(vsi);
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	err = ice_vsi_cfg(vsi, &params);
 	if (err)
 		goto err_vsi_cfg;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 3ba1408c56a9..a101768b1cc5 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -248,11 +248,16 @@ void ice_free_vfs(struct ice_pf *pf)
  */
 static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
 {
-	struct ice_port_info *pi = ice_vf_get_port_info(vf);
+	struct ice_vsi_cfg_params params = {};
 	struct ice_pf *pf = vf->pf;
 	struct ice_vsi *vsi;
 
-	vsi = ice_vsi_setup(pf, pi, ICE_VSI_VF, vf, NULL);
+	params.type = ICE_VSI_VF;
+	params.pi = ice_vf_get_port_info(vf);
+	params.vf = vf;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	vsi = ice_vsi_setup(pf, &params);
 
 	if (!vsi) {
 		dev_err(ice_pf_to_dev(pf), "Failed to create VF VSI\n");
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index c3b406df269f..1b7e919b9275 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -1115,11 +1115,16 @@ void ice_vf_ctrl_vsi_release(struct ice_vf *vf)
  */
 struct ice_vsi *ice_vf_ctrl_vsi_setup(struct ice_vf *vf)
 {
-	struct ice_port_info *pi = ice_vf_get_port_info(vf);
+	struct ice_vsi_cfg_params params = {};
 	struct ice_pf *pf = vf->pf;
 	struct ice_vsi *vsi;
 
-	vsi = ice_vsi_setup(pf, pi, ICE_VSI_CTRL, vf, NULL);
+	params.type = ICE_VSI_CTRL;
+	params.pi = ice_vf_get_port_info(vf);
+	params.vf = vf;
+	params.flags = ICE_VSI_FLAG_INIT;
+
+	vsi = ice_vsi_setup(pf, &params);
 	if (!vsi) {
 		dev_err(ice_pf_to_dev(pf), "Failed to create VF control VSI\n");
 		ice_vf_ctrl_invalidate_vsi(vf);
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 03/13] ice: refactor VSI setup to use parameter structure

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Thursday, January 19, 2023 6:47 AM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 03/13] ice: refactor VSI setup
> to use parameter structure
> 
> The ice_vsi_setup function, ice_vsi_alloc, and ice_vsi_cfg functions have
> grown a large number of parameters. These parameters are used to initialize
> a new VSI, as well as re-configure an existing VSI
> 
> Any time we want to add a new parameter to this function chain, even if it
> will usually be unset, we have to change many call sites due to changing the
> function signature.
> 
> A future change is going to refactor ice_vsi_alloc and ice_vsi_cfg to move the
> VSI configuration and initialization all into ice_vsi_cfg.
> 
> Before this, refactor the VSI setup flow to use a new ice_vsi_cfg_params
> structure. This will contain the configuration (mainly pointers) used to
> initialize a VSI.
> 
> Pass this from ice_vsi_setup into the related functions such as ice_vsi_alloc,
> ice_vsi_cfg, and ice_vsi_cfg_def.
> 
> Introduce a helper, ice_vsi_to_params to convert an existing VSI to the
> parameters used to initialize it. This will aid in the flows where we rebuild an
> existing VSI.
> 
> Since we also pass the ICE_VSI_FLAG_INIT to more functions which do not
> need (or cannot yet have) the VSI parameters, lets make this clear by
> renaming the function parameter to vsi_flags and using a u32 instead of a
> signed integer. The name vsi_flags also makes it clear that we may extend
> the flags in the future.
> 
> This change will make it easier to refactor the setup flow in the future, and
> will reduce the complexity required to add a new parameter for
> configuration in the future.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Changes since v1:
> * New patch to introduce ice_vsi_cfg_params structure
> 
>  drivers/net/ethernet/intel/ice/ice_eswitch.c |  8 +-
>  drivers/net/ethernet/intel/ice/ice_lib.c     | 92 ++++++++++----------
>  drivers/net/ethernet/intel/ice/ice_lib.h     | 52 +++++++++--
>  drivers/net/ethernet/intel/ice/ice_main.c    | 40 +++++++--
>  drivers/net/ethernet/intel/ice/ice_sriov.c   |  9 +-
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c  |  9 +-
>  6 files changed, 147 insertions(+), 63 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 04/13] ice: move vsi_type assignment from ice_vsi_alloc to ice_vsi_cfg

[Jacob Keller]

The ice_vsi_alloc and ice_vsi_cfg functions are used together to allocate
and configure a new VSI, called as part of the ice_vsi_setup function.

In the future with the addition of the subfunction code the ice driver
will want to be able to allocate a VSI while delaying the configuration to
a later point of the port activation.

Currently this requires that the port code know what type of VSI should
be allocated. This is required because ice_vsi_alloc assigns the VSI type.

Refactor the ice_vsi_alloc and ice_vsi_cfg functions so that VSI type
assignment isn't done until the configuration stage. This will allow the
devlink port addition logic to reserve a VSI as early as possible before
the type of the port is known. In this way, the port add can fail in the
event that all hardware VSI resources are exhausted.

Since the ice_vsi_cfg function already takes the ice_vsi_cfg_params
structure, this is relatively straight forward.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes since v1:
* Re-order after new patch
* Changed to take into account ice_vsi_cfg_params

 drivers/net/ethernet/intel/ice/ice_lib.c | 51 ++++++++++++------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1a2af5a9cffe..e2f0b28a89d7 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -639,23 +639,18 @@ ice_vsi_alloc_def(struct ice_vsi *vsi, struct ice_channel *ch)
 /**
  * ice_vsi_alloc - Allocates the next available struct VSI in the PF
  * @pf: board private structure
- * @params: parameters to use when allocating the new VSI
  *
- * The VF pointer is used for ICE_VSI_VF and ICE_VSI_CTRL. For ICE_VSI_CTRL,
- * it may be NULL in the case there is no association with a VF. For
- * ICE_VSI_VF the VF pointer *must not* be NULL.
+ * Reserves a VSI index from the PF and allocates an empty VSI structure
+ * without a type. The VSI structure must later be initialized by calling
+ * ice_vsi_cfg().
  *
  * returns a pointer to a VSI on success, NULL on failure.
  */
-static struct ice_vsi *
-ice_vsi_alloc(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
+static struct ice_vsi *ice_vsi_alloc(struct ice_pf *pf)
 {
 	struct device *dev = ice_pf_to_dev(pf);
 	struct ice_vsi *vsi = NULL;
 
-	if (WARN_ON(params->type == ICE_VSI_VF && !params->vf))
-		return NULL;
-
 	/* Need to protect the allocation of the VSIs at the PF level */
 	mutex_lock(&pf->sw_mutex);
 
@@ -672,11 +667,7 @@ ice_vsi_alloc(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
 	if (!vsi)
 		goto unlock_pf;
 
-	vsi->type = params->type;
 	vsi->back = pf;
-	vsi->port_info = params->pi;
-	/* For VSIs which don't have a connected VF, this will be NULL */
-	vsi->vf = params->vf;
 	set_bit(ICE_VSI_DOWN, vsi->state);
 
 	/* fill slot and make note of the index */
@@ -687,16 +678,6 @@ ice_vsi_alloc(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
 	pf->next_vsi = ice_get_free_slot(pf->vsi, pf->num_alloc_vsi,
 					 pf->next_vsi);
 
-	if (vsi->type == ICE_VSI_CTRL) {
-		if (vsi->vf) {
-			WARN_ON(vsi->vf->ctrl_vsi_idx != ICE_NO_VSI);
-			vsi->vf->ctrl_vsi_idx = vsi->idx;
-		} else {
-			WARN_ON(pf->ctrl_vsi_idx != ICE_NO_VSI);
-			pf->ctrl_vsi_idx = vsi->idx;
-		}
-	}
-
 unlock_pf:
 	mutex_unlock(&pf->sw_mutex);
 	return vsi;
@@ -2856,14 +2837,24 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
 }
 
 /**
- * ice_vsi_cfg - configure VSI and tc on it
+ * ice_vsi_cfg - configure a previously allocated VSI
  * @vsi: pointer to VSI
  * @params: parameters used to configure this VSI
  */
 int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
 {
+	struct ice_pf *pf = vsi->back;
 	int ret;
 
+	if (WARN_ON(params->type == ICE_VSI_VF && !params->vf))
+		return -EINVAL;
+
+	vsi->type = params->type;
+	vsi->port_info = params->pi;
+
+	/* For VSIs which don't have a connected VF, this will be NULL */
+	vsi->vf = params->vf;
+
 	ret = ice_vsi_cfg_def(vsi, params);
 	if (ret)
 		return ret;
@@ -2872,6 +2863,16 @@ int ice_vsi_cfg(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
 	if (ret)
 		ice_vsi_decfg(vsi);
 
+	if (vsi->type == ICE_VSI_CTRL) {
+		if (vsi->vf) {
+			WARN_ON(vsi->vf->ctrl_vsi_idx != ICE_NO_VSI);
+			vsi->vf->ctrl_vsi_idx = vsi->idx;
+		} else {
+			WARN_ON(pf->ctrl_vsi_idx != ICE_NO_VSI);
+			pf->ctrl_vsi_idx = vsi->idx;
+		}
+	}
+
 	return ret;
 }
 
@@ -2956,7 +2957,7 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
 	    WARN_ON(!params->pi))
 		return NULL;
 
-	vsi = ice_vsi_alloc(pf, params);
+	vsi = ice_vsi_alloc(pf);
 	if (!vsi) {
 		dev_err(dev, "could not allocate VSI\n");
 		return NULL;
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 04/13] ice: move vsi_type assignment from ice_vsi_alloc to ice_vsi_cfg

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Thursday, January 19, 2023 6:47 AM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 04/13] ice: move vsi_type
> assignment from ice_vsi_alloc to ice_vsi_cfg
> 
> The ice_vsi_alloc and ice_vsi_cfg functions are used together to allocate and
> configure a new VSI, called as part of the ice_vsi_setup function.
> 
> In the future with the addition of the subfunction code the ice driver will
> want to be able to allocate a VSI while delaying the configuration to a later
> point of the port activation.
> 
> Currently this requires that the port code know what type of VSI should be
> allocated. This is required because ice_vsi_alloc assigns the VSI type.
> 
> Refactor the ice_vsi_alloc and ice_vsi_cfg functions so that VSI type
> assignment isn't done until the configuration stage. This will allow the devlink
> port addition logic to reserve a VSI as early as possible before the type of the
> port is known. In this way, the port add can fail in the event that all hardware
> VSI resources are exhausted.
> 
> Since the ice_vsi_cfg function already takes the ice_vsi_cfg_params
> structure, this is relatively straight forward.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Changes since v1:
> * Re-order after new patch
> * Changed to take into account ice_vsi_cfg_params
> 
>  drivers/net/ethernet/intel/ice/ice_lib.c | 51 ++++++++++++------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency issue by allowing write-combining

[Jacob Keller]

The current method of mapping the entire BAR region as a single uncacheable
region does not allow RDMA to use write combining (WC). This results in
increased latency with RDMA.

To fix this, we initially planned to reduce the size of the map made by the
PF driver to include only up to the beginning of the RDMA space.
Unfortunately this will not work in the future as there are some hardware
features which use registers beyond the RDMA area. This includes Scalable
IOV, a virtualization feature being worked on currently.

Instead of simply reducing the size of the map, we need a solution which
will allow access to all areas of the address space while leaving the RDMA
area open to be mapped with write combining.

To allow for this, and fix the RMDA latency issue without blocking the
higher areas of the BAR, we need to create multiple separate memory maps.
Doing so will create a sparse mapping rather than a contiguous single area.

Replace the void *hw_addr with a special ice_hw_addr structure which
represents the multiple mappings as a flexible array.

Based on the available BAR size, map up to 3 regions:

 * The space before the RDMA section
 * The RDMA section which wants write combining behavior
 * The space after the RDMA section

Add an ice_get_hw_addr function which converts a register offset into the
appropriate kernel address based on which chunk it falls into. This does
cost us slightly more computation overhead for register access as we now
must check the table each access. However, we can pre-compute the addresses
where this would most be a problem.

With this change, the RDMA driver is now free to map the RDMA register
section as write-combined without impacting access to other device
registers used by the main PF driver.

Reported-by: Dave Ertman <david.m.ertman@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes since v1:
* Export ice_get_hw_addr
* Use ice_get_hw_addr in iRDMA driver
* Fix the WARN_ON to use %pa instead of %llx for printing a resource_size_t

 drivers/infiniband/hw/irdma/main.c           |   2 +-
 drivers/net/ethernet/intel/ice/ice.h         |   4 +-
 drivers/net/ethernet/intel/ice/ice_base.c    |   5 +-
 drivers/net/ethernet/intel/ice/ice_ethtool.c |   3 +-
 drivers/net/ethernet/intel/ice/ice_main.c    | 177 +++++++++++++++++--
 drivers/net/ethernet/intel/ice/ice_osdep.h   |  48 ++++-
 drivers/net/ethernet/intel/ice/ice_txrx.h    |   2 +-
 drivers/net/ethernet/intel/ice/ice_type.h    |   2 +-
 8 files changed, 219 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c
index 514453777e07..37a2650abbbb 100644
--- a/drivers/infiniband/hw/irdma/main.c
+++ b/drivers/infiniband/hw/irdma/main.c
@@ -228,7 +228,7 @@ static void irdma_fill_device_info(struct irdma_device *iwdev, struct ice_pf *pf
 	rf->cdev = pf;
 	rf->gen_ops.register_qset = irdma_lan_register_qset;
 	rf->gen_ops.unregister_qset = irdma_lan_unregister_qset;
-	rf->hw.hw_addr = pf->hw.hw_addr;
+	rf->hw.hw_addr = ice_get_hw_addr(&pf->hw, 0);
 	rf->pcidev = pf->pdev;
 	rf->msix_count =  pf->num_rdma_msix;
 	rf->pf_id = pf->hw.pf_id;
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 51a1a89f7b5a..cd81974822cc 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -75,7 +75,9 @@
 #include "ice_vsi_vlan_ops.h"
 #include "ice_gnss.h"
 
-#define ICE_BAR0		0
+#define ICE_BAR0			0
+#define ICE_BAR_RDMA_WC_START		0x0800000
+#define ICE_BAR_RDMA_WC_END		0x1000000
 #define ICE_REQ_DESC_MULTIPLE	32
 #define ICE_MIN_NUM_DESC	64
 #define ICE_MAX_NUM_DESC	8160
diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
index 554095b25f44..332d5a1b326c 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -480,7 +480,7 @@ static int ice_setup_rx_ctx(struct ice_rx_ring *ring)
 	ring->rx_offset = ice_rx_offset(ring);
 
 	/* init queue specific tail register */
-	ring->tail = hw->hw_addr + QRX_TAIL(pf_q);
+	ring->tail = ice_get_hw_addr(hw, QRX_TAIL(pf_q));
 	writel(0, ring->tail);
 
 	return 0;
@@ -790,8 +790,7 @@ ice_vsi_cfg_txq(struct ice_vsi *vsi, struct ice_tx_ring *ring,
 	/* init queue specific tail reg. It is referred as
 	 * transmit comm scheduler queue doorbell.
 	 */
-	ring->tail = hw->hw_addr + QTX_COMM_DBELL(pf_q);
-
+	ring->tail = ice_get_hw_addr(hw, QTX_COMM_DBELL(pf_q));
 	if (IS_ENABLED(CONFIG_DCB))
 		tc = ring->dcb_tc;
 	else
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 936f0e0c553d..b54f470be8d7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3085,7 +3085,8 @@ ice_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring,
 		/* this is to allow wr32 to have something to write to
 		 * during early allocation of Rx buffers
 		 */
-		rx_rings[i].tail = vsi->back->hw.hw_addr + PRTGEN_STATUS;
+		rx_rings[i].tail = ice_get_hw_addr(&vsi->back->hw,
+						   PRTGEN_STATUS);
 
 		err = ice_setup_rx_ring(&rx_rings[i]);
 		if (err)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 4165fde0106d..3b98721fd9d8 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -596,6 +596,163 @@ ice_prepare_for_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
 	set_bit(ICE_PREPARED_FOR_RESET, pf->state);
 }
 
+/**
+ * ice_get_hw_addr - Get memory address for a given device register
+ * @hw: pointer to the HW struct
+ * @reg: the register to get address of
+ *
+ * Convert a register offset into the appropriate memory mapped kernel
+ * address.
+ *
+ * Returns the pointer address or an ERR_PTR on failure.
+ */
+void __iomem *ice_get_hw_addr(struct ice_hw *hw, resource_size_t reg)
+{
+	struct ice_hw_addr *hw_addr = (struct ice_hw_addr *)hw->hw_addr;
+	struct ice_hw_addr_map *map;
+	unsigned int i;
+
+	if (WARN_ON(!hw_addr))
+		return (void __iomem *)ERR_PTR(-EIO);
+
+	for (i = 0, map = hw_addr->maps; i < hw_addr->nr; i++, map++)
+		if (reg >= map->start && reg < map->end)
+			return (u8 __iomem *)map->addr + (reg - map->start);
+
+	WARN_ONCE(1, "Unable to map register address %pa to kernel address",
+		  &reg);
+
+	return (void __iomem *)ERR_PTR(-EFAULT);
+}
+EXPORT_SYMBOL_GPL(ice_get_hw_addr);
+
+/**
+ * ice_map_hw_addr - map a region of device registers to memory
+ * @pdev: the PCI device
+ * @map: the address map structure
+ *
+ * Map the specified section of the hardware registers into memory, storing
+ * the memory mapped address in the provided structure.
+ *
+ * Returns 0 on success or an error code on failure.
+ */
+static int ice_map_hw_addr(struct pci_dev *pdev, struct ice_hw_addr_map *map)
+{
+	struct device *dev = &pdev->dev;
+	resource_size_t size, base;
+	void __iomem *addr;
+
+	if (WARN_ON(map->end <= map->start))
+		return -EIO;
+
+	size = map->end - map->start;
+	base = pci_resource_start(pdev, map->bar) + map->start;
+	addr = ioremap(base, size);
+	if (!addr) {
+		dev_err(dev, "%s: remap at offset %llu failed\n",
+			__func__, map->start);
+		return -EIO;
+	}
+
+	map->addr = addr;
+
+	return 0;
+}
+
+/**
+ * ice_map_all_hw_addr - Request and map PCI BAR memory
+ * @pf: pointer to the PF structure
+ *
+ * Request and reserve all PCI BAR regions. Memory map chunks of the PCI BAR
+ * 0 into a sparse memory map to allow the RDMA region to be mapped with write
+ * combining.
+ *
+ * Returns 0 on success or an error code on failure.
+ */
+static int ice_map_all_hw_addr(struct ice_pf *pf)
+{
+	struct pci_dev *pdev = pf->pdev;
+	struct device *dev = &pdev->dev;
+	struct ice_hw_addr *hw_addr;
+	resource_size_t bar_len;
+	unsigned int nr_maps;
+	int err;
+
+	bar_len = pci_resource_len(pdev, 0);
+	if (bar_len > ICE_BAR_RDMA_WC_END)
+		nr_maps = 2;
+	else
+		nr_maps = 1;
+
+	hw_addr = kzalloc(struct_size(hw_addr, maps, nr_maps), GFP_KERNEL);
+	if (!hw_addr)
+		return -ENOMEM;
+
+	hw_addr->nr = nr_maps;
+
+	err = pci_request_mem_regions(pdev, dev_driver_string(dev));
+	if (err) {
+		dev_err(dev, "pci_request_mem_regions failed, err %pe\n",
+			ERR_PTR(err));
+		goto err_free_hw_addr;
+	}
+
+	/* Map the start of the BAR as uncachable */
+	hw_addr->maps[0].bar = 0;
+	hw_addr->maps[0].start = 0;
+	hw_addr->maps[0].end = min_t(resource_size_t, bar_len,
+				     ICE_BAR_RDMA_WC_START);
+	err = ice_map_hw_addr(pdev, &hw_addr->maps[0]);
+	if (err)
+		goto err_release_mem_regions;
+
+	/* Map everything past the RDMA section as uncachable */
+	if (nr_maps > 1) {
+		hw_addr->maps[1].bar = 0;
+		hw_addr->maps[1].start = ICE_BAR_RDMA_WC_END;
+		hw_addr->maps[1].end = bar_len;
+		err = ice_map_hw_addr(pdev, &hw_addr->maps[1]);
+		if (err)
+			goto err_unmap_bar_start;
+	}
+
+	pf->hw.hw_addr = (typeof(pf->hw.hw_addr))hw_addr;
+
+	return 0;
+
+err_unmap_bar_start:
+	iounmap(hw_addr->maps[0].addr);
+err_release_mem_regions:
+	pci_release_mem_regions(pdev);
+err_free_hw_addr:
+	kfree(hw_addr);
+
+	return err;
+}
+
+/**
+ * ice_unmap_all_hw_addr - Release device register memory maps
+ * @pf: pointer to the PF structure
+ *
+ * Release all PCI memory maps and regions.
+ */
+static void ice_unmap_all_hw_addr(struct ice_pf *pf)
+{
+	struct ice_hw_addr *hw_addr = (struct ice_hw_addr *)pf->hw.hw_addr;
+	struct pci_dev *pdev = pf->pdev;
+	unsigned int i;
+
+	if (WARN_ON(!hw_addr))
+		return;
+
+	pf->hw.hw_addr = NULL;
+	for (i = 0; i < hw_addr->nr; i++)
+		iounmap(hw_addr->maps[i].addr);
+	kfree(hw_addr);
+
+	pci_release_mem_regions(pdev);
+}
+
 /**
  * ice_do_reset - Initiate one of many types of resets
  * @pf: board private structure
@@ -5101,19 +5258,10 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		return -EINVAL;
 	}
 
-	/* this driver uses devres, see
-	 * Documentation/driver-api/driver-model/devres.rst
-	 */
-	err = pcim_enable_device(pdev);
+	err = pci_enable_device(pdev);
 	if (err)
 		return err;
 
-	err = pcim_iomap_regions(pdev, BIT(ICE_BAR0), dev_driver_string(dev));
-	if (err) {
-		dev_err(dev, "BAR0 I/O map error %d\n", err);
-		return err;
-	}
-
 	pf = ice_allocate_pf(dev);
 	if (!pf)
 		return -ENOMEM;
@@ -5138,7 +5286,11 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 	set_bit(ICE_SERVICE_DIS, pf->state);
 
 	hw = &pf->hw;
-	hw->hw_addr = pcim_iomap_table(pdev)[ICE_BAR0];
+
+	err = ice_map_all_hw_addr(pf);
+	if (err)
+		goto err_init_iomap_fail;
+
 	pci_save_state(pdev);
 
 	hw->back = pf;
@@ -5186,6 +5338,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 err_init_eth:
 	ice_deinit(pf);
 err_init:
+	ice_unmap_all_hw_addr(pf);
+err_init_iomap_fail:
 	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
 	return err;
@@ -5295,6 +5449,7 @@ static void ice_remove(struct pci_dev *pdev)
 	 */
 	ice_reset(&pf->hw, ICE_RESET_PFR);
 	pci_wait_for_pending_transaction(pdev);
+	ice_unmap_all_hw_addr(pf);
 	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_osdep.h b/drivers/net/ethernet/intel/ice/ice_osdep.h
index 82bc54fec7f3..4b16ff489c3a 100644
--- a/drivers/net/ethernet/intel/ice/ice_osdep.h
+++ b/drivers/net/ethernet/intel/ice/ice_osdep.h
@@ -18,10 +18,49 @@
 #endif
 #include <net/udp_tunnel.h>
 
-#define wr32(a, reg, value)	writel((value), ((a)->hw_addr + (reg)))
-#define rd32(a, reg)		readl((a)->hw_addr + (reg))
-#define wr64(a, reg, value)	writeq((value), ((a)->hw_addr + (reg)))
-#define rd64(a, reg)		readq((a)->hw_addr + (reg))
+struct ice_hw;
+
+/**
+ * struct ice_hw_addr_map - a single hardware address memory map
+ * @addr: iomem address of the start of this map
+ * @start: register offset at the start of this map, inclusive bound
+ * @end: register offset at the end of this map, exclusive bound
+ * @bar: the BAR this map is for
+ *
+ * Structure representing one map of a device BAR register space. Stored as
+ * part of the ice_hw_addr structure in an array ordered by the start offset.
+ *
+ * The addr value is an iomem address returned by ioremap. The start indicates
+ * the first register offset this map is valid for. The end indicates the end
+ * of the map, and is an exclusive bound.
+ */
+struct ice_hw_addr_map {
+	void __iomem *addr;
+	resource_size_t start;
+	resource_size_t end;
+	int bar;
+};
+
+/**
+ * struct ice_hw_addr - a list of hardware address memory maps
+ * @nr: the number of maps made
+ * @maps: flexible array of maps made during device initialization
+ *
+ * Structure representing a series of sparse maps of the device BAR 0 address
+ * space to kernel addresses. Users must convert a register offset to an iomem
+ * address using ice_get_hw_addr.
+ */
+struct ice_hw_addr {
+	unsigned int nr;
+	struct ice_hw_addr_map maps[];
+};
+
+void __iomem *ice_get_hw_addr(struct ice_hw *hw, resource_size_t reg);
+
+#define wr32(a, reg, value)	writel((value), ice_get_hw_addr((a), (reg)))
+#define rd32(a, reg)		readl(ice_get_hw_addr((a), (reg)))
+#define wr64(a, reg, value)	writeq((value), ice_get_hw_addr((a), (reg)))
+#define rd64(a, reg)		readq(ice_get_hw_addr((a), (reg)))
 
 #define ice_flush(a)		rd32((a), GLGEN_STAT)
 #define ICE_M(m, s)		((m) << (s))
@@ -32,7 +71,6 @@ struct ice_dma_mem {
 	size_t size;
 };
 
-struct ice_hw;
 struct device *ice_hw_to_dev(struct ice_hw *hw);
 
 #ifdef CONFIG_DYNAMIC_DEBUG
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
index 4fd0e5d0a313..3d2834673903 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -272,7 +272,7 @@ struct ice_rx_ring {
 	struct net_device *netdev;	/* netdev ring maps to */
 	struct ice_vsi *vsi;		/* Backreference to associated VSI */
 	struct ice_q_vector *q_vector;	/* Backreference to associated vector */
-	u8 __iomem *tail;
+	void __iomem *tail;
 	union {
 		struct ice_rx_buf *rx_buf;
 		struct xdp_buff **xdp_buf;
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index e3f622cad425..f34975efeed7 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -821,7 +821,7 @@ struct ice_mbx_data {
 
 /* Port hardware description */
 struct ice_hw {
-	u8 __iomem *hw_addr;
+	void *hw_addr;
 	void *back;
 	struct ice_aqc_layer_props *layer_info;
 	struct ice_port_info *port_info;
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency issue by allowing write-combining

[Andrysiak, Jakub]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Thursday, January 19, 2023 2:17 AM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency
> issue by allowing write-combining
> 
> The current method of mapping the entire BAR region as a single
> uncacheable region does not allow RDMA to use write combining (WC). This
> results in increased latency with RDMA.
> 
> To fix this, we initially planned to reduce the size of the map made by the PF
> driver to include only up to the beginning of the RDMA space.
> Unfortunately this will not work in the future as there are some hardware
> features which use registers beyond the RDMA area. This includes Scalable
> IOV, a virtualization feature being worked on currently.
> 
> Instead of simply reducing the size of the map, we need a solution which will
> allow access to all areas of the address space while leaving the RDMA area
> open to be mapped with write combining.
> 
> To allow for this, and fix the RMDA latency issue without blocking the higher
> areas of the BAR, we need to create multiple separate memory maps.
> Doing so will create a sparse mapping rather than a contiguous single area.
> 
> Replace the void *hw_addr with a special ice_hw_addr structure which
> represents the multiple mappings as a flexible array.
> 
> Based on the available BAR size, map up to 3 regions:
> 
>  * The space before the RDMA section
>  * The RDMA section which wants write combining behavior
>  * The space after the RDMA section
> 
> Add an ice_get_hw_addr function which converts a register offset into the
> appropriate kernel address based on which chunk it falls into. This does cost
> us slightly more computation overhead for register access as we now must
> check the table each access. However, we can pre-compute the addresses
> where this would most be a problem.
> 
> With this change, the RDMA driver is now free to map the RDMA register
> section as write-combined without impacting access to other device registers
> used by the main PF driver.
> 
> Reported-by: Dave Ertman <david.m.ertman@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Changes since v1:
> * Export ice_get_hw_addr
> * Use ice_get_hw_addr in iRDMA driver
> * Fix the WARN_ON to use %pa instead of %llx for printing a resource_size_t
> 
>  drivers/infiniband/hw/irdma/main.c           |   2 +-
>  drivers/net/ethernet/intel/ice/ice.h         |   4 +-
>  drivers/net/ethernet/intel/ice/ice_base.c    |   5 +-
>  drivers/net/ethernet/intel/ice/ice_ethtool.c |   3 +-
>  drivers/net/ethernet/intel/ice/ice_main.c    | 177 +++++++++++++++++--
>  drivers/net/ethernet/intel/ice/ice_osdep.h   |  48 ++++-
>  drivers/net/ethernet/intel/ice/ice_txrx.h    |   2 +-
>  drivers/net/ethernet/intel/ice/ice_type.h    |   2 +-
>  8 files changed, 219 insertions(+), 24 deletions(-)
> 
Tested-by: Jakub Andrysiak <jakub.andrysiak@intel.com> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency issue by allowing write-combining

[Alexander Lobakin]

From: Jacob Keller <jacob.e.keller@intel.com>
Date: Wed, 18 Jan 2023 17:16:45 -0800

> The current method of mapping the entire BAR region as a single uncacheable
> region does not allow RDMA to use write combining (WC). This results in
> increased latency with RDMA.
> 
> To fix this, we initially planned to reduce the size of the map made by the
> PF driver to include only up to the beginning of the RDMA space.
> Unfortunately this will not work in the future as there are some hardware
> features which use registers beyond the RDMA area. This includes Scalable
> IOV, a virtualization feature being worked on currently.
> 
> Instead of simply reducing the size of the map, we need a solution which
> will allow access to all areas of the address space while leaving the RDMA
> area open to be mapped with write combining.
> 
> To allow for this, and fix the RMDA latency issue without blocking the
> higher areas of the BAR, we need to create multiple separate memory maps.
> Doing so will create a sparse mapping rather than a contiguous single area.
> 
> Replace the void *hw_addr with a special ice_hw_addr structure which
> represents the multiple mappings as a flexible array.
> 
> Based on the available BAR size, map up to 3 regions:
> 
>  * The space before the RDMA section
>  * The RDMA section which wants write combining behavior
>  * The space after the RDMA section

Please don't.

You have[0]:

* io_mapping_init_wc() (+ io_mapping_fini());
* io_mapping_create_wc() (+ io_mapping_free());

^ they do the same (the second just allocates a struct ad-hoc, but it
  can be allocated manually or embedded into a driver structure),

* arch_phys_wc_add() (+ arch_phys_wc_del())[1];

^ optional to make MTRR happy

-- precisely for the case when you need to remap *a part* of BAR in a
different mode.

Splitting BARs, dropping pcim_iomap_regions() and so on, is very wrong.
Not speaking of that it's PCI driver which must own and map all the
memory the device advertises in its PCI config space, and in case of
ice, PCI driver is combined with Ethernet, so it's ice which must own
and map all the memory.
Not speaking of that using a structure with a flex array and creating a
static inline to calculate the pointer each time you need to read/write
a register, hurts performance and looks properly ugly.

The interfaces above must be used by the RDMA driver, right before
mapping its part in WC mode. PCI driver has no idea that someone else
wants to remap its memory differently, so the code doesn't belong here.
I'd drop the patch and let the RDMA team fix/improve their driver.

> 
> Add an ice_get_hw_addr function which converts a register offset into the
> appropriate kernel address based on which chunk it falls into. This does
> cost us slightly more computation overhead for register access as we now
> must check the table each access. However, we can pre-compute the addresses
> where this would most be a problem.
> 
> With this change, the RDMA driver is now free to map the RDMA register
> section as write-combined without impacting access to other device
> registers used by the main PF driver.
> 
> Reported-by: Dave Ertman <david.m.ertman@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Changes since v1:
> * Export ice_get_hw_addr
> * Use ice_get_hw_addr in iRDMA driver
> * Fix the WARN_ON to use %pa instead of %llx for printing a resource_size_t
> 
>  drivers/infiniband/hw/irdma/main.c           |   2 +-
>  drivers/net/ethernet/intel/ice/ice.h         |   4 +-
>  drivers/net/ethernet/intel/ice/ice_base.c    |   5 +-
>  drivers/net/ethernet/intel/ice/ice_ethtool.c |   3 +-
>  drivers/net/ethernet/intel/ice/ice_main.c    | 177 +++++++++++++++++--
>  drivers/net/ethernet/intel/ice/ice_osdep.h   |  48 ++++-
>  drivers/net/ethernet/intel/ice/ice_txrx.h    |   2 +-
>  drivers/net/ethernet/intel/ice/ice_type.h    |   2 +-
>  8 files changed, 219 insertions(+), 24 deletions(-)
[0]
https://elixir.bootlin.com/linux/v6.2-rc6/source/include/linux/io-mapping.h#L42
[1]
https://elixir.bootlin.com/linux/v6.2-rc6/source/arch/x86/include/asm/io.h#L339

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

Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency issue by allowing write-combining

[Keller, Jacob E]

> -----Original Message-----
> From: Lobakin, Alexandr <alexandr.lobakin@intel.com>
> Sent: Monday, January 30, 2023 2:03 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Linga, Pavan Kumar
> <pavan.kumar.linga@intel.com>; netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency
> issue by allowing write-combining
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> Date: Wed, 18 Jan 2023 17:16:45 -0800
> 
> > The current method of mapping the entire BAR region as a single uncacheable
> > region does not allow RDMA to use write combining (WC). This results in
> > increased latency with RDMA.
> >
> > To fix this, we initially planned to reduce the size of the map made by the
> > PF driver to include only up to the beginning of the RDMA space.
> > Unfortunately this will not work in the future as there are some hardware
> > features which use registers beyond the RDMA area. This includes Scalable
> > IOV, a virtualization feature being worked on currently.
> >
> > Instead of simply reducing the size of the map, we need a solution which
> > will allow access to all areas of the address space while leaving the RDMA
> > area open to be mapped with write combining.
> >
> > To allow for this, and fix the RMDA latency issue without blocking the
> > higher areas of the BAR, we need to create multiple separate memory maps.
> > Doing so will create a sparse mapping rather than a contiguous single area.
> >
> > Replace the void *hw_addr with a special ice_hw_addr structure which
> > represents the multiple mappings as a flexible array.
> >
> > Based on the available BAR size, map up to 3 regions:
> >
> >  * The space before the RDMA section
> >  * The RDMA section which wants write combining behavior
> >  * The space after the RDMA section
> 
> Please don't.
> 
> You have[0]:
> 
> * io_mapping_init_wc() (+ io_mapping_fini());
> * io_mapping_create_wc() (+ io_mapping_free());
> 
> ^ they do the same (the second just allocates a struct ad-hoc, but it
>   can be allocated manually or embedded into a driver structure),
> 
> * arch_phys_wc_add() (+ arch_phys_wc_del())[1];
> 
> ^ optional to make MTRR happy
> 
> -- precisely for the case when you need to remap *a part* of BAR in a
> different mode.
> 
> Splitting BARs, dropping pcim_iomap_regions() and so on, is very wrong.
> Not speaking of that it's PCI driver which must own and map all the
> memory the device advertises in its PCI config space, and in case of
> ice, PCI driver is combined with Ethernet, so it's ice which must own
> and map all the memory.
> Not speaking of that using a structure with a flex array and creating a
> static inline to calculate the pointer each time you need to read/write
> a register, hurts performance and looks properly ugly.
> 
> The interfaces above must be used by the RDMA driver, right before
> mapping its part in WC mode. PCI driver has no idea that someone else
> wants to remap its memory differently, so the code doesn't belong here.
> I'd drop the patch and let the RDMA team fix/improve their driver.
> 

Appreciate the review! I proposed this option after the original change was to simply reduce the initial size of our bar mapping, resulting in losing access to the registers beyond the RDMA section, which was a non-starter for us once we finish implementing Scalable IOV support.

Searching for io_mapping_init_wc and io_mapping_create_wc there are only a handful of users and not much documentation so no wonder I had trouble locating it! Thanks for helping me learn about it.

@Dave.Ertman@intel.com, @Saleem, Shiraz it looks like we need to drop this patch and modify the iRDMA driver's method of requesting write combined regions to use these new interfaces.

@Nguyen, Anthony L Can you drop this patch from the series on IWL or should I send a v3?

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

Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency issue by allowing write-combining

[Tony Nguyen]

On 1/30/2023 3:34 PM, Keller, Jacob E wrote:
>
> @Nguyen, Anthony L Can you drop this patch from the series on IWL or should I send a v3?

It looks like it cleanly drops out of the series; I can take care of it, 
no need for v3.

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

Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency issue by allowing write-combining

[Jacob Keller]

On 1/30/2023 3:34 PM, Keller, Jacob E wrote:
> 
> 
>> -----Original Message-----
>> From: Lobakin, Alexandr <alexandr.lobakin@intel.com>
>> Sent: Monday, January 30, 2023 2:03 AM
>> To: Keller, Jacob E <jacob.e.keller@intel.com>
>> Cc: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Nguyen, Anthony L
>> <anthony.l.nguyen@intel.com>; Linga, Pavan Kumar
>> <pavan.kumar.linga@intel.com>; netdev@vger.kernel.org
>> Subject: Re: [Intel-wired-lan] [PATCH net-next v2 05/13] ice: Fix RDMA latency
>> issue by allowing write-combining
>>
>> From: Jacob Keller <jacob.e.keller@intel.com>
>> Date: Wed, 18 Jan 2023 17:16:45 -0800
>>
>>> The current method of mapping the entire BAR region as a single uncacheable
>>> region does not allow RDMA to use write combining (WC). This results in
>>> increased latency with RDMA.
>>>
>>> To fix this, we initially planned to reduce the size of the map made by the
>>> PF driver to include only up to the beginning of the RDMA space.
>>> Unfortunately this will not work in the future as there are some hardware
>>> features which use registers beyond the RDMA area. This includes Scalable
>>> IOV, a virtualization feature being worked on currently.
>>>
>>> Instead of simply reducing the size of the map, we need a solution which
>>> will allow access to all areas of the address space while leaving the RDMA
>>> area open to be mapped with write combining.
>>>
>>> To allow for this, and fix the RMDA latency issue without blocking the
>>> higher areas of the BAR, we need to create multiple separate memory maps.
>>> Doing so will create a sparse mapping rather than a contiguous single area.
>>>
>>> Replace the void *hw_addr with a special ice_hw_addr structure which
>>> represents the multiple mappings as a flexible array.
>>>
>>> Based on the available BAR size, map up to 3 regions:
>>>
>>>  * The space before the RDMA section
>>>  * The RDMA section which wants write combining behavior
>>>  * The space after the RDMA section
>>
>> Please don't.
>>
>> You have[0]:
>>
>> * io_mapping_init_wc() (+ io_mapping_fini());
>> * io_mapping_create_wc() (+ io_mapping_free());
>>
>> ^ they do the same (the second just allocates a struct ad-hoc, but it
>>   can be allocated manually or embedded into a driver structure),
>>
>> * arch_phys_wc_add() (+ arch_phys_wc_del())[1];
>>
>> ^ optional to make MTRR happy
>>
>> -- precisely for the case when you need to remap *a part* of BAR in a
>> different mode.
>>
>> Splitting BARs, dropping pcim_iomap_regions() and so on, is very wrong.
>> Not speaking of that it's PCI driver which must own and map all the
>> memory the device advertises in its PCI config space, and in case of
>> ice, PCI driver is combined with Ethernet, so it's ice which must own
>> and map all the memory.
>> Not speaking of that using a structure with a flex array and creating a
>> static inline to calculate the pointer each time you need to read/write
>> a register, hurts performance and looks properly ugly.
>>
>> The interfaces above must be used by the RDMA driver, right before
>> mapping its part in WC mode. PCI driver has no idea that someone else
>> wants to remap its memory differently, so the code doesn't belong here.
>> I'd drop the patch and let the RDMA team fix/improve their driver.
>>
> 
> Appreciate the review! I proposed this option after the original change was to simply reduce the initial size of our bar mapping, resulting in losing access to the registers beyond the RDMA section, which was a non-starter for us once we finish implementing Scalable IOV support.
> 
> Searching for io_mapping_init_wc and io_mapping_create_wc there are only a handful of users and not much documentation so no wonder I had trouble locating it! Thanks for helping me learn about it.
> 
> @Dave.Ertman@intel.com, @Saleem, Shiraz it looks like we need to drop this patch and modify the iRDMA driver's method of requesting write combined regions to use these new interfaces.
> 
> @Nguyen, Anthony L Can you drop this patch from the series on IWL or should I send a v3?
> 
> Thanks,
> Jake 

Mustafa,

It looks like Shiraz is on vacation and you're his coverage. Can you
help investigate this and the proposed io_mapping solution from Olek?

Dave,

I tried to add you on the original thread above but it got lost due to
an incorrect email.

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

[Intel-wired-lan] [PATCH net-next v2 06/13] ice: move ice_vf_vsi_release into ice_vf_lib.c

[Jacob Keller]

The ice_vf_vsi_release function will be used in a future change to
refactor the .vsi_rebuild function. Move this over to ice_vf_lib.c so
that it can be used there.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
No changes since v1.

 drivers/net/ethernet/intel/ice/ice_sriov.c     | 15 ---------------
 drivers/net/ethernet/intel/ice/ice_vf_lib.c    | 18 ++++++++++++++++++
 .../ethernet/intel/ice/ice_vf_lib_private.h    |  1 +
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index a101768b1cc5..de20e50623d7 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -40,21 +40,6 @@ static void ice_free_vf_entries(struct ice_pf *pf)
 	}
 }
 
-/**
- * ice_vf_vsi_release - invalidate the VF's VSI after freeing it
- * @vf: invalidate this VF's VSI after freeing it
- */
-static void ice_vf_vsi_release(struct ice_vf *vf)
-{
-	struct ice_vsi *vsi = ice_get_vf_vsi(vf);
-
-	if (WARN_ON(!vsi))
-		return;
-
-	ice_vsi_release(vsi);
-	ice_vf_invalidate_vsi(vf);
-}
-
 /**
  * ice_free_vf_res - Free a VF's resources
  * @vf: pointer to the VF info
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 1b7e919b9275..5fecbec55f54 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -1143,6 +1143,24 @@ void ice_vf_invalidate_vsi(struct ice_vf *vf)
 	vf->lan_vsi_num = ICE_NO_VSI;
 }
 
+/**
+ * ice_vf_vsi_release - Release the VF VSI and invalidate indexes
+ * @vf: pointer to the VF structure
+ *
+ * Release the VF associated with this VSI and then invalidate the VSI
+ * indexes.
+ */
+void ice_vf_vsi_release(struct ice_vf *vf)
+{
+	struct ice_vsi *vsi = ice_get_vf_vsi(vf);
+
+	if (WARN_ON(!vsi))
+		return;
+
+	ice_vsi_release(vsi);
+	ice_vf_invalidate_vsi(vf);
+}
+
 /**
  * ice_vf_set_initialized - VF is ready for VIRTCHNL communication
  * @vf: VF to set in initialized state
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
index 9c8ef2b01f0f..a0f204746f4e 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
@@ -36,6 +36,7 @@ void ice_vf_ctrl_invalidate_vsi(struct ice_vf *vf);
 void ice_vf_ctrl_vsi_release(struct ice_vf *vf);
 struct ice_vsi *ice_vf_ctrl_vsi_setup(struct ice_vf *vf);
 void ice_vf_invalidate_vsi(struct ice_vf *vf);
+void ice_vf_vsi_release(struct ice_vf *vf);
 void ice_vf_set_initialized(struct ice_vf *vf);
 
 #endif /* _ICE_VF_LIB_PRIVATE_H_ */
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 06/13] ice: move ice_vf_vsi_release into ice_vf_lib.c

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 06/13] ice: move
> ice_vf_vsi_release into ice_vf_lib.c
> 
> The ice_vf_vsi_release function will be used in a future change to refactor the
> .vsi_rebuild function. Move this over to ice_vf_lib.c so that it can be used
> there.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> No changes since v1.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c     | 15 ---------------
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c    | 18 ++++++++++++++++++
>  .../ethernet/intel/ice/ice_vf_lib_private.h    |  1 +
>  3 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index a101768b1cc5..de20e50623d7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 07/13] ice: Pull common tasks into ice_vf_post_vsi_rebuild

[Jacob Keller]

The Single Root IOV implementation of .post_vsi_rebuild performs some tasks
that will ultimately need to be shared with the Scalable IOV implementation
such as rebuilding the host configuration.

Refactor by introducing a new wrapper function, ice_vf_post_vsi_rebuild
which performs the tasks that will be shared between SR-IOV and Scalable
IOV. Move the ice_vf_rebuild_host_cfg and ice_vf_set_initialized calls into
this wrapper. Then call the implementation specific post_vsi_rebuild
handler afterwards.

This ensures that we will properly re-initialize filters and expected
settings for both SR-IOV and Scalable IOV.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
No Changes since v1.

 drivers/net/ethernet/intel/ice/ice_sriov.c  |  2 --
 drivers/net/ethernet/intel/ice/ice_vf_lib.c | 19 +++++++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index de20e50623d7..6ff29be974c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -816,8 +816,6 @@ static int ice_sriov_vsi_rebuild(struct ice_vf *vf)
  */
 static void ice_sriov_post_vsi_rebuild(struct ice_vf *vf)
 {
-	ice_vf_rebuild_host_cfg(vf);
-	ice_vf_set_initialized(vf);
 	ice_ena_vf_mappings(vf);
 	wr32(&vf->pf->hw, VFGEN_RSTAT(vf->vf_id), VIRTCHNL_VFR_VFACTIVE);
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 5fecbec55f54..624c7de8b205 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -270,6 +270,21 @@ static int ice_vf_rebuild_vsi(struct ice_vf *vf)
 	return 0;
 }
 
+/**
+ * ice_vf_post_vsi_rebuild - Reset tasks that occur after VSI rebuild
+ * @vf: the VF being reset
+ *
+ * Perform reset tasks which must occur after the VSI has been re-created or
+ * rebuilt during a VF reset.
+ */
+static void ice_vf_post_vsi_rebuild(struct ice_vf *vf)
+{
+	ice_vf_rebuild_host_cfg(vf);
+	ice_vf_set_initialized(vf);
+
+	vf->vf_ops->post_vsi_rebuild(vf);
+}
+
 /**
  * ice_is_any_vf_in_unicast_promisc - check if any VF(s)
  * are in unicast promiscuous mode
@@ -495,7 +510,7 @@ void ice_reset_all_vfs(struct ice_pf *pf)
 
 		ice_vf_pre_vsi_rebuild(vf);
 		ice_vf_rebuild_vsi(vf);
-		vf->vf_ops->post_vsi_rebuild(vf);
+		ice_vf_post_vsi_rebuild(vf);
 
 		mutex_unlock(&vf->cfg_lock);
 	}
@@ -646,7 +661,7 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags)
 		goto out_unlock;
 	}
 
-	vf->vf_ops->post_vsi_rebuild(vf);
+	ice_vf_post_vsi_rebuild(vf);
 	vsi = ice_get_vf_vsi(vf);
 	if (WARN_ON(!vsi)) {
 		err = -EINVAL;
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 07/13] ice: Pull common tasks into ice_vf_post_vsi_rebuild

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 07/13] ice: Pull common tasks
> into ice_vf_post_vsi_rebuild
> 
> The Single Root IOV implementation of .post_vsi_rebuild performs some
> tasks that will ultimately need to be shared with the Scalable IOV
> implementation such as rebuilding the host configuration.
> 
> Refactor by introducing a new wrapper function, ice_vf_post_vsi_rebuild
> which performs the tasks that will be shared between SR-IOV and Scalable
> IOV. Move the ice_vf_rebuild_host_cfg and ice_vf_set_initialized calls into
> this wrapper. Then call the implementation specific post_vsi_rebuild handler
> afterwards.
> 
> This ensures that we will properly re-initialize filters and expected settings for
> both SR-IOV and Scalable IOV.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> No Changes since v1.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c  |  2 --
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 19 +++++++++++++++++--
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index de20e50623d7..6ff29be974c5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 08/13] ice: add a function to initialize vf entry

[Jacob Keller]

Some of the initialization code for Single Root IOV VFs will need to be
reused when we introduce Scalable IOV. Pull this code out into a new
ice_initialize_vf_entry helper function.

Co-developed-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
Signed-off-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes since v1:
* Added Harshitha's Co-developed-by and Signed-off-by

Note that this patch was originally authored by Harshitha internally, but
she no longer works for Intel. I took over authorship with her blessing.
I've added her Co-developed-by to indicate authorship, but her @intel.com
address is no longer valid.

I did not think it right to have her be the "From:" address since she will
not be responsible for addressing feedback.

 drivers/net/ethernet/intel/ice/ice_sriov.c    | 16 ++----------
 drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 26 +++++++++++++++++++
 .../ethernet/intel/ice/ice_vf_lib_private.h   |  1 +
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 6ff29be974c5..6c07f661d44c 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -867,21 +867,9 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs)
 		/* set sriov vf ops for VFs created during SRIOV flow */
 		vf->vf_ops = &ice_sriov_vf_ops;
 
+		ice_initialize_vf_entry(vf);
+
 		vf->vf_sw_id = pf->first_sw;
-		/* assign default capabilities */
-		vf->spoofchk = true;
-		vf->num_vf_qs = pf->vfs.num_qps_per;
-		ice_vc_set_default_allowlist(vf);
-
-		/* ctrl_vsi_idx will be set to a valid value only when VF
-		 * creates its first fdir rule.
-		 */
-		ice_vf_ctrl_invalidate_vsi(vf);
-		ice_vf_fdir_init(vf);
-
-		ice_virtchnl_set_dflt_ops(vf);
-
-		mutex_init(&vf->cfg_lock);
 
 		hash_add_rcu(vfs->table, &vf->entry, vf_id);
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 624c7de8b205..b6fd1e852968 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -698,6 +698,32 @@ void ice_set_vf_state_qs_dis(struct ice_vf *vf)
 
 /* Private functions only accessed from other virtualization files */
 
+/**
+ * ice_initialize_vf_entry - Initialize a VF entry
+ * @vf: pointer to the VF structure
+ */
+void ice_initialize_vf_entry(struct ice_vf *vf)
+{
+	struct ice_pf *pf = vf->pf;
+	struct ice_vfs *vfs;
+
+	vfs = &pf->vfs;
+
+	/* assign default capabilities */
+	vf->spoofchk = true;
+	vf->num_vf_qs = vfs->num_qps_per;
+	ice_vc_set_default_allowlist(vf);
+	ice_virtchnl_set_dflt_ops(vf);
+
+	/* ctrl_vsi_idx will be set to a valid value only when iAVF
+	 * creates its first fdir rule.
+	 */
+	ice_vf_ctrl_invalidate_vsi(vf);
+	ice_vf_fdir_init(vf);
+
+	mutex_init(&vf->cfg_lock);
+}
+
 /**
  * ice_dis_vf_qs - Disable the VF queues
  * @vf: pointer to the VF structure
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
index a0f204746f4e..552d1d02982d 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
@@ -23,6 +23,7 @@
 #warning "Only include ice_vf_lib_private.h in CONFIG_PCI_IOV virtualization files"
 #endif
 
+void ice_initialize_vf_entry(struct ice_vf *vf);
 void ice_dis_vf_qs(struct ice_vf *vf);
 int ice_check_vf_init(struct ice_vf *vf);
 enum virtchnl_status_code ice_err_to_virt_err(int err);
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 08/13] ice: add a function to initialize vf entry

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Harshitha
> Ramamurthy <harshitha.ramamurthy@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 08/13] ice: add a function to
> initialize vf entry
> 
> Some of the initialization code for Single Root IOV VFs will need to be reused
> when we introduce Scalable IOV. Pull this code out into a new
> ice_initialize_vf_entry helper function.
> 
> Co-developed-by: Harshitha Ramamurthy
> <harshitha.ramamurthy@intel.com>
> Signed-off-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Changes since v1:
> * Added Harshitha's Co-developed-by and Signed-off-by
> 
> Note that this patch was originally authored by Harshitha internally, but she
> no longer works for Intel. I took over authorship with her blessing.
> I've added her Co-developed-by to indicate authorship, but her @intel.com
> address is no longer valid.
> 
> I did not think it right to have her be the "From:" address since she will not be
> responsible for addressing feedback.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c    | 16 ++----------
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 26 +++++++++++++++++++
>  .../ethernet/intel/ice/ice_vf_lib_private.h   |  1 +
>  3 files changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 6ff29be974c5..6c07f661d44c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 09/13] ice: introduce ice_vf_init_host_cfg function

[Jacob Keller]

Introduce a new generic helper ice_vf_init_host_cfg which performs common
host configuration initialization tasks that will need to be done for both
Single Root IOV and the new Scalable IOV implementation.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
No changes since v1.

 drivers/net/ethernet/intel/ice/ice_sriov.c    | 36 +------------
 drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 54 +++++++++++++++++++
 .../ethernet/intel/ice/ice_vf_lib_private.h   |  1 +
 3 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 6c07f661d44c..5450fa122729 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -573,51 +573,19 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16 num_vfs)
  */
 static int ice_init_vf_vsi_res(struct ice_vf *vf)
 {
-	struct ice_vsi_vlan_ops *vlan_ops;
 	struct ice_pf *pf = vf->pf;
-	u8 broadcast[ETH_ALEN];
 	struct ice_vsi *vsi;
-	struct device *dev;
 	int err;
 
 	vf->first_vector_idx = ice_calc_vf_first_vector_idx(pf, vf);
 
-	dev = ice_pf_to_dev(pf);
 	vsi = ice_vf_vsi_setup(vf);
 	if (!vsi)
 		return -ENOMEM;
 
-	err = ice_vsi_add_vlan_zero(vsi);
-	if (err) {
-		dev_warn(dev, "Failed to add VLAN 0 filter for VF %d\n",
-			 vf->vf_id);
+	err = ice_vf_init_host_cfg(vf, vsi);
+	if (err)
 		goto release_vsi;
-	}
-
-	vlan_ops = ice_get_compat_vsi_vlan_ops(vsi);
-	err = vlan_ops->ena_rx_filtering(vsi);
-	if (err) {
-		dev_warn(dev, "Failed to enable Rx VLAN filtering for VF %d\n",
-			 vf->vf_id);
-		goto release_vsi;
-	}
-
-	eth_broadcast_addr(broadcast);
-	err = ice_fltr_add_mac(vsi, broadcast, ICE_FWD_TO_VSI);
-	if (err) {
-		dev_err(dev, "Failed to add broadcast MAC filter for VF %d, error %d\n",
-			vf->vf_id, err);
-		goto release_vsi;
-	}
-
-	err = ice_vsi_apply_spoofchk(vsi, vf->spoofchk);
-	if (err) {
-		dev_warn(dev, "Failed to initialize spoofchk setting for VF %d\n",
-			 vf->vf_id);
-		goto release_vsi;
-	}
-
-	vf->num_mac = 1;
 
 	return 0;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index b6fd1e852968..c93d24fee60d 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -1174,6 +1174,60 @@ struct ice_vsi *ice_vf_ctrl_vsi_setup(struct ice_vf *vf)
 	return vsi;
 }
 
+/**
+ * ice_vf_init_host_cfg - Initialize host admin configuration
+ * @vf: VF to initialize
+ * @vsi: the VSI created at initialization
+ *
+ * Initialize the VF host configuration. Called during VF creation to setup
+ * VLAN 0, add the VF VSI broadcast filter, and setup spoof checking. It
+ * should only be called during VF creation.
+ */
+int ice_vf_init_host_cfg(struct ice_vf *vf, struct ice_vsi *vsi)
+{
+	struct ice_vsi_vlan_ops *vlan_ops;
+	struct ice_pf *pf = vf->pf;
+	u8 broadcast[ETH_ALEN];
+	struct device *dev;
+	int err;
+
+	dev = ice_pf_to_dev(pf);
+
+	err = ice_vsi_add_vlan_zero(vsi);
+	if (err) {
+		dev_warn(dev, "Failed to add VLAN 0 filter for VF %d\n",
+			 vf->vf_id);
+		return err;
+	}
+
+	vlan_ops = ice_get_compat_vsi_vlan_ops(vsi);
+	err = vlan_ops->ena_rx_filtering(vsi);
+	if (err) {
+		dev_warn(dev, "Failed to enable Rx VLAN filtering for VF %d\n",
+			 vf->vf_id);
+		return err;
+	}
+
+	eth_broadcast_addr(broadcast);
+	err = ice_fltr_add_mac(vsi, broadcast, ICE_FWD_TO_VSI);
+	if (err) {
+		dev_err(dev, "Failed to add broadcast MAC filter for VF %d, status %d\n",
+			vf->vf_id, err);
+		return err;
+	}
+
+	vf->num_mac = 1;
+
+	err = ice_vsi_apply_spoofchk(vsi, vf->spoofchk);
+	if (err) {
+		dev_warn(dev, "Failed to initialize spoofchk setting for VF %d\n",
+			 vf->vf_id);
+		return err;
+	}
+
+	return 0;
+}
+
 /**
  * ice_vf_invalidate_vsi - invalidate vsi_idx/vsi_num to remove VSI access
  * @vf: VF to remove access to VSI for
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
index 552d1d02982d..6f3293b793b5 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h
@@ -36,6 +36,7 @@ void ice_vf_rebuild_host_cfg(struct ice_vf *vf);
 void ice_vf_ctrl_invalidate_vsi(struct ice_vf *vf);
 void ice_vf_ctrl_vsi_release(struct ice_vf *vf);
 struct ice_vsi *ice_vf_ctrl_vsi_setup(struct ice_vf *vf);
+int ice_vf_init_host_cfg(struct ice_vf *vf, struct ice_vsi *vsi);
 void ice_vf_invalidate_vsi(struct ice_vf *vf);
 void ice_vf_vsi_release(struct ice_vf *vf);
 void ice_vf_set_initialized(struct ice_vf *vf);
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 09/13] ice: introduce ice_vf_init_host_cfg function

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 09/13] ice: introduce
> ice_vf_init_host_cfg function
> 
> Introduce a new generic helper ice_vf_init_host_cfg which performs common
> host configuration initialization tasks that will need to be done for both Single
> Root IOV and the new Scalable IOV implementation.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> No changes since v1.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c    | 36 +------------
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 54 +++++++++++++++++++
>  .../ethernet/intel/ice/ice_vf_lib_private.h   |  1 +
>  3 files changed, 57 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 6c07f661d44c..5450fa122729 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 10/13] ice: convert vf_ops .vsi_rebuild to .create_vsi

[Jacob Keller]

The .vsi_rebuild function exists for ice_reset_vf. It is used to release
and re-create the VSI during a single-VF reset.

This function is only called when we need to re-create the VSI, and not
when rebuilding an existing VSI. This makes the single-VF reset process
different from the process used to restore functionality after a
hardware reset such as the PF reset or EMP reset.

When we add support for Scalable IOV VFs, the implementation will be very
similar. The primary difference will be in the fact that each VF type uses
a different underlying VSI type in hardware.

Move the common functionality into a new ice_vf_recreate VSI function. This
will allow the two IOV paths to share this functionality. Rework the
.vsi_rebuild vf_op into .create_vsi, only performing the task of creating a
new VSI.

This creates a nice dichotomy between the ice_vf_rebuild_vsi and
ice_vf_recreate_vsi, and should make it more clear why the two flows atre
distinct.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
No changes since v1.

 drivers/net/ethernet/intel/ice/ice_sriov.c  | 22 ++++++---------
 drivers/net/ethernet/intel/ice/ice_vf_lib.c | 31 ++++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_vf_lib.h |  2 +-
 3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 5450fa122729..46088c05d485 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -757,23 +757,19 @@ static void ice_sriov_clear_reset_trigger(struct ice_vf *vf)
 }
 
 /**
- * ice_sriov_vsi_rebuild - release and rebuild VF's VSI
- * @vf: VF to release and setup the VSI for
+ * ice_sriov_create_vsi - Create a new VSI for a VF
+ * @vf: VF to create the VSI for
  *
- * This is only called when a single VF is being reset (i.e. VFR, VFLR, host VF
- * configuration change, etc.).
+ * This is called by ice_vf_recreate_vsi to create the new VSI after the old
+ * VSI has been released.
  */
-static int ice_sriov_vsi_rebuild(struct ice_vf *vf)
+static int ice_sriov_create_vsi(struct ice_vf *vf)
 {
-	struct ice_pf *pf = vf->pf;
+	struct ice_vsi *vsi;
 
-	ice_vf_vsi_release(vf);
-	if (!ice_vf_vsi_setup(vf)) {
-		dev_err(ice_pf_to_dev(pf),
-			"Failed to release and setup the VF%u's VSI\n",
-			vf->vf_id);
+	vsi = ice_vf_vsi_setup(vf);
+	if (!vsi)
 		return -ENOMEM;
-	}
 
 	return 0;
 }
@@ -795,7 +791,7 @@ static const struct ice_vf_ops ice_sriov_vf_ops = {
 	.trigger_reset_register = ice_sriov_trigger_reset_register,
 	.poll_reset_status = ice_sriov_poll_reset_status,
 	.clear_reset_trigger = ice_sriov_clear_reset_trigger,
-	.vsi_rebuild = ice_sriov_vsi_rebuild,
+	.create_vsi = ice_sriov_create_vsi,
 	.post_vsi_rebuild = ice_sriov_post_vsi_rebuild,
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index c93d24fee60d..1a5d64454f99 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -241,12 +241,41 @@ static void ice_vf_pre_vsi_rebuild(struct ice_vf *vf)
 	vf->vf_ops->clear_reset_trigger(vf);
 }
 
+/**
+ * ice_vf_recreate_vsi - Release and re-create the VF's VSI
+ * @vf: VF to recreate the VSI for
+ *
+ * This is only called when a single VF is being reset (i.e. VVF, VFLR, host
+ * VF configuration change, etc)
+ *
+ * It releases and then re-creates a new VSI.
+ */
+static int ice_vf_recreate_vsi(struct ice_vf *vf)
+{
+	struct ice_pf *pf = vf->pf;
+	int err;
+
+	ice_vf_vsi_release(vf);
+
+	err = vf->vf_ops->create_vsi(vf);
+	if (err) {
+		dev_err(ice_pf_to_dev(pf),
+			"Failed to recreate the VF%u's VSI, error %d\n",
+			vf->vf_id, err);
+		return err;
+	}
+
+	return 0;
+}
+
 /**
  * ice_vf_rebuild_vsi - rebuild the VF's VSI
  * @vf: VF to rebuild the VSI for
  *
  * This is only called when all VF(s) are being reset (i.e. PCIe Reset on the
  * host, PFR, CORER, etc.).
+ *
+ * It reprograms the VSI configuration back into hardware.
  */
 static int ice_vf_rebuild_vsi(struct ice_vf *vf)
 {
@@ -654,7 +683,7 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags)
 
 	ice_vf_pre_vsi_rebuild(vf);
 
-	if (vf->vf_ops->vsi_rebuild(vf)) {
+	if (ice_vf_recreate_vsi(vf)) {
 		dev_err(dev, "Failed to release and setup the VF%u's VSI\n",
 			vf->vf_id);
 		err = -EFAULT;
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
index 52bd9a3816bf..e3d94f3ca40d 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
@@ -60,7 +60,7 @@ struct ice_vf_ops {
 	void (*trigger_reset_register)(struct ice_vf *vf, bool is_vflr);
 	bool (*poll_reset_status)(struct ice_vf *vf);
 	void (*clear_reset_trigger)(struct ice_vf *vf);
-	int (*vsi_rebuild)(struct ice_vf *vf);
+	int (*create_vsi)(struct ice_vf *vf);
 	void (*post_vsi_rebuild)(struct ice_vf *vf);
 };
 
-- 
2.39.1.405.gd4c25cc71f83

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

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

[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
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.

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
  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)
 {
 }
 
-- 
2.39.1.405.gd4c25cc71f83

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

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

[Paul Menzel]

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

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

[Jacob Keller]

On 1/19/2023 12:42 AM, Paul Menzel wrote:
> 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.
> 

The operation can be tested by triggering a device reset while having
VFs. But note that the ice driver does not yet support Scalable IOV. We
are working on this but are not yet ready to submit a series
implementing the support. Some of this series are patches intended to
aid in implementing Scalable IOV, and I am sending them now in order to
reduce the size and scope of the series that will actually finish the
implementation.

In general the reset behavior can be tested by adding a Single Root IOV
VF (via sriov_numvfs), and then triggering a reset. This can be done in
a variety of ways including by configuring some of the netlink VF
parameters like the trust setting, or by any operation on the PF that
triggers a PF reset. (Unfortunately I do not recall a specific example
offhand).

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

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

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 11/13] ice: introduce
> clear_reset_state operation
> 
> 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
> 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.
> 
> 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
>   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

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 12/13] ice: introduce .irq_close VF operation

[Jacob Keller]

The Scalable IOV implementation will require notifying the VDCM driver when
an IRQ must be closed. This allows the VDCM to handle releasing stale IRQ
context values and properly reconfigure.

To handle this, introduce a new optional .irq_close callback to the VF
operations structure. This will be implemented by Scalable IOV to handle
the shutdown of the IRQ context.

Since the SR-IOV implementation does not need this, we must check that its
non-NULL before calling it.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
No changes since v1.

 drivers/net/ethernet/intel/ice/ice_sriov.c  | 1 +
 drivers/net/ethernet/intel/ice/ice_vf_lib.c | 4 ++++
 drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 4d8930b83b35..356ac76ef90f 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -807,6 +807,7 @@ static const struct ice_vf_ops ice_sriov_vf_ops = {
 	.trigger_reset_register = ice_sriov_trigger_reset_register,
 	.poll_reset_status = ice_sriov_poll_reset_status,
 	.clear_reset_trigger = ice_sriov_clear_reset_trigger,
+	.irq_close = NULL,
 	.create_vsi = ice_sriov_create_vsi,
 	.post_vsi_rebuild = ice_sriov_post_vsi_rebuild,
 };
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 2ea801ebb2ac..d16c2ea83873 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -237,6 +237,10 @@ static void ice_vf_clear_counters(struct ice_vf *vf)
  */
 static void ice_vf_pre_vsi_rebuild(struct ice_vf *vf)
 {
+	/* Close any IRQ mapping now */
+	if (vf->vf_ops->irq_close)
+		vf->vf_ops->irq_close(vf);
+
 	ice_vf_clear_counters(vf);
 	vf->vf_ops->clear_reset_trigger(vf);
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
index 5bb75edb6cef..b4e6480f30a7 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
@@ -61,6 +61,7 @@ struct ice_vf_ops {
 	void (*trigger_reset_register)(struct ice_vf *vf, bool is_vflr);
 	bool (*poll_reset_status)(struct ice_vf *vf);
 	void (*clear_reset_trigger)(struct ice_vf *vf);
+	void (*irq_close)(struct ice_vf *vf);
 	int (*create_vsi)(struct ice_vf *vf);
 	void (*post_vsi_rebuild)(struct ice_vf *vf);
 };
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 12/13] ice: introduce .irq_close VF operation

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 12/13] ice: introduce .irq_close
> VF operation
> 
> The Scalable IOV implementation will require notifying the VDCM driver
> when an IRQ must be closed. This allows the VDCM to handle releasing stale
> IRQ context values and properly reconfigure.
> 
> To handle this, introduce a new optional .irq_close callback to the VF
> operations structure. This will be implemented by Scalable IOV to handle the
> shutdown of the IRQ context.
> 
> Since the SR-IOV implementation does not need this, we must check that its
> non-NULL before calling it.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> No changes since v1.
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c  | 1 +
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 4 ++++
> drivers/net/ethernet/intel/ice/ice_vf_lib.h | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index 4d8930b83b35..356ac76ef90f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -807,6 +807,7 @@ static const struct ice_vf_ops ice_sriov_vf_ops = {
>  	.trigger_reset_register = ice_sriov_trigger_reset_register,
>  	.poll_reset_status = ice_sriov_poll_reset_status,
>  	.clear_reset_trigger = ice_sriov_clear_reset_trigger,
> +	.irq_close = NULL,
>  	.create_vsi = ice_sriov_create_vsi,
>  	.post_vsi_rebuild = ice_sriov_post_vsi_rebuild,  }; diff --git
> a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> index 2ea801ebb2ac..d16c2ea83873 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next v2 13/13] ice: remove unnecessary virtchnl_ether_addr struct use

[Jacob Keller]

The dev_lan_addr and hw_lan_addr members of ice_vf are used only to store
the MAC address for the VF. They are defined using virtchnl_ether_addr, but
only the .addr sub-member is actually used. Drop the use of
virtchnl_ether_addr and just use a u8 array of length [ETH_ALEN].

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
No changes since v1.

 drivers/net/ethernet/intel/ice/ice_eswitch.c  | 18 +++++++-------
 drivers/net/ethernet/intel/ice/ice_sriov.c    | 16 ++++++-------
 drivers/net/ethernet/intel/ice/ice_vf_lib.c   |  8 +++----
 drivers/net/ethernet/intel/ice/ice_vf_lib.h   |  4 ++--
 drivers/net/ethernet/intel/ice/ice_virtchnl.c | 24 +++++++++----------
 5 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
index b86d173a20af..f6dd3f8fd936 100644
--- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
+++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
@@ -71,17 +71,17 @@ void ice_eswitch_replay_vf_mac_rule(struct ice_vf *vf)
 	if (!ice_is_switchdev_running(vf->pf))
 		return;
 
-	if (is_valid_ether_addr(vf->hw_lan_addr.addr)) {
+	if (is_valid_ether_addr(vf->hw_lan_addr)) {
 		err = ice_eswitch_add_vf_mac_rule(vf->pf, vf,
-						  vf->hw_lan_addr.addr);
+						  vf->hw_lan_addr);
 		if (err) {
 			dev_err(ice_pf_to_dev(vf->pf), "Failed to add MAC %pM for VF %d\n, error %d\n",
-				vf->hw_lan_addr.addr, vf->vf_id, err);
+				vf->hw_lan_addr, vf->vf_id, err);
 			return;
 		}
 		vf->num_mac++;
 
-		ether_addr_copy(vf->dev_lan_addr.addr, vf->hw_lan_addr.addr);
+		ether_addr_copy(vf->dev_lan_addr, vf->hw_lan_addr);
 	}
 }
 
@@ -237,7 +237,7 @@ ice_eswitch_release_reprs(struct ice_pf *pf, struct ice_vsi *ctrl_vsi)
 		ice_vsi_update_security(vsi, ice_vsi_ctx_set_antispoof);
 		metadata_dst_free(vf->repr->dst);
 		vf->repr->dst = NULL;
-		ice_fltr_add_mac_and_broadcast(vsi, vf->hw_lan_addr.addr,
+		ice_fltr_add_mac_and_broadcast(vsi, vf->hw_lan_addr,
 					       ICE_FWD_TO_VSI);
 
 		netif_napi_del(&vf->repr->q_vector->napi);
@@ -265,14 +265,14 @@ static int ice_eswitch_setup_reprs(struct ice_pf *pf)
 						   GFP_KERNEL);
 		if (!vf->repr->dst) {
 			ice_fltr_add_mac_and_broadcast(vsi,
-						       vf->hw_lan_addr.addr,
+						       vf->hw_lan_addr,
 						       ICE_FWD_TO_VSI);
 			goto err;
 		}
 
 		if (ice_vsi_update_security(vsi, ice_vsi_ctx_clear_antispoof)) {
 			ice_fltr_add_mac_and_broadcast(vsi,
-						       vf->hw_lan_addr.addr,
+						       vf->hw_lan_addr,
 						       ICE_FWD_TO_VSI);
 			metadata_dst_free(vf->repr->dst);
 			vf->repr->dst = NULL;
@@ -281,7 +281,7 @@ static int ice_eswitch_setup_reprs(struct ice_pf *pf)
 
 		if (ice_vsi_add_vlan_zero(vsi)) {
 			ice_fltr_add_mac_and_broadcast(vsi,
-						       vf->hw_lan_addr.addr,
+						       vf->hw_lan_addr,
 						       ICE_FWD_TO_VSI);
 			metadata_dst_free(vf->repr->dst);
 			vf->repr->dst = NULL;
@@ -338,7 +338,7 @@ void ice_eswitch_update_repr(struct ice_vsi *vsi)
 
 	ret = ice_vsi_update_security(vsi, ice_vsi_ctx_clear_antispoof);
 	if (ret) {
-		ice_fltr_add_mac_and_broadcast(vsi, vf->hw_lan_addr.addr, ICE_FWD_TO_VSI);
+		ice_fltr_add_mac_and_broadcast(vsi, vf->hw_lan_addr, ICE_FWD_TO_VSI);
 		dev_err(ice_pf_to_dev(pf), "Failed to update VF %d port representor",
 			vsi->vf->vf_id);
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 356ac76ef90f..96a64c25e2ef 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -1242,7 +1242,7 @@ ice_get_vf_cfg(struct net_device *netdev, int vf_id, struct ifla_vf_info *ivi)
 		goto out_put_vf;
 
 	ivi->vf = vf_id;
-	ether_addr_copy(ivi->mac, vf->hw_lan_addr.addr);
+	ether_addr_copy(ivi->mac, vf->hw_lan_addr);
 
 	/* VF configuration for VLAN and applicable QoS */
 	ivi->vlan = ice_vf_get_port_vlan_id(vf);
@@ -1290,8 +1290,8 @@ int ice_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 		return -EINVAL;
 
 	/* nothing left to do, unicast MAC already set */
-	if (ether_addr_equal(vf->dev_lan_addr.addr, mac) &&
-	    ether_addr_equal(vf->hw_lan_addr.addr, mac)) {
+	if (ether_addr_equal(vf->dev_lan_addr, mac) &&
+	    ether_addr_equal(vf->hw_lan_addr, mac)) {
 		ret = 0;
 		goto out_put_vf;
 	}
@@ -1305,8 +1305,8 @@ int ice_set_vf_mac(struct net_device *netdev, int vf_id, u8 *mac)
 	/* VF is notified of its new MAC via the PF's response to the
 	 * VIRTCHNL_OP_GET_VF_RESOURCES message after the VF has been reset
 	 */
-	ether_addr_copy(vf->dev_lan_addr.addr, mac);
-	ether_addr_copy(vf->hw_lan_addr.addr, mac);
+	ether_addr_copy(vf->dev_lan_addr, mac);
+	ether_addr_copy(vf->hw_lan_addr, mac);
 	if (is_zero_ether_addr(mac)) {
 		/* VF will send VIRTCHNL_OP_ADD_ETH_ADDR message with its MAC */
 		vf->pf_set_mac = false;
@@ -1707,7 +1707,7 @@ void ice_print_vf_rx_mdd_event(struct ice_vf *vf)
 
 	dev_info(dev, "%d Rx Malicious Driver Detection events detected on PF %d VF %d MAC %pM. mdd-auto-reset-vfs=%s\n",
 		 vf->mdd_rx_events.count, pf->hw.pf_id, vf->vf_id,
-		 vf->dev_lan_addr.addr,
+		 vf->dev_lan_addr,
 		 test_bit(ICE_FLAG_MDD_AUTO_RESET_VF, pf->flags)
 			  ? "on" : "off");
 }
@@ -1751,7 +1751,7 @@ void ice_print_vfs_mdd_events(struct ice_pf *pf)
 
 			dev_info(dev, "%d Tx Malicious Driver Detection events detected on PF %d VF %d MAC %pM.\n",
 				 vf->mdd_tx_events.count, hw->pf_id, vf->vf_id,
-				 vf->dev_lan_addr.addr);
+				 vf->dev_lan_addr);
 		}
 	}
 	mutex_unlock(&pf->vfs.table_lock);
@@ -1841,7 +1841,7 @@ ice_is_malicious_vf(struct ice_pf *pf, struct ice_rq_event_info *event,
 
 			if (pf_vsi)
 				dev_warn(dev, "VF MAC %pM on PF MAC %pM is generating asynchronous messages and may be overflowing the PF message queue. Please see the Adapter User Guide for more information\n",
-					 &vf->dev_lan_addr.addr[0],
+					 &vf->dev_lan_addr[0],
 					 pf_vsi->netdev->dev_addr);
 		}
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index d16c2ea83873..0e57bd1b85fd 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -1008,18 +1008,18 @@ static int ice_vf_rebuild_host_mac_cfg(struct ice_vf *vf)
 
 	vf->num_mac++;
 
-	if (is_valid_ether_addr(vf->hw_lan_addr.addr)) {
-		status = ice_fltr_add_mac(vsi, vf->hw_lan_addr.addr,
+	if (is_valid_ether_addr(vf->hw_lan_addr)) {
+		status = ice_fltr_add_mac(vsi, vf->hw_lan_addr,
 					  ICE_FWD_TO_VSI);
 		if (status) {
 			dev_err(dev, "failed to add default unicast MAC filter %pM for VF %u, error %d\n",
-				&vf->hw_lan_addr.addr[0], vf->vf_id,
+				&vf->hw_lan_addr[0], vf->vf_id,
 				status);
 			return status;
 		}
 		vf->num_mac++;
 
-		ether_addr_copy(vf->dev_lan_addr.addr, vf->hw_lan_addr.addr);
+		ether_addr_copy(vf->dev_lan_addr, vf->hw_lan_addr);
 	}
 
 	return 0;
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
index b4e6480f30a7..ef30f05b5d02 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
@@ -98,8 +98,8 @@ struct ice_vf {
 	struct ice_sw *vf_sw_id;	/* switch ID the VF VSIs connect to */
 	struct virtchnl_version_info vf_ver;
 	u32 driver_caps;		/* reported by VF driver */
-	struct virtchnl_ether_addr dev_lan_addr;
-	struct virtchnl_ether_addr hw_lan_addr;
+	u8 dev_lan_addr[ETH_ALEN];
+	u8 hw_lan_addr[ETH_ALEN];
 	struct ice_time_mac legacy_last_added_umac;
 	DECLARE_BITMAP(txq_ena, ICE_MAX_RSS_QS_PER_VF);
 	DECLARE_BITMAP(rxq_ena, ICE_MAX_RSS_QS_PER_VF);
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index dab3cd5d300e..e24e3f5017ca 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -507,7 +507,7 @@ static int ice_vc_get_vf_res_msg(struct ice_vf *vf, u8 *msg)
 	vfres->vsi_res[0].vsi_type = VIRTCHNL_VSI_SRIOV;
 	vfres->vsi_res[0].num_queue_pairs = vsi->num_txq;
 	ether_addr_copy(vfres->vsi_res[0].default_mac_addr,
-			vf->hw_lan_addr.addr);
+			vf->hw_lan_addr);
 
 	/* match guest capabilities */
 	vf->driver_caps = vfres->vf_cap_flags;
@@ -1802,10 +1802,10 @@ ice_vfhw_mac_add(struct ice_vf *vf, struct virtchnl_ether_addr *vc_ether_addr)
 	 * was correctly specified over VIRTCHNL
 	 */
 	if ((ice_is_vc_addr_legacy(vc_ether_addr) &&
-	     is_zero_ether_addr(vf->hw_lan_addr.addr)) ||
+	     is_zero_ether_addr(vf->hw_lan_addr)) ||
 	    ice_is_vc_addr_primary(vc_ether_addr)) {
-		ether_addr_copy(vf->dev_lan_addr.addr, mac_addr);
-		ether_addr_copy(vf->hw_lan_addr.addr, mac_addr);
+		ether_addr_copy(vf->dev_lan_addr, mac_addr);
+		ether_addr_copy(vf->hw_lan_addr, mac_addr);
 	}
 
 	/* hardware and device MACs are already set, but its possible that the
@@ -1836,7 +1836,7 @@ ice_vc_add_mac_addr(struct ice_vf *vf, struct ice_vsi *vsi,
 	int ret;
 
 	/* device MAC already added */
-	if (ether_addr_equal(mac_addr, vf->dev_lan_addr.addr))
+	if (ether_addr_equal(mac_addr, vf->dev_lan_addr))
 		return 0;
 
 	if (is_unicast_ether_addr(mac_addr) && !ice_can_vf_change_mac(vf)) {
@@ -1891,8 +1891,8 @@ ice_update_legacy_cached_mac(struct ice_vf *vf,
 	    ice_is_legacy_umac_expired(&vf->legacy_last_added_umac))
 		return;
 
-	ether_addr_copy(vf->dev_lan_addr.addr, vf->legacy_last_added_umac.addr);
-	ether_addr_copy(vf->hw_lan_addr.addr, vf->legacy_last_added_umac.addr);
+	ether_addr_copy(vf->dev_lan_addr, vf->legacy_last_added_umac.addr);
+	ether_addr_copy(vf->hw_lan_addr, vf->legacy_last_added_umac.addr);
 }
 
 /**
@@ -1906,15 +1906,15 @@ ice_vfhw_mac_del(struct ice_vf *vf, struct virtchnl_ether_addr *vc_ether_addr)
 	u8 *mac_addr = vc_ether_addr->addr;
 
 	if (!is_valid_ether_addr(mac_addr) ||
-	    !ether_addr_equal(vf->dev_lan_addr.addr, mac_addr))
+	    !ether_addr_equal(vf->dev_lan_addr, mac_addr))
 		return;
 
 	/* allow the device MAC to be repopulated in the add flow and don't
-	 * clear the hardware MAC (i.e. hw_lan_addr.addr) here as that is meant
+	 * clear the hardware MAC (i.e. hw_lan_addr) here as that is meant
 	 * to be persistent on VM reboot and across driver unload/load, which
 	 * won't work if we clear the hardware MAC here
 	 */
-	eth_zero_addr(vf->dev_lan_addr.addr);
+	eth_zero_addr(vf->dev_lan_addr);
 
 	ice_update_legacy_cached_mac(vf, vc_ether_addr);
 }
@@ -1934,7 +1934,7 @@ ice_vc_del_mac_addr(struct ice_vf *vf, struct ice_vsi *vsi,
 	int status;
 
 	if (!ice_can_vf_change_mac(vf) &&
-	    ether_addr_equal(vf->dev_lan_addr.addr, mac_addr))
+	    ether_addr_equal(vf->dev_lan_addr, mac_addr))
 		return 0;
 
 	status = ice_fltr_remove_mac(vsi, mac_addr, ICE_FWD_TO_VSI);
@@ -3733,7 +3733,7 @@ static int ice_vc_repr_add_mac(struct ice_vf *vf, u8 *msg)
 		int result;
 
 		if (!is_unicast_ether_addr(mac_addr) ||
-		    ether_addr_equal(mac_addr, vf->hw_lan_addr.addr))
+		    ether_addr_equal(mac_addr, vf->hw_lan_addr))
 			continue;
 
 		if (vf->pf_set_mac) {
-- 
2.39.1.405.gd4c25cc71f83

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

Re: [Intel-wired-lan] [PATCH net-next v2 13/13] ice: remove unnecessary virtchnl_ether_addr struct use

[Szlosek, Marek]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: czwartek, 19 stycznia 2023 02:17
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH net-next v2 13/13] ice: remove unnecessary
> virtchnl_ether_addr struct use
> 
> The dev_lan_addr and hw_lan_addr members of ice_vf are used only to
> store the MAC address for the VF. They are defined using
> virtchnl_ether_addr, but only the .addr sub-member is actually used. Drop
> the use of virtchnl_ether_addr and just use a u8 array of length [ETH_ALEN].
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> No changes since v1.
> 
>  drivers/net/ethernet/intel/ice/ice_eswitch.c  | 18 +++++++-------
>  drivers/net/ethernet/intel/ice/ice_sriov.c    | 16 ++++++-------
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   |  8 +++----
>  drivers/net/ethernet/intel/ice/ice_vf_lib.h   |  4 ++--
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c | 24 +++++++++----------
>  5 files changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c
> b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> index b86d173a20af..f6dd3f8fd936 100644
> --- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c

Tested-by: Marek Szlosek <marek.szlosek@intel.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan