diff mbox series

[iwl-net] ice: fix DDP package download for packages without signature segment

Message ID 20231104182908.15389-1-paul.greenwalt@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [iwl-net] ice: fix DDP package download for packages without signature segment | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1312 this patch: 1312
netdev/cc_maintainers fail 1 blamed authors not CCed: jacob.e.keller@intel.com; 3 maintainers not CCed: edumazet@google.com pabeni@redhat.com jacob.e.keller@intel.com
netdev/build_clang success Errors and warnings before: 1340 this patch: 1340
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1340 this patch: 1340
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 130 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Greenwalt, Paul Nov. 4, 2023, 6:29 p.m. UTC 
From: Dan Nowlin <dan.nowlin@intel.com>

Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
incorrectly removed support for package download for packages without a
signature segment. These packages include the signature buffer inline
in the configurations buffers, and do not in a signature segment.

Fix package download by providing download support for both packages
with (ice_download_pkg_with_sig_seg()) and without signature segment
(ice_download_pkg_without_sig_seg()).

Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
Reported-by: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ddp.c | 106 ++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 3 deletions(-)


base-commit: 016b9332a3346e97a6cacffea0f9dc10e1235a75

Comments

Jacob Keller Nov. 6, 2023, 5:39 a.m. UTC | #1 
On 11/4/2023 11:29 AM, Paul Greenwalt wrote:
> From: Dan Nowlin <dan.nowlin@intel.com>
> 
> Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> incorrectly removed support for package download for packages without a
> signature segment. These packages include the signature buffer inline
> in the configurations buffers, and do not in a signature segment.
> 
> Fix package download by providing download support for both packages
> with (ice_download_pkg_with_sig_seg()) and without signature segment
> (ice_download_pkg_without_sig_seg()).
> 
> Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> Reported-by: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Fijalkowski, Maciej Nov. 6, 2023, 7:13 a.m. UTC | #2 
On Sat, Nov 04, 2023 at 02:29:08PM -0400, Paul Greenwalt wrote:
> From: Dan Nowlin <dan.nowlin@intel.com>
> 
> Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> incorrectly removed support for package download for packages without a
> signature segment. These packages include the signature buffer inline
> in the configurations buffers, and do not in a signature segment.
> 
> Fix package download by providing download support for both packages
> with (ice_download_pkg_with_sig_seg()) and without signature segment
> (ice_download_pkg_without_sig_seg()).
> 
> Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> Reported-by: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>

s/Fijalkowski, Maciej/Maciej Fijalkowski

> Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>

Tested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Thanks a lot for the quick fix!

> ---
>  drivers/net/ethernet/intel/ice/ice_ddp.c | 106 ++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
> index cfb1580f5850..3f1a11d0252c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
> @@ -1479,14 +1479,14 @@ ice_post_dwnld_pkg_actions(struct ice_hw *hw)
>  }
>  
>  /**
> - * ice_download_pkg
> + * ice_download_pkg_with_sig_seg
>   * @hw: pointer to the hardware structure
>   * @pkg_hdr: pointer to package header
>   *
>   * Handles the download of a complete package.
>   */
>  static enum ice_ddp_state
> -ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
> +ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
>  {
>  	enum ice_aq_err aq_err = hw->adminq.sq_last_status;
>  	enum ice_ddp_state state = ICE_DDP_PKG_ERR;
> @@ -1519,6 +1519,106 @@ ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
>  		state = ice_post_dwnld_pkg_actions(hw);
>  
>  	ice_release_global_cfg_lock(hw);
> +
> +	return state;
> +}
> +
> +/**
> + * ice_dwnld_cfg_bufs
> + * @hw: pointer to the hardware structure
> + * @bufs: pointer to an array of buffers
> + * @count: the number of buffers in the array
> + *
> + * Obtains global config lock and downloads the package configuration buffers
> + * to the firmware.
> + */
> +static enum ice_ddp_state
> +ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
> +{
> +	enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
> +	struct ice_buf_hdr *bh;
> +	int status;
> +
> +	if (!bufs || !count)
> +		return ICE_DDP_PKG_ERR;
> +
> +	/* If the first buffer's first section has its metadata bit set
> +	 * then there are no buffers to be downloaded, and the operation is
> +	 * considered a success.
> +	 */
> +	bh = (struct ice_buf_hdr *)bufs;
> +	if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF)
> +		return ICE_DDP_PKG_SUCCESS;
> +
> +	status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE);
> +	if (status) {
> +		if (status == -EALREADY)
> +			return ICE_DDP_PKG_ALREADY_LOADED;
> +		return ice_map_aq_err_to_ddp_state(hw->adminq.sq_last_status);
> +	}
> +
> +	state = ice_dwnld_cfg_bufs_no_lock(hw, bufs, 0, count, true);
> +	if (!state)
> +		state = ice_post_dwnld_pkg_actions(hw);
> +
> +	ice_release_global_cfg_lock(hw);
> +
> +	return state;
> +}
> +
> +/**
> + * ice_download_pkg_without_sig_seg
> + * @hw: pointer to the hardware structure
> + * @ice_seg: pointer to the segment of the package to be downloaded
> + *
> + * Handles the download of a complete package without signature segment.
> + */
> +static enum ice_ddp_state
> +ice_download_pkg_without_sig_seg(struct ice_hw *hw, struct ice_seg *ice_seg)
> +{
> +	struct ice_buf_table *ice_buf_tbl;
> +	enum ice_ddp_state state;
> +
> +	ice_debug(hw, ICE_DBG_PKG, "Segment format version: %d.%d.%d.%d\n",
> +		  ice_seg->hdr.seg_format_ver.major,
> +		  ice_seg->hdr.seg_format_ver.minor,
> +		  ice_seg->hdr.seg_format_ver.update,
> +		  ice_seg->hdr.seg_format_ver.draft);
> +
> +	ice_debug(hw, ICE_DBG_PKG, "Seg: type 0x%X, size %d, name %s\n",
> +		  le32_to_cpu(ice_seg->hdr.seg_type),
> +		  le32_to_cpu(ice_seg->hdr.seg_size), ice_seg->hdr.seg_id);
> +
> +	ice_buf_tbl = ice_find_buf_table(ice_seg);
> +
> +	ice_debug(hw, ICE_DBG_PKG, "Seg buf count: %d\n",
> +		  le32_to_cpu(ice_buf_tbl->buf_count));
> +
> +	state = ice_dwnld_cfg_bufs(hw, ice_buf_tbl->buf_array,
> +				   le32_to_cpu(ice_buf_tbl->buf_count));
> +
> +	return state;
> +}
> +
> +/**
> + * ice_download_pkg
> + * @hw: pointer to the hardware structure
> + * @pkg_hdr: pointer to package header
> + * @ice_seg: pointer to the segment of the package to be downloaded
> + *
> + * Handles the download of a complete package.
> + */
> +static enum ice_ddp_state
> +ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr,
> +		 struct ice_seg *ice_seg)
> +{
> +	enum ice_ddp_state state;
> +
> +	if (hw->pkg_has_signing_seg)
> +		state = ice_download_pkg_with_sig_seg(hw, pkg_hdr);
> +	else
> +		state = ice_download_pkg_without_sig_seg(hw, ice_seg);
> +
>  	ice_post_pkg_dwnld_vlan_mode_cfg(hw);
>  
>  	return state;
> @@ -2083,7 +2183,7 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
>  
>  	/* initialize package hints and then download package */
>  	ice_init_pkg_hints(hw, seg);
> -	state = ice_download_pkg(hw, pkg);
> +	state = ice_download_pkg(hw, pkg, seg);
>  	if (state == ICE_DDP_PKG_ALREADY_LOADED) {
>  		ice_debug(hw, ICE_DBG_INIT,
>  			  "package previously loaded - no work.\n");
> 
> base-commit: 016b9332a3346e97a6cacffea0f9dc10e1235a75
> -- 
> 2.41.0
>
Wojciech Drewek Nov. 6, 2023, 9:44 a.m. UTC | #3 
On 04.11.2023 19:29, Paul Greenwalt wrote:
> From: Dan Nowlin <dan.nowlin@intel.com>
> 
> Commit 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> incorrectly removed support for package download for packages without a
> signature segment. These packages include the signature buffer inline
> in the configurations buffers, and do not in a signature segment.
> 
> Fix package download by providing download support for both packages
> with (ice_download_pkg_with_sig_seg()) and without signature segment
> (ice_download_pkg_without_sig_seg()).
> 
> Fixes: 3cbdb0343022 ("ice: Add support for E830 DDP package segment")
> Reported-by: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

> ---
>  drivers/net/ethernet/intel/ice/ice_ddp.c | 106 ++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
> index cfb1580f5850..3f1a11d0252c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
> @@ -1479,14 +1479,14 @@ ice_post_dwnld_pkg_actions(struct ice_hw *hw)
>  }
>  
>  /**
> - * ice_download_pkg
> + * ice_download_pkg_with_sig_seg
>   * @hw: pointer to the hardware structure
>   * @pkg_hdr: pointer to package header
>   *
>   * Handles the download of a complete package.
>   */
>  static enum ice_ddp_state
> -ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
> +ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
>  {
>  	enum ice_aq_err aq_err = hw->adminq.sq_last_status;
>  	enum ice_ddp_state state = ICE_DDP_PKG_ERR;
> @@ -1519,6 +1519,106 @@ ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
>  		state = ice_post_dwnld_pkg_actions(hw);
>  
>  	ice_release_global_cfg_lock(hw);
> +
> +	return state;
> +}
> +
> +/**
> + * ice_dwnld_cfg_bufs
> + * @hw: pointer to the hardware structure
> + * @bufs: pointer to an array of buffers
> + * @count: the number of buffers in the array
> + *
> + * Obtains global config lock and downloads the package configuration buffers
> + * to the firmware.
> + */
> +static enum ice_ddp_state
> +ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
> +{
> +	enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
> +	struct ice_buf_hdr *bh;
> +	int status;
> +
> +	if (!bufs || !count)
> +		return ICE_DDP_PKG_ERR;
> +
> +	/* If the first buffer's first section has its metadata bit set
> +	 * then there are no buffers to be downloaded, and the operation is
> +	 * considered a success.
> +	 */
> +	bh = (struct ice_buf_hdr *)bufs;
> +	if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF)
> +		return ICE_DDP_PKG_SUCCESS;
> +
> +	status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE);
> +	if (status) {
> +		if (status == -EALREADY)
> +			return ICE_DDP_PKG_ALREADY_LOADED;
> +		return ice_map_aq_err_to_ddp_state(hw->adminq.sq_last_status);
> +	}
> +
> +	state = ice_dwnld_cfg_bufs_no_lock(hw, bufs, 0, count, true);
> +	if (!state)
> +		state = ice_post_dwnld_pkg_actions(hw);
> +
> +	ice_release_global_cfg_lock(hw);
> +
> +	return state;
> +}
> +
> +/**
> + * ice_download_pkg_without_sig_seg
> + * @hw: pointer to the hardware structure
> + * @ice_seg: pointer to the segment of the package to be downloaded
> + *
> + * Handles the download of a complete package without signature segment.
> + */
> +static enum ice_ddp_state
> +ice_download_pkg_without_sig_seg(struct ice_hw *hw, struct ice_seg *ice_seg)
> +{
> +	struct ice_buf_table *ice_buf_tbl;
> +	enum ice_ddp_state state;
> +
> +	ice_debug(hw, ICE_DBG_PKG, "Segment format version: %d.%d.%d.%d\n",
> +		  ice_seg->hdr.seg_format_ver.major,
> +		  ice_seg->hdr.seg_format_ver.minor,
> +		  ice_seg->hdr.seg_format_ver.update,
> +		  ice_seg->hdr.seg_format_ver.draft);
> +
> +	ice_debug(hw, ICE_DBG_PKG, "Seg: type 0x%X, size %d, name %s\n",
> +		  le32_to_cpu(ice_seg->hdr.seg_type),
> +		  le32_to_cpu(ice_seg->hdr.seg_size), ice_seg->hdr.seg_id);
> +
> +	ice_buf_tbl = ice_find_buf_table(ice_seg);
> +
> +	ice_debug(hw, ICE_DBG_PKG, "Seg buf count: %d\n",
> +		  le32_to_cpu(ice_buf_tbl->buf_count));
> +
> +	state = ice_dwnld_cfg_bufs(hw, ice_buf_tbl->buf_array,
> +				   le32_to_cpu(ice_buf_tbl->buf_count));
> +
> +	return state;
> +}
> +
> +/**
> + * ice_download_pkg
> + * @hw: pointer to the hardware structure
> + * @pkg_hdr: pointer to package header
> + * @ice_seg: pointer to the segment of the package to be downloaded
> + *
> + * Handles the download of a complete package.
> + */
> +static enum ice_ddp_state
> +ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr,
> +		 struct ice_seg *ice_seg)
> +{
> +	enum ice_ddp_state state;
> +
> +	if (hw->pkg_has_signing_seg)
> +		state = ice_download_pkg_with_sig_seg(hw, pkg_hdr);
> +	else
> +		state = ice_download_pkg_without_sig_seg(hw, ice_seg);
> +
>  	ice_post_pkg_dwnld_vlan_mode_cfg(hw);
>  
>  	return state;
> @@ -2083,7 +2183,7 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
>  
>  	/* initialize package hints and then download package */
>  	ice_init_pkg_hints(hw, seg);
> -	state = ice_download_pkg(hw, pkg);
> +	state = ice_download_pkg(hw, pkg, seg);
>  	if (state == ICE_DDP_PKG_ALREADY_LOADED) {
>  		ice_debug(hw, ICE_DBG_INIT,
>  			  "package previously loaded - no work.\n");
> 
> base-commit: 016b9332a3346e97a6cacffea0f9dc10e1235a75
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index cfb1580f5850..3f1a11d0252c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -1479,14 +1479,14 @@  ice_post_dwnld_pkg_actions(struct ice_hw *hw)
 }
 
 /**
- * ice_download_pkg
+ * ice_download_pkg_with_sig_seg
  * @hw: pointer to the hardware structure
  * @pkg_hdr: pointer to package header
  *
  * Handles the download of a complete package.
  */
 static enum ice_ddp_state
-ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
+ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
 {
 	enum ice_aq_err aq_err = hw->adminq.sq_last_status;
 	enum ice_ddp_state state = ICE_DDP_PKG_ERR;
@@ -1519,6 +1519,106 @@  ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr)
 		state = ice_post_dwnld_pkg_actions(hw);
 
 	ice_release_global_cfg_lock(hw);
+
+	return state;
+}
+
+/**
+ * ice_dwnld_cfg_bufs
+ * @hw: pointer to the hardware structure
+ * @bufs: pointer to an array of buffers
+ * @count: the number of buffers in the array
+ *
+ * Obtains global config lock and downloads the package configuration buffers
+ * to the firmware.
+ */
+static enum ice_ddp_state
+ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
+{
+	enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
+	struct ice_buf_hdr *bh;
+	int status;
+
+	if (!bufs || !count)
+		return ICE_DDP_PKG_ERR;
+
+	/* If the first buffer's first section has its metadata bit set
+	 * then there are no buffers to be downloaded, and the operation is
+	 * considered a success.
+	 */
+	bh = (struct ice_buf_hdr *)bufs;
+	if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF)
+		return ICE_DDP_PKG_SUCCESS;
+
+	status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE);
+	if (status) {
+		if (status == -EALREADY)
+			return ICE_DDP_PKG_ALREADY_LOADED;
+		return ice_map_aq_err_to_ddp_state(hw->adminq.sq_last_status);
+	}
+
+	state = ice_dwnld_cfg_bufs_no_lock(hw, bufs, 0, count, true);
+	if (!state)
+		state = ice_post_dwnld_pkg_actions(hw);
+
+	ice_release_global_cfg_lock(hw);
+
+	return state;
+}
+
+/**
+ * ice_download_pkg_without_sig_seg
+ * @hw: pointer to the hardware structure
+ * @ice_seg: pointer to the segment of the package to be downloaded
+ *
+ * Handles the download of a complete package without signature segment.
+ */
+static enum ice_ddp_state
+ice_download_pkg_without_sig_seg(struct ice_hw *hw, struct ice_seg *ice_seg)
+{
+	struct ice_buf_table *ice_buf_tbl;
+	enum ice_ddp_state state;
+
+	ice_debug(hw, ICE_DBG_PKG, "Segment format version: %d.%d.%d.%d\n",
+		  ice_seg->hdr.seg_format_ver.major,
+		  ice_seg->hdr.seg_format_ver.minor,
+		  ice_seg->hdr.seg_format_ver.update,
+		  ice_seg->hdr.seg_format_ver.draft);
+
+	ice_debug(hw, ICE_DBG_PKG, "Seg: type 0x%X, size %d, name %s\n",
+		  le32_to_cpu(ice_seg->hdr.seg_type),
+		  le32_to_cpu(ice_seg->hdr.seg_size), ice_seg->hdr.seg_id);
+
+	ice_buf_tbl = ice_find_buf_table(ice_seg);
+
+	ice_debug(hw, ICE_DBG_PKG, "Seg buf count: %d\n",
+		  le32_to_cpu(ice_buf_tbl->buf_count));
+
+	state = ice_dwnld_cfg_bufs(hw, ice_buf_tbl->buf_array,
+				   le32_to_cpu(ice_buf_tbl->buf_count));
+
+	return state;
+}
+
+/**
+ * ice_download_pkg
+ * @hw: pointer to the hardware structure
+ * @pkg_hdr: pointer to package header
+ * @ice_seg: pointer to the segment of the package to be downloaded
+ *
+ * Handles the download of a complete package.
+ */
+static enum ice_ddp_state
+ice_download_pkg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr,
+		 struct ice_seg *ice_seg)
+{
+	enum ice_ddp_state state;
+
+	if (hw->pkg_has_signing_seg)
+		state = ice_download_pkg_with_sig_seg(hw, pkg_hdr);
+	else
+		state = ice_download_pkg_without_sig_seg(hw, ice_seg);
+
 	ice_post_pkg_dwnld_vlan_mode_cfg(hw);
 
 	return state;
@@ -2083,7 +2183,7 @@  enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
 
 	/* initialize package hints and then download package */
 	ice_init_pkg_hints(hw, seg);
-	state = ice_download_pkg(hw, pkg);
+	state = ice_download_pkg(hw, pkg, seg);
 	if (state == ICE_DDP_PKG_ALREADY_LOADED) {
 		ice_debug(hw, ICE_DBG_INIT,
 			  "package previously loaded - no work.\n");