Re: From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

[Tomasz Lichwala <tomasz.lichwala@linux.intel.com>]

On 17.09.2026 11:43, Aleksandr Loktionov wrote:
> diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
> index 21f3b61..9ae7de2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_nvm.c
> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
> @@ -1105,6 +1105,45 @@ static int ice_determine_css_hdr_len(struct ice_hw *hw)
>  	return 0;
>  }
>  
> +/**
> + * ice_parse_erot_presence - detect eRoT and populate hw->erot_present
> + * @hw: pointer to the HW struct
> + *
> + * Uses the device capability LIBIE_AQC_CAPS_EXTERNAL_PQC_ROT_PRESENT when
> + * advertised by firmware, falling back to the eRoT Presence fuse only when
> + * the capability is not advertised at all (older firmware).
> + *
> + * Priority:
> + *  1. Device capability external_pqc_rot_present (advertised && == 1)
> + *     => eRoT present
> + *  2. Fuse BIT(0) set (only when capability not advertised)
> + *     => eRoT present
> + *  3. Otherwise => eRoT not present
> + */
> +void ice_parse_erot_presence(struct ice_hw *hw)
> +{
> +	u16 fuse = 0;
> +	int err;
> +
> +	hw->erot_present = false;
> +
> +	if (hw->dev_caps.common_cap.external_pqc_rot_present) {
> +		hw->erot_present = true;
> +	} else if (!hw->dev_caps.common_cap.external_pqc_rot_present_cap_advertised) {
> +		err = ice_read_sr_word(hw, ICE_SR_EROT_PRESENCE_FUSE, &fuse);
> +		if (err)

On a read failure the code assumes erot_present = false, i.e. fails open on a mechanism whose whole purpose is preventing bricking from partial updates. A transient SR read failure on real eRoT hardware silently disables the guard in patch 2. Consider treating a read failure as erot_present = true instead, or justify why fail-open is safe here.

> +			dev_warn(ice_hw_to_dev(hw),
> +				 "Unable to read eRoT presence fuse (SR 0x%04x), err %d; assuming eRoT not present\n",
> +				 ICE_SR_EROT_PRESENCE_FUSE, err);
> +		else if (fuse & ICE_EROT_PRESENCE_FUSE_PRESENT)
> +			hw->erot_present = true;
> +	}
> +
> +	if (hw->erot_present)
> +		dev_info(ice_hw_to_dev(hw),
> +			 "eRoT (external Root of Trust) present\n");
> +}
> +
>  /**
>   * ice_init_nvm - initializes NVM setting
>   * @hw: pointer to the HW struct
> diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.h b/drivers/net/ethernet/intel/ice/ice_nvm.h
> index e1d1a11..4b31dfc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_nvm.h
> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.h
> @@ -28,7 +28,12 @@ int ice_get_inactive_nvm_ver(struct ice_hw *hw, struct ice_nvm_info *nvm);
>  int
>  ice_get_inactive_netlist_ver(struct ice_hw *hw, struct ice_netlist_info *netlist);
>  int ice_read_pba_string(struct ice_hw *hw, u8 *pba_num, u32 pba_num_size);
> +/* eRoT Presence fuse SR offset; BIT(0) set means eRoT present */
> +#define ICE_SR_EROT_PRESENCE_FUSE	0x1016
> +#define ICE_EROT_PRESENCE_FUSE_PRESENT	BIT(0)

Commit message and function doc describe the fuse as "bits[1:0]", but only bit 0 is checked/masked. Please confirm with the fuse spec whether bit 1 is truly unused, or whether this needs GENMASK(1,0) with a specific expected value.

> +
>  int ice_init_nvm(struct ice_hw *hw);
> +void ice_parse_erot_presence(struct ice_hw *hw);
>  int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data);
>  int
>  ice_aq_update_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset,