All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iwl-next v2 0/2] ice: eRoT adapter NVM update guard
@ 2026-10-08 14:14 Aleksandr Loktionov
  2026-10-08 14:14 ` [PATCH iwl-next v2 1/2] ice: parse external Root of Trust capability Aleksandr Loktionov
  2026-10-08 14:14 ` [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters Aleksandr Loktionov
  0 siblings, 2 replies; 7+ messages in thread
From: Aleksandr Loktionov @ 2026-10-08 14:14 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov

Parse the external Root of Trust capability and use a fresh device-capability
snapshot for each flash attempt. When firmware reports eRoT presence,
require NVM, OROM, NetList and Manifest together. Reject single-component
requests, duplicates and incomplete images before erase, and reject an
empty applicable component list before activation.

An omitted capability is treated as absent, preserving legacy full-image
and fw.mgmt component updates in normal and recovery modes. Manifest is
accepted only when firmware explicitly reports eRoT presence. Older eRoT
hardware whose firmware omits the capability is not protected by these
early completeness checks. No fuse fallback or hardware-ID inference is
used.

Explicit values other than 0 or 1 and capability-discovery errors stop the
attempt before pending-update cancellation or the NVM write lock. Recovery
mode bypasses only the unified-update capability check. Cancellation
already calls capability discovery in the base driver, so that query must
work for recovery flashing even without this series. Its error handling
is unchanged.

The AQ activation-select mask defines NVM, OROM and NetList bits, but no
Manifest bit; the Manifest flag remains zero. This interface limitation
does not establish a physical layout or implicit activation with NVM.
Manifest activation timing and the firmware contract still need
confirmation. No firmware specification was supplied for that contract,
and this series has not been hardware validated. The component checks do
not authenticate images or make updates atomic.

Changes in this v2 respin:
  - Treat omission as absent; retain a separate invalid-value bit.
  - Define and initialize the scoped capability allocation together after
    overwrite-mask validation, without changing lock scopes.
  - Explain the missing Manifest selector with a one-line comment.
  - Document the legacy omission limitation and recovery query dependency.
  - Preserve the component and finalization guards, original author dates,
    and two-patch split. No review tags are carried.

Validation: source-extracted executable parser and update tests, including
automatic-cleanup accounting; strict checkpatch, kernel-doc and whitespace
checks; actual W=1 ice.ko links for both commits with matching base config
and symbol versions. The tests mock firmware I/O and PLDM image parsing;
they do not establish the hardware or firmware behavior described above.

Aleksandr Loktionov (2):
  ice: parse external Root of Trust capability
  ice: reject partial NVM update on eRoT adapters

 Documentation/networking/devlink/ice.rst      | 21 +++++
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
 drivers/net/ethernet/intel/ice/ice_common.c   |  6 ++
 .../net/ethernet/intel/ice/ice_fw_update.c    | 87 ++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_type.h     |  3 +
 include/linux/net/intel/libie/adminq.h        |  1 +
 6 files changed, 115 insertions(+), 4 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH iwl-next v2 1/2] ice: parse external Root of Trust capability
  2026-10-08 14:14 [PATCH iwl-next v2 0/2] ice: eRoT adapter NVM update guard Aleksandr Loktionov
@ 2026-10-08 14:14 ` Aleksandr Loktionov
  2026-10-15 11:57   ` Tomasz Lichwala
  2026-10-08 14:14 ` [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters Aleksandr Loktionov
  1 sibling, 1 reply; 7+ messages in thread
From: Aleksandr Loktionov @ 2026-10-08 14:14 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov

Reserve the external PQC Root of Trust capability ID in the shared libie
capability list and parse its value into the common capabilities.

Firmware reports 1 for present and 0 for absent. Treat an omitted
capability as absent to preserve legacy behavior. Keep a separate invalid
bit so consumers can reject an explicitly reported unknown encoding
instead of treating it as absence. Log the reported value for debugging.

Leave the decision to the capability consumer. No NVM layout assumptions
or probe-time derived state are needed.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_common.c | 6 ++++++
 drivers/net/ethernet/intel/ice/ice_type.h   | 2 ++
 include/linux/net/intel/libie/adminq.h      | 1 +
 3 files changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 6bde8f5..f1262d9 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -2484,6 +2484,12 @@ ice_parse_common_caps(struct ice_hw *hw, struct ice_hw_common_caps *caps,
 	case LIBIE_AQC_CAPS_TX_SCHED_TOPO_COMP_MODE:
 		caps->tx_sched_topo_comp_mode_en = (number == 1);
 		break;
+	case LIBIE_AQC_CAPS_EXTERNAL_PQC_ROT_PRESENT:
+		caps->external_pqc_rot_present = (number == 1);
+		caps->external_pqc_rot_invalid = (number != 0 && number != 1);
+		ice_debug(hw, ICE_DBG_INIT, "%s: external_pqc_rot = %u\n",
+			  prefix, number);
+		break;
 	default:
 		/* Not one of the recognized common capabilities */
 		found = false;
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 67b90c0..1fede0f 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -311,6 +311,8 @@ struct ice_hw_common_caps {
 	/* Post update reset restriction */
 	bool reset_restrict_support;
 	bool tx_sched_topo_comp_mode_en;
+	bool external_pqc_rot_present;
+	bool external_pqc_rot_invalid;
 };
 
 /* IEEE 1588 TIME_SYNC specific info */
diff --git a/include/linux/net/intel/libie/adminq.h b/include/linux/net/intel/libie/adminq.h
index 839114d..df42f5b 100644
--- a/include/linux/net/intel/libie/adminq.h
+++ b/include/linux/net/intel/libie/adminq.h
@@ -175,6 +175,7 @@ LIBIE_CHECK_STRUCT_LEN(16, libie_aqc_list_caps);
 #define LIBIE_AQC_CAPS_PENDING_OROM_VER			0x004B
 #define LIBIE_AQC_CAPS_NET_VER				0x004C
 #define LIBIE_AQC_CAPS_PENDING_NET_VER			0x004D
+#define LIBIE_AQC_CAPS_EXTERNAL_PQC_ROT_PRESENT		0x004E
 #define LIBIE_AQC_CAPS_RDMA				0x0051
 #define LIBIE_AQC_CAPS_LED				0x0061
 #define LIBIE_AQC_CAPS_SDP				0x0062
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters
  2026-10-08 14:14 [PATCH iwl-next v2 0/2] ice: eRoT adapter NVM update guard Aleksandr Loktionov
  2026-10-08 14:14 ` [PATCH iwl-next v2 1/2] ice: parse external Root of Trust capability Aleksandr Loktionov
@ 2026-10-08 14:14 ` Aleksandr Loktionov
  2026-10-13 21:51   ` Tony Nguyen
                     ` (2 more replies)
  1 sibling, 3 replies; 7+ messages in thread
From: Aleksandr Loktionov @ 2026-10-08 14:14 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov

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>
---
 Documentation/networking/devlink/ice.rst      | 21 +++++
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
 .../net/ethernet/intel/ice/ice_fw_update.c    | 87 ++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_type.h     |  1 +
 4 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst
index 792e9f8..e03630a 100644
--- a/Documentation/networking/devlink/ice.rst
+++ b/Documentation/networking/devlink/ice.rst
@@ -205,6 +205,27 @@ The ``ice`` driver implements support for flash update using the
 combined flash image that contains the ``fw.mgmt``, ``fw.undi``, and
 ``fw.netlist`` components.
 
+When firmware reports an external Root of Trust (eRoT) as present, the
+image must also contain the ``fw.pqc_manifest`` component, and all four
+components must be updated together. Images missing any of them, or
+containing a component more than once, are rejected before any flash bank
+is erased. An empty applicable component list is rejected before
+activation. Updating a single component (``component fw.mgmt``) is not
+supported when eRoT presence is reported. ``fw.pqc_manifest`` is accepted
+only when firmware reports eRoT presence. These checks do not authenticate
+the image contents or make the update atomic.
+
+Before each flash attempt, the driver reads device capabilities from
+firmware. An omitted eRoT capability is treated as absent, preserving
+legacy full-image and ``component fw.mgmt`` updates. Older eRoT hardware
+whose firmware omits the capability is not protected by the early
+completeness checks. An explicitly reported value other than 0 (absent)
+or 1 (present), or failure to read capabilities, blocks the update before
+any pending update is canceled or a flash write lock is acquired. This
+policy also applies in recovery mode, which only bypasses the separate
+unified-update capability check. Pending-update cancellation already
+requires capability discovery, including in recovery mode.
+
 .. list-table:: List of supported overwrite modes
    :widths: 5 95
 
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 848ac58..072cd0b 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1748,6 +1748,7 @@ struct ice_aqc_nvm_comp_tbl {
 #define NVM_COMP_ID_OROM	0x5
 #define NVM_COMP_ID_NVM		0x6
 #define NVM_COMP_ID_NETLIST	0x8
+#define NVM_COMP_ID_MANIFEST	0x12
 
 	u8 comp_class_idx;
 #define FWU_COMP_CLASS_IDX_NOT_USE 0x0
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
@@ -9,6 +9,13 @@
 #include "ice_lib.h"
 #include "ice_fw_update.h"
 
+#define ICE_FWU_COMP_NVM		BIT(0)
+#define ICE_FWU_COMP_OROM		BIT(1)
+#define ICE_FWU_COMP_NETLIST	BIT(2)
+#define ICE_FWU_COMP_MANIFEST	BIT(3)
+#define ICE_FWU_COMP_ALL_EROT	(ICE_FWU_COMP_NVM | ICE_FWU_COMP_OROM | \
+				 ICE_FWU_COMP_NETLIST | ICE_FWU_COMP_MANIFEST)
+
 struct ice_fwu_priv {
 	struct pldmfw context;
 
@@ -29,6 +36,8 @@ struct ice_fwu_priv {
 
 	/* Track if EMP reset is available */
 	u8 emp_reset_available;
+	u8 seen_components;
+	bool erot_present;
 };
 
 /**
@@ -108,6 +117,9 @@ ice_check_component_response(struct ice_pf *pf, u16 id, u8 response, u8 code,
 	case NVM_COMP_ID_NETLIST:
 		component = "fw.netlist";
 		break;
+	case NVM_COMP_ID_MANIFEST:
+		component = "fw.pqc_manifest";
+		break;
 	default:
 		WARN(1, "Unexpected unknown component identifier 0x%02x", id);
 		return -EINVAL;
@@ -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",
+			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;
+	}
+
 	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";
+		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);
+	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;
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 */
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters
  2026-10-08 14:14 ` [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters Aleksandr Loktionov
@ 2026-10-13 21:51   ` Tony Nguyen
  2026-10-14 22:59   ` Przemek Kitszel
  2026-10-15 11:58   ` Tomasz Lichwala
  2 siblings, 0 replies; 7+ messages in thread
From: Tony Nguyen @ 2026-10-13 21:51 UTC (permalink / raw)
  To: Aleksandr Loktionov, intel-wired-lan



On 10/8/2026 7:14 AM, Aleksandr Loktionov wrote:

...

> @@ -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");

Hi Alex,

I don't think this should be reported to both:

"
A number of driver configuration interfaces pass a Netlink extended ACK 
(extack) object to the driver (either directly as an argument or as a 
member of a parameter struct). The drivers should try to report most 
errors via the extack object. System level exceptions, indicating that 
system or device is misbehaving or is in bad state, should continue to 
be reported to system logs.

Messages should be passed either via extack or to system logs. Drivers 
should not try to report the same information to both.
"
https://docs.kernel.org/networking/driver.html#error-message-reporting

> +			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",

...

> @@ -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);

__free() is marked as deprecated by netdev now so we should probably 
avoid it if we can. Also, this is missing the separating newline for 
declarations.

Thanks,
Tony

> +	if (!dev_caps)
> +		return -ENOMEM;
> +




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters
  2026-10-08 14:14 ` [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters Aleksandr Loktionov
  2026-10-13 21:51   ` Tony Nguyen
@ 2026-10-14 22:59   ` Przemek Kitszel
  2026-10-15 11:58   ` Tomasz Lichwala
  2 siblings, 0 replies; 7+ messages in thread
From: Przemek Kitszel @ 2026-10-14 22:59 UTC (permalink / raw)
  To: Aleksandr Loktionov, intel-wired-lan, anthony.l.nguyen

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 */


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iwl-next v2 1/2] ice: parse external Root of Trust capability
  2026-10-08 14:14 ` [PATCH iwl-next v2 1/2] ice: parse external Root of Trust capability Aleksandr Loktionov
@ 2026-10-15 11:57   ` Tomasz Lichwala
  0 siblings, 0 replies; 7+ messages in thread
From: Tomasz Lichwala @ 2026-10-15 11:57 UTC (permalink / raw)
  To: Aleksandr Loktionov, intel-wired-lan, anthony.l.nguyen



On 8.10.2026 16:14, Aleksandr Loktionov wrote:
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 6bde8f5..f1262d9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -2484,6 +2484,12 @@ ice_parse_common_caps(struct ice_hw *hw, struct ice_hw_common_caps *caps,
>  	case LIBIE_AQC_CAPS_TX_SCHED_TOPO_COMP_MODE:
>  		caps->tx_sched_topo_comp_mode_en = (number == 1);
>  		break;
> +	case LIBIE_AQC_CAPS_EXTERNAL_PQC_ROT_PRESENT:
> +		caps->external_pqc_rot_present = (number == 1);
> +		caps->external_pqc_rot_invalid = (number != 0 && number != 1);

Nit: plain assignment, not sticky - if firmware ever emitted this capability entry twice in one response, a later valid value would silently clear a prior "invalid" verdict. Consider |= if "fail closed" is meant to be strict.

> +		ice_debug(hw, ICE_DBG_INIT, "%s: external_pqc_rot = %u\n",
> +			  prefix, number);
> +		break;
>  	default:
>  		/* Not one of the recognized common capabilities */
>  		found = false;

Thanks,
Tomasz


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters
  2026-10-08 14:14 ` [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters Aleksandr Loktionov
  2026-10-13 21:51   ` Tony Nguyen
  2026-10-14 22:59   ` Przemek Kitszel
@ 2026-10-15 11:58   ` Tomasz Lichwala
  2 siblings, 0 replies; 7+ messages in thread
From: Tomasz Lichwala @ 2026-10-15 11:58 UTC (permalink / raw)
  To: Aleksandr Loktionov, intel-wired-lan, anthony.l.nguyen



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-15 11:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 14:14 [PATCH iwl-next v2 0/2] ice: eRoT adapter NVM update guard Aleksandr Loktionov
2026-10-08 14:14 ` [PATCH iwl-next v2 1/2] ice: parse external Root of Trust capability Aleksandr Loktionov
2026-10-15 11:57   ` Tomasz Lichwala
2026-10-08 14:14 ` [PATCH iwl-next v2 2/2] ice: reject partial NVM update on eRoT adapters Aleksandr Loktionov
2026-10-13 21:51   ` Tony Nguyen
2026-10-14 22:59   ` Przemek Kitszel
2026-10-15 11:58   ` Tomasz Lichwala

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.