On 10/27/26 6:45 PM, Aleksandr Loktionov wrote:
An eRoT authenticates the complete NVM image, so a PLDM capsule that
carries only part of the components is not a valid update on such an
adapter. Report that from the driver: it knows the components an eRoT
adapter needs, so it can reject the image with a reason in the extended
ACK before anything is written to the device.

Reject a repeated component and a component without data while the
component tables are sent, and require all four components before the
first bank is erased. Check the component set again before activation,
because an image whose device record selects no component at all skips
the flash path and still reaches that step. Reject a single component
request up front, and accept the Manifest component only when firmware
reports eRoT presence. Adapters whose firmware does not report eRoT
keep their current handling.

Read the device capabilities once per flash attempt and act on that
snapshot, then reuse it for the pending update check the attempt
already makes, so an update still costs one List Device Capabilities
round trip. The cached copy in hw->dev_caps cannot decide this: a
driver loaded while firmware was already in recovery mode never reads
it, and a reset re-parses it in place. An explicit capability value
other than 0 or 1 rejects every update, because an answer the driver
cannot interpret is not evidence that the image is complete.

No activation is requested for the Manifest: the AQ activation select
mask and the pending update capabilities have no Manifest entry, so the
driver writes the bank and leaves the activation flag zero. It can
neither activate nor roll back that bank, which is documented along
with the other eRoT update limits.

Signed-off-by: Aleksandr Loktionov <[email protected]>
---
  Documentation/networking/devlink/ice.rst      |  24 +++
  .../net/ethernet/intel/ice/ice_adminq_cmd.h   |   1 +
  .../net/ethernet/intel/ice/ice_fw_update.c    | 193 +++++++++++++++---
  drivers/net/ethernet/intel/ice/ice_type.h     |   1 +
  4 files changed, 192 insertions(+), 27 deletions(-)


two nitpicks, otherwise the whole series is fine, feel free to add my RB
to both patches:
Reviewed-by: Przemek Kitszel <[email protected]>

@@ -994,6 +1093,7 @@ int ice_devlink_flash_update(struct devlink *devlink,
        struct ice_pf *pf = devlink_priv(devlink);
        struct device *dev = ice_pf_to_dev(pf);
        struct ice_hw *hw = &pf->hw;
+       struct ice_hw_dev_caps *dev_caps;

RCT

        struct ice_fwu_priv priv;
        u8 preservation;
        int err;
@@ -1013,18 +1113,52 @@ int ice_devlink_flash_update(struct devlink *devlink,
                return -EOPNOTSUPP;
        }
- if (!hw->dev_caps.common_cap.nvm_unified_update && !ice_is_recovery_mode(hw)) {
+       dev_caps = kzalloc_obj(*dev_caps);
+       if (!dev_caps)
+               return -ENOMEM;
+
+       /* Read the most recent device capabilities from firmware. Do not use
+        * the cached values in hw->dev_caps: a driver loaded while firmware
+        * was already in recovery mode never reads them, and a reset may be
+        * rebuilding them while this request runs. The same read serves the
+        * pending update check below, so this costs one AQ command per flash
+        * attempt, as it did before.
+        */
+       err = ice_discover_dev_caps(hw, dev_caps);
+       if (err) {
+               NL_SET_ERR_MSG_MOD(extack, "Unable to read device 
capabilities");
+               kfree(dev_caps);
+               return err;

goto free_caps instead

+       }
+
+       if (dev_caps->common_cap.external_pqc_rot_invalid) {
+               NL_SET_ERR_MSG_MOD(extack, "Invalid eRoT capability reported by 
firmware");
+               err = -EIO;
+               goto free_caps;
+       }

[...]

@@ -1066,4 +1201,8 @@ int ice_devlink_flash_update(struct devlink *devlink,
        ice_release_nvm(hw);
return err;
+
+free_caps:
+       kfree(dev_caps);
+       return err;
  }

Reply via email to