Netdev List
 help / color / mirror / Atom feed
* [PATCH next-queue v1 0/2] ice: eRoT adapter NVM update guard
@ 2026-09-17  9:43 Aleksandr Loktionov
  2026-09-17  9:43 ` From: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Aleksandr Loktionov
  2026-09-17  9:43 ` Aleksandr Loktionov
  0 siblings, 2 replies; 5+ messages in thread
From: Aleksandr Loktionov @ 2026-09-17  9:43 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev

CNSA 2.0 PQC adapters carry an external Root of Trust (eRoT) controller
that performs full-image authentication on every NVM update.  The eRoT
does not allow updating individual components; it requires NVM, OROM,
NetList, and Manifest all together in one shot.  If you hand it a partial
PLDM capsule today, firmware rejects the write mid-session (i.e. partway
through the PLDM component transfer sequence), after the erase has
already been issued.

Patch 1 detects eRoT presence.  Newer firmware advertises it directly
via device capability 0x004E; older firmware does not, so there is an
NVM-based fallback that reads the eRoT presence fuse (SR 0x1016 bits[1:0]).
The result goes into hw->erot_present and is surfaced as ICE_F_EROT.

Patch 2 uses that flag to gate the flash path.  Component identifiers
are tracked as the PLDM component table is walked, and if the full
required set is not present when ice_flash_component() is first entered,
the whole update is rejected with -EINVAL and a netlink error message
before a single flash bank is touched.  The Manifest component itself is
only ever accepted on eRoT adapters in the first place.

Testing: validated on real eRoT-equipped hardware.

Aleksandr Loktionov (2):
  ice: detect eRoT presence via device capability
  ice: reject partial NVM update on eRoT adapters

 drivers/net/ethernet/intel/ice/ice.h          |  1 +
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
 drivers/net/ethernet/intel/ice/ice_common.c   |  7 ++
 .../net/ethernet/intel/ice/ice_fw_update.c    | 80 +++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_lib.c      |  3 +
 drivers/net/ethernet/intel/ice/ice_nvm.c      | 39 ++++++++++
 drivers/net/ethernet/intel/ice/ice_nvm.h      |  5 ++
 drivers/net/ethernet/intel/ice/ice_type.h     |  6 ++
 include/linux/net/intel/libie/adminq.h        |  1 +
 9 files changed, 143 insertions(+)

-- 
2.52.0

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

* From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
  2026-09-17  9:43 [PATCH next-queue v1 0/2] ice: eRoT adapter NVM update guard Aleksandr Loktionov
@ 2026-09-17  9:43 ` Aleksandr Loktionov
  2026-09-18 13:11   ` Tomasz Lichwala
  2026-09-17  9:43 ` Aleksandr Loktionov
  1 sibling, 1 reply; 5+ messages in thread
From: Aleksandr Loktionov @ 2026-09-17  9:43 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov
  Cc: netdev, 6b0e0deb9a688f5d6289c31b0eba49801deb832e, Mon, Sep, 17,
	00

CNSA 2.0 PQC adapters carry an external Root of Trust (eRoT) controller
that enforces full-image authentication during NVM updates.  Detect its
presence at init time so later patches can gate firmware update flows.

Primary detection uses the LIBIE_AQC_CAPS_EXTERNAL_PQC_ROT_PRESENT device
capability (0x004E) reported by firmware.  When running on older firmware
that does not advertise the capability at all, fall back to reading the
eRoT presence fuse (SR 0x1016 bits[1:0]).

The capability ID is reserved in the shared libie adminq.h rather than
defined locally in ice, alongside the other LIBIE_AQC_CAPS_* values that
ice_parse_common_caps() already switches on.  AQ capability IDs are a
single firmware-defined numbering space; reserving 0x004E centrally
prevents libie from later assigning it to an unrelated capability out
from under ice.

The result is stored in hw->erot_present and exposed through the
ICE_F_EROT feature bit.  ice_parse_erot_presence() is called from
ice_init_hw() after ice_get_caps() so the device capability field is
already populated at detection time.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h        |  1 +
 drivers/net/ethernet/intel/ice/ice_common.c |  7 ++++
 drivers/net/ethernet/intel/ice/ice_lib.c    |  3 ++
 drivers/net/ethernet/intel/ice/ice_nvm.c    | 39 +++++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_nvm.h    |  5 +++
 drivers/net/ethernet/intel/ice/ice_type.h   |  5 +++
 include/linux/net/intel/libie/adminq.h      |  1 +
 7 files changed, 61 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index db3c701..a78fa17 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -210,6 +210,7 @@ enum ice_feature {
 	ICE_F_SRIOV_LAG,
 	ICE_F_SRIOV_AA_LAG,
 	ICE_F_MBX_LIMIT,
+	ICE_F_EROT,
 	ICE_F_MAX
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index ef1ce10..1e0936d 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1031,6 +1031,7 @@ int ice_init_hw(struct ice_hw *hw)
 	if (status)
 		goto err_unroll_cqinit;
 
+	ice_parse_erot_presence(hw);
 	if (!hw->port_info)
 		hw->port_info = devm_kzalloc(ice_hw_to_dev(hw),
 					     sizeof(*hw->port_info),
@@ -2479,6 +2480,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_present_cap_advertised = true;
+		ice_debug(hw, ICE_DBG_INIT, "%s: external_pqc_rot_present = %d\n",
+			  prefix, caps->external_pqc_rot_present);
+		break;
 	default:
 		/* Not one of the recognized common capabilities */
 		found = false;
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 9e08db3..ad2b6fb 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -4003,6 +4003,9 @@ void ice_init_feature_support(struct ice_pf *pf)
 		ice_set_feature_support(pf, ICE_F_GCS);
 		ice_set_feature_support(pf, ICE_F_TXTIME);
 	}
+
+	if (pf->hw.erot_present)
+		ice_set_feature_support(pf, ICE_F_EROT);
 }
 
 /**
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)
+			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)
+
 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,
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index d9a5c1a..1375106 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -311,6 +311,9 @@ struct ice_hw_common_caps {
 	/* Post update reset restriction */
 	bool reset_restrict_support;
 	bool tx_sched_topo_comp_mode_en;
+	/* eRoT (external Root of Trust) controller present */
+	bool external_pqc_rot_present;
+	bool external_pqc_rot_present_cap_advertised;
 };
 
 /* IEEE 1588 TIME_SYNC specific info */
@@ -1040,6 +1043,8 @@ struct ice_hw {
 	u8 dvm_ena;
 	u16 io_expander_handle;
 	u8 cgu_part_number;
+	/* true when eRoT (external Root of Trust) controller is present */
+	bool erot_present;
 };
 
 /* Statistics collected by each port, VSI, VEB, and S-channel */
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] 5+ messages in thread

* From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
  2026-09-17  9:43 [PATCH next-queue v1 0/2] ice: eRoT adapter NVM update guard Aleksandr Loktionov
  2026-09-17  9:43 ` From: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Aleksandr Loktionov
@ 2026-09-17  9:43 ` Aleksandr Loktionov
  2026-09-18 13:21   ` Tomasz Lichwala
  1 sibling, 1 reply; 5+ messages in thread
From: Aleksandr Loktionov @ 2026-09-17  9:43 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov
  Cc: netdev, e02905820face5404ac5eb8193ba94f9823c4606, Mon, Sep, 17,
	00

On eRoT adapters, firmware rejects partial NVM updates mid-session.
Detect this proactively before any flash writes by tracking which PLDM
components have been presented in ice_send_component_table().

All four authentication components must be present for an eRoT update:
NVM Bank, OROM, NetList, and Manifest.  If any are absent when
ice_flash_component() is called, reject with -EPERM and a clear
netlink error message before erasing any flash bank.

NVM_COMP_ID_MANIFEST only exists on eRoT adapters; the Shadow RAM
pointer backing it (module 0x4E) is unallocated on other hardware.
Accept it in ice_send_component_table() only when ICE_F_EROT is
supported, so a non-eRoT adapter still rejects it as an unknown
component before any flash write, same as before this patch. Its
activate-select flag is 0, since the ACTIV_SEL bitmask only defines
bits for NVM, OROM, and NetList.

Duplicate component IDs within a single PLDM image are also rejected
in ice_send_component_table() before the component table is sent to
firmware.

Unlike its NVM/OROM/NetList siblings, the manifest bank has no
ICE_SR_PQC_MANIFEST_BANK_SIZE counterpart: nothing reads the manifest
area size back via ice_read_sr_area_size(), so only the bank pointer
is defined.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  1 +
 .../net/ethernet/intel/ice/ice_fw_update.c    | 80 +++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_type.h     |  1 +
 3 files changed, 82 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 42878ab..b240f2c 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1747,6 +1747,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..fe68efa 100644
--- a/drivers/net/ethernet/intel/ice/ice_fw_update.c
+++ b/drivers/net/ethernet/intel/ice/ice_fw_update.c
@@ -9,6 +9,18 @@
 #include "ice_lib.h"
 #include "ice_fw_update.h"
 
+/* Bitmask values for ice_fwu_priv::seen_components */
+#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)
+/* Components required for a complete eRoT-authenticated update.
+ * If firmware adds further authentication components in future
+ * hardware, this mask must be extended to match.
+ */
+#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 +41,9 @@ struct ice_fwu_priv {
 
 	/* Track if EMP reset is available */
 	u8 emp_reset_available;
+
+	/* Bitmask of auth components seen in this image (ICE_FWU_COMP_*) */
+	u8 seen_components;
 };
 
 /**
@@ -108,6 +123,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 +238,28 @@ 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 (!ice_is_feature_supported(pf, ICE_F_EROT)) {
+			dev_err(dev, "Unable to update due to a firmware component with unknown ID %u\n",
+				component->identifier);
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Unable to update due to unknown firmware component");
+			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 +268,14 @@ ice_send_component_table(struct pldmfw *context, struct pldmfw_component *compon
 		return -EOPNOTSUPP;
 	}
 
+	if (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)
@@ -623,6 +665,24 @@ ice_switch_flash_banks(struct ice_pf *pf, u8 activate_flags,
 	return 0;
 }
 
+/**
+ * ice_has_erot_incomplete - check whether an eRoT update is missing a component
+ * @priv: PLDM firmware update private data
+ *
+ * On eRoT adapters all four authentication components must be present.
+ *
+ * Return: true when a required component is absent on an eRoT adapter (the
+ * update must be rejected), false otherwise (including non-eRoT adapters).
+ */
+static bool
+ice_has_erot_incomplete(struct ice_fwu_priv *priv)
+{
+	if (!ice_is_feature_supported(priv->pf, ICE_F_EROT))
+		return false;
+
+	return priv->seen_components != ICE_FWU_COMP_ALL_EROT;
+}
+
 /**
  * ice_flash_component - Flash a component of the NVM
  * @context: PLDM fw update structure
@@ -667,6 +727,16 @@ 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;
+		/* Manifest has no ACTIV_SEL bit of its own; ACTIV_SEL only
+		 * defines activation bits for NVM, OROM, and NetList, so
+		 * there is nothing to add to priv->activate_flags here.
+		 */
+		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.
@@ -676,6 +746,16 @@ ice_flash_component(struct pldmfw *context, struct pldmfw_component *component)
 		return -EINVAL;
 	}
 
+	/* ice_send_component_table() is called for every component in the
+	 * PLDM image before pldmfw_flash_image() ever calls this function,
+	 * so priv->seen_components is already fully populated by the time
+	 * the first component reaches ice_flash_component().
+	 */
+	if (ice_has_erot_incomplete(priv)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "eRoT adapter requires all four components (NVM, OROM, NetList, Manifest) in a single update");
+		return -EINVAL;
+	}
 	/* Mark this component for activating at the end */
 	priv->activate_flags |= flag;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 1375106..847f17e6 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -1154,6 +1154,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] 5+ messages in thread

* Re: From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
  2026-09-17  9:43 ` From: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Aleksandr Loktionov
@ 2026-09-18 13:11   ` Tomasz Lichwala
  0 siblings, 0 replies; 5+ messages in thread
From: Tomasz Lichwala @ 2026-09-18 13:11 UTC (permalink / raw)
  To: Aleksandr Loktionov, intel-wired-lan, anthony.l.nguyen
  Cc: netdev, Przemek Kitszel



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,



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

* Re: From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
  2026-09-17  9:43 ` Aleksandr Loktionov
@ 2026-09-18 13:21   ` Tomasz Lichwala
  0 siblings, 0 replies; 5+ messages in thread
From: Tomasz Lichwala @ 2026-09-18 13:21 UTC (permalink / raw)
  To: Aleksandr Loktionov, intel-wired-lan, anthony.l.nguyen
  Cc: netdev, Przemek Kitszel



On 17.09.2026 11:43, Aleksandr Loktionov wrote:

> @@ -220,12 +238,28 @@ 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;

Nit: initialize to u8 comp_bit = 0; - harmless as-is since the default: path returns early, but it silences static-analysis warnings about a variable set only inside a switch.

>  	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 (!ice_is_feature_supported(pf, ICE_F_EROT)) {
> +			dev_err(dev, "Unable to update due to a firmware component with unknown ID %u\n",
> +				component->identifier);
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Unable to update due to unknown firmware component");
> +			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",



> @@ -676,6 +746,16 @@ ice_flash_component(struct pldmfw *context, struct pldmfw_component *component)
>  		return -EINVAL;
>  	}
>  
> +	/* ice_send_component_table() is called for every component in the
> +	 * PLDM image before pldmfw_flash_image() ever calls this function,
> +	 * so priv->seen_components is already fully populated by the time
> +	 * the first component reaches ice_flash_component().
> +	 */
> +	if (ice_has_erot_incomplete(priv)) {

This check runs on every call to ice_flash_component(), but pldmfw_ops guarantees all send_component_table() calls finish before any flash_component() call. Since transfer_flag on the last call to ice_send_component_table() carries PLDM_TRANSFER_FLAG_END, the completeness check could run exactly once there instead of being repeated for every component in ice_flash_component().

> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "eRoT adapter requires all four components (NVM, OROM, NetList, Manifest) in a single update");
> +		return -EINVAL;

Commit message says "reject with -EPERM", but the code returns -EINVAL. Please align the commit message with the actual errno, or fix the errno if -EPERM was intended.

> +	}
>  	/* Mark this component for activating at the end */
>  	priv->activate_flags |= flag;
>  




Both patches are missing a proper Subject: line — the commit message starts directly with the body text, so git log --oneline shows the From: line instead of a subject, and there is no [PATCH net-next vN x/2] ice: ... tag prefix as listed in the cover letter's shortlog. Please regenerate/resend with git format-patch so each patch carries its own subject line and revision/series tag, matching the titles already given in the cover letter shortlog.


Thanks,
Tomasz


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

end of thread, other threads:[~2026-09-18 13:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-17  9:43 [PATCH next-queue v1 0/2] ice: eRoT adapter NVM update guard Aleksandr Loktionov
2026-09-17  9:43 ` From: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Aleksandr Loktionov
2026-09-18 13:11   ` Tomasz Lichwala
2026-09-17  9:43 ` Aleksandr Loktionov
2026-09-18 13:21   ` Tomasz Lichwala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox