On 8.10.2026 16:14, Aleksandr Loktionov wrote:
> diff --git a/drivers/net/ethernet/intel/ice/ice_fw_update.c
> b/drivers/net/ethernet/intel/ice/ice_fw_update.c
> index 3631461..9da7df6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_fw_update.c
> +++ b/drivers/net/ethernet/intel/ice/ice_fw_update.c
> @@ -220,12 +232,27 @@ ice_send_component_table(struct pldmfw *context, struct
> pldmfw_component *compon
> struct ice_pf *pf = priv->pf;
> struct ice_hw *hw = &pf->hw;
> size_t length;
> + u8 comp_bit;
> int status;
>
> switch (component->identifier) {
> case NVM_COMP_ID_OROM:
> + comp_bit = ICE_FWU_COMP_OROM;
> + break;
> case NVM_COMP_ID_NVM:
> + comp_bit = ICE_FWU_COMP_NVM;
> + break;
> case NVM_COMP_ID_NETLIST:
> + comp_bit = ICE_FWU_COMP_NETLIST;
> + break;
> + case NVM_COMP_ID_MANIFEST:
> + if (!priv->erot_present) {
> + dev_err(dev, "Manifest component is not supported on
> adapters without eRoT\n");
> + NL_SET_ERR_MSG_MOD(extack,
> + "Manifest component is not supported
> on adapters without eRoT");
> + return -EOPNOTSUPP;
> + }
> + comp_bit = ICE_FWU_COMP_MANIFEST;
> break;
> default:
> dev_err(dev, "Unable to update due to a firmware component with
> unknown ID %u\n",
> @@ -234,6 +261,14 @@ ice_send_component_table(struct pldmfw *context, struct
> pldmfw_component *compon
> return -EOPNOTSUPP;
> }
>
> + if (priv->erot_present && (priv->seen_components & comp_bit)) {
> + dev_err(dev, "Duplicate component in PLDM image: component ID
> 0x%02x\n",
Nit: same dual dev_err()/extack reporting pattern already flagged for the
Manifest-without-eRoT case above - please fix both occurrences together.
> + component->identifier);
> + NL_SET_ERR_MSG_MOD(extack, "Duplicate component in PLDM image");
> + return -EOPNOTSUPP;
> + }
> + priv->seen_components |= comp_bit;
> +
> length = struct_size(comp_tbl, cvs, component->version_len);
> comp_tbl = kzalloc(length, GFP_KERNEL);
> if (!comp_tbl)
> @@ -648,6 +683,12 @@ ice_flash_component(struct pldmfw *context, struct
> pldmfw_component *component)
> u8 flag;
> int err;
>
> + if (priv->erot_present && priv->seen_components !=
> ICE_FWU_COMP_ALL_EROT) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "eRoT adapter requires all four components
> (NVM, OROM, NetList, Manifest) in a single update");
> + return -EINVAL;
Nit: duplicate-component rejection uses -EOPNOTSUPP while this (equally
malformed-image) case uses -EINVAL. Consider using one consistently.
> + }
> +
> switch (component->identifier) {
> case NVM_COMP_ID_OROM:
> module = ICE_SR_1ST_OROM_BANK_PTR;
> @@ -667,6 +708,13 @@ ice_flash_component(struct pldmfw *context, struct
> pldmfw_component *component)
> reset_level = NULL;
> name = "fw.netlist";
> break;
> + case NVM_COMP_ID_MANIFEST:
> + module = ICE_SR_PQC_MANIFEST_BANK_PTR;
> + /* The AQ activation-select mask has no Manifest bit. */
> + flag = 0;
> + reset_level = NULL;
> + name = "fw.pqc_manifest";
ICE_AQC_NVM_ACTIV_SEL_MASK only covers bits 3-5 (NVM/OROM/NetList), so there's
no activation bit for Manifest.
ice_get_pending_updates()/ice_cancel_pending_update() also have no notion of a
pending Manifest update, so canceling a pending 4-component eRoT update reverts
NVM/OROM/NetList but leaves Manifest state undefined. Please confirm the
firmware activation contract for Manifest before merging - the cover letter
itself says this is unconfirmed and not hardware-validated.
> + break;
> default:
> /* This should not trigger, since we check the id before
> * sending the component table to firmware.
> @@ -706,6 +754,12 @@ static int ice_finalize_update(struct pldmfw *context)
> struct devlink *devlink;
> int err;
>
> + if (priv->erot_present && priv->seen_components !=
> ICE_FWU_COMP_ALL_EROT) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "eRoT adapter requires all four components
> (NVM, OROM, NetList, Manifest) in a single update");
> + return -EINVAL;
> + }
> +
> /* Finally, notify firmware to activate the written NVM banks */
> err = ice_switch_flash_banks(pf, priv->activate_flags,
> &priv->emp_reset_available, extack);
> @@ -981,9 +1035,9 @@ ice_cancel_pending_update(struct ice_pf *pf, const char
> *component,
> * formatted image that matches this device.
> *
> * Extract the device record Package Data and Component Tables and send them
> - * to the firmware. Extract and write the flash data for each of the three
> - * main flash components, "fw.mgmt", "fw.undi", and "fw.netlist". Notify
> - * firmware once the data is written to the inactive banks.
> + * to the firmware. Extract and write the flash data for the selected
> firmware
> + * components: "fw.mgmt", "fw.undi", "fw.netlist", and, when firmware reports
> + * eRoT presence, "fw.pqc_manifest". Notify firmware once the data is
> written.
> *
> * Returns: zero on success or a negative error code on failure.
> */
> @@ -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);
> + if (!dev_caps)
> + return -ENOMEM;
> +
> + err = ice_discover_dev_caps(hw, dev_caps);
This duplicates the ice_discover_dev_caps() call already done inside
ice_get_pending_updates(), reached right after via ice_cancel_pending_update()
— two AQ list-caps round-trips per flash attempt. Also note
ice_switch_flash_banks()/ice_write_one_nvm_block() still read the cached
hw->dev_caps.common_cap.* instead of this fresh snapshot, so the "per-update
snapshot" isn't actually used consistently within the same update.
> + 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;
> +
> + 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;
priv.context.component_identifier = NVM_COMP_ID_NVM;
} else if (params->component) {
return -EOPNOTSUPP;
}
/* the E822 device needs a slightly different ops */
if (hw->mac_type == ICE_MAC_GENERIC)
priv.context.ops = &ice_fwu_ops_e822;
else
priv.context.ops = &ice_fwu_ops_e810;
priv.context.dev = dev;
priv.extack = extack;
priv.pf = pf;
priv.activate_flags = preservation;
devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL,
0, 0);
err = ice_cancel_pending_update(pf, NULL, extack);
This unconditionally reverts any previously staged pending update before the
new completeness/duplicate checks run (they only fire later, inside the pldmfw
callbacks). So a malformed eRoT image is rejected "before any bank is erased"
as documented, but only after a valid prior pending update has already been
reverted. Worth calling out explicitly in the docs, or checking completeness
before canceling.
if (err)
return err;
> 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 */
Thanks,
Tomasz
