Re: [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 10/8/26 16:14, Aleksandr Loktionov wrote:
> Require NVM, OROM, NetList and Manifest together when firmware reports
> eRoT presence. Track the component IDs while sending the PLDM component
> tables and reject an incomplete set before erasing a bank. Reject
> duplicates only for eRoT updates, leaving ordinary duplicate handling
> unchanged.
> 
> Read fresh device capabilities for each flash attempt before canceling
> a pending update or acquiring the NVM write lock. Use the confirmed eRoT
> presence as a per-update snapshot for component selection and validation.
> This avoids depending on probe-time state after reset or in recovery,
> where normal hardware initialization is skipped. Use the same fresh
> query for the unified-update check, retaining its recovery bypass.
> 
> Treat an omitted capability as absent, retaining legacy full-image and
> fw.mgmt component updates. Enable eRoT-specific restrictions only when
> firmware explicitly reports presence. Older eRoT hardware whose firmware
> omits the capability is not protected by the early completeness checks.
> 
> Propagate discovery errors and reject explicitly reported values other
> than 0 or 1 before pending-update cancellation or the NVM write lock.
> Neither case reaches package transfer, erase, write or activation.
> Recovery mode does not bypass this check or require full hardware
> initialization. Pending-update cancellation already discovers device
> capabilities, so recovery flashing required that query before this series.
> 
> Reject single-component requests up front when eRoT presence is reported.
> Keep the completeness check independent of the PLDM END flag: component
> filtering can omit the component that would carry END. Also check before
> finalization, since an empty applicable component list skips the flash
> callback but still reaches bank activation.
> 
> Accept Manifest only with confirmed eRoT presence. Its activation flag
> is zero: the AQ activation-select mask has bits for NVM, OROM and NetList,
> but none for Manifest. This does not establish its activation timing.
> Define its bank pointer, without adding an unused bank-size constant.
> 
> Document the component rule and the capability-only policy in normal
> and recovery modes. These checks neither authenticate image contents
> nor make the update atomic.
> 
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

IMO this series have not changed that much from human reviewer
perspective to warrant dropping Reviewed-by tags. Just some more
LLM-driven hardening against LLM-reported edge cases.

[...]

> @@ -1013,12 +1067,37 @@ int ice_devlink_flash_update(struct devlink *devlink,
>   		return -EOPNOTSUPP;
>   	}
>   
> -	if (!hw->dev_caps.common_cap.nvm_unified_update && !ice_is_recovery_mode(hw)) {
> +	struct ice_hw_dev_caps *dev_caps __free(kfree) =
> +		kzalloc_obj(*dev_caps);

openly declaring variables mid-function is ugly, also
<insert the usual __free rambling />

> +	if (!dev_caps)
> +		return -ENOMEM;
> +
> +	err = ice_discover_dev_caps(hw, dev_caps);
> +	if (err) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Cannot determine eRoT state: unable to read firmware device capabilities");
> +		return err;
> +	}
> +
> +	if (dev_caps->common_cap.external_pqc_rot_invalid) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Invalid eRoT capability reported by firmware");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (!dev_caps->common_cap.nvm_unified_update && !ice_is_recovery_mode(hw)) {
>   		NL_SET_ERR_MSG_MOD(extack, "Current firmware does not support unified update");
>   		return -EOPNOTSUPP;
>   	}
>   
>   	memset(&priv, 0, sizeof(priv));
> +	priv.erot_present = dev_caps->common_cap.external_pqc_rot_present;
> +

nit: no blank line

> +	if (params->component && priv.erot_present) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "eRoT adapter does not support single component update");
> +		return -EOPNOTSUPP;
> +	}
>   
>   	if (params->component && strcmp(params->component, "fw.mgmt") == 0) {
>   		priv.context.mode = PLDMFW_UPDATE_MODE_SINGLE_COMPONENT;
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
> index 1fede0f..43f3208 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -1151,6 +1151,7 @@ struct ice_aq_get_set_rss_lut_params {
>   #define ICE_SR_OROM_BANK_SIZE		0x45
>   #define ICE_SR_NETLIST_BANK_PTR		0x46
>   #define ICE_SR_NETLIST_BANK_SIZE	0x47
> +#define ICE_SR_PQC_MANIFEST_BANK_PTR	0x4E
>   #define ICE_SR_SECTOR_SIZE_IN_WORDS	0x800
>   
>   /* CSS Header words */