[Intel-wired-lan] [PATCH net v1] ice: Fix inventory failed error during flash update
Tony Nguyen
anthony.l.nguyen at intel.com
Wed Aug 17 22:58:44 UTC 2022
On 8/11/2022 4:45 AM, Mateusz Palczewski wrote:
> From: Sylwester Dziedziuch <sylwesterx.dziedziuch at intel.com>
>
> After updating flash image on E810 card with NVM update tool
> there was an error: The inventory process failed.
>
> This was reported at bugzilla thread #2114483 and caused by the tool
> trying to read devlink parameters fw.mgmt.minsrev and fw.undi.minsrev
> but those parameters were not registered by the driver.
>
> The ice NVM flash has a security revision field for the main NVM bank
> and the Option ROM bank. In addition to the revision within the module,
> the device also has a minimum security revision TLV area. This minimum
> security revision field indicates the minimum value that will be
> accepted for the associated security revision when loading the NVM bank.
>
> These parameters are permanent (i.e. stored in flash), and are used to
> indicate the minimum security revision of the associated NVM bank. If
> the image in the bank has a lower security revision, then the flash
> loader will not continue loading that flash bank.
>
> Fix this by adding two new devlink parameters fw.mgmt.minsrev
> and fw.undi.minsrev and function to read they respective values.
>
> This idea was proposed before with both write and read funcionality
> but was rejected by community. This patch focuses on read only.
Adding Jake for his input since he sent the initial series.
> Fixes: 1adf7ead8204 ("ice: enable initial devlink support")
> Signed-off-by: Sylwester Dziedziuch <sylwesterx.dziedziuch at intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski at intel.com>
> Link: https://lore.kernel.org/netdev/20210129004332.3004826-5-anthony.l.nguyen@intel.com/
> ---
> Documentation/networking/devlink/ice.rst | 33 +++++++
> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 17 ++++
> drivers/net/ethernet/intel/ice/ice_devlink.c | 90 +++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_nvm.c | 53 +++++++++++
> drivers/net/ethernet/intel/ice/ice_nvm.h | 1 +
> drivers/net/ethernet/intel/ice/ice_type.h | 8 ++
> 6 files changed, 202 insertions(+)
>
> diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst
> index 8c082b139bbd..aabd33a7f7da 100644
> --- a/Documentation/networking/devlink/ice.rst
> +++ b/Documentation/networking/devlink/ice.rst
> @@ -90,6 +90,39 @@ The ``ice`` driver reports the following versions
> - 0xee16ced7
> - The first 4 bytes of the hash of the netlist module contents.
>
> +Parameters
> +==========
> +
> +The minimum security revision fields of the ice device control whether the
> +associated flash section can be loaded. If the security revision field of
> +the section -- ``fw.mgmt.srev`` for the main firmware section and
> +``fw.undi.srev`` for the Option ROM -- is lower than the associated minimum
> +security revision, then the device will not load that section of firmware.
> +
> +The ``ice`` driver implements driver-specific parameters for reading the
> +minimum security revision fields associated those two sections of the device
> +flash. Note that the device will not allow lowering a minimum security
> +revision, nor will it allow increasing the security revision higher than the
> +associated security revision of the active flash image.
> +
> +.. list-table:: Minimum security revision parameters
> + :widths: 5 5 5 85
> +
> + * - Name
> + - Type
> + - Mode
> + - Description
> + * - ``fw.undi.minsrev``
> + - u32
> + - permanent
> + - The device's minimum security revision for the ``fw.undi`` section of
> + the flash.
> + * - ``fw.mgmt.minsrev``
> + - u32
> + - permanent
> + - The device's minimum security revision for the ``fw.mgmt`` section of
> + the flash.
> +
> Flash Update
> ============
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 9939238573a4..4d46f91adbdc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -1509,6 +1509,23 @@ struct ice_aqc_nvm_checksum {
> u8 rsvd2[12];
> };
>
> +#define ICE_AQC_NVM_MINSREV_MOD_ID 0x130
> +
> +/* Used for reading and writing MinSRev using 0x0701 and 0x0703. Note that the
> + * type field is excluded from the section when reading and writing from
> + * a module using the module_typeid field with these AQ commands.
> + */
> +struct ice_aqc_nvm_minsrev {
> + __le16 length;
> + __le16 validity;
> +#define ICE_AQC_NVM_MINSREV_NVM_VALID BIT(0)
> +#define ICE_AQC_NVM_MINSREV_OROM_VALID BIT(1)
> + __le16 nvm_minsrev_l;
> + __le16 nvm_minsrev_h;
> + __le16 orom_minsrev_l;
> + __le16 orom_minsrev_h;
> +};
> +
> /* Used for NVM Set Package Data command - 0x070A */
> struct ice_aqc_nvm_pkg_data {
> u8 reserved[3];
> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index 3337314a7b35..95f1653306d5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -372,6 +372,83 @@ static int ice_devlink_info_get(struct devlink *devlink,
> return err;
> }
>
> +enum ice_devlink_param_id {
> + ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
> + ICE_DEVLINK_PARAM_ID_FW_MGMT_MINSREV,
> + ICE_DEVLINK_PARAM_ID_FW_UNDI_MINSREV,
> +};
> +
> +/**
> + * ice_devlink_minsrev_get - Get the current minimum security revision
> + * @devlink: pointer to the devlink instance
> + * @id: the parameter ID to get
> + * @ctx: context to return the parameter value
> + *
> + * Returns: zero on success, or an error code on failure.
> + */
> +static int
> +ice_devlink_minsrev_get(struct devlink *devlink, u32 id,
> + struct devlink_param_gset_ctx *ctx)
> +{
> + struct ice_pf *pf = devlink_priv(devlink);
> + struct device *dev = ice_pf_to_dev(pf);
> + struct ice_minsrev_info minsrevs = {};
> +
> + if (id != ICE_DEVLINK_PARAM_ID_FW_MGMT_MINSREV &&
> + id != ICE_DEVLINK_PARAM_ID_FW_UNDI_MINSREV)
> + return -EINVAL;
> +
> + if (ice_get_nvm_minsrevs(&pf->hw, &minsrevs)) {
> + dev_warn(dev, "Failed to read minimum security revision data from flash\n");
> + return -EIO;
> + }
> +
> + /* We report zero if the device has not yet had a valid minimum
> + * security revision programmed for the associated module. This makes
> + * sense because it is not possible to have a security revision of
> + * less than zero. Thus, all images will be able to load if the
> + * minimum security revision is zero, the same as the case where the
> + * minimum value is indicated as invalid.
> + */
> + switch (id) {
> + case ICE_DEVLINK_PARAM_ID_FW_MGMT_MINSREV:
> + if (minsrevs.nvm_valid)
> + ctx->val.vu32 = minsrevs.nvm;
> + else
> + ctx->val.vu32 = 0;
> + break;
> + case ICE_DEVLINK_PARAM_ID_FW_UNDI_MINSREV:
> + if (minsrevs.orom_valid)
> + ctx->val.vu32 = minsrevs.orom;
> + else
> + ctx->val.vu32 = 0;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * ice_devlink_minsrev_set - Set the minimum security revision
> + * @devlink: pointer to the devlink instance
> + * @id: the parameter ID to set
> + * @ctx: context to return the parameter value
> + *
> + * Currently manually changing minimum security revision is not supported.
> + *
> + * Returns: EINVAL.
> + */
> +static int
> +ice_devlink_minsrev_set(struct devlink *devlink, u32 id,
> + struct devlink_param_gset_ctx *ctx)
> +{
> + struct ice_pf *pf = devlink_priv(devlink);
> +
> + dev_err(ice_pf_to_dev(pf), "Setting minimum security revision is not available\n");
> +
> + return -EINVAL;
> +}
> +
> /**
> * ice_devlink_reload_empr_start - Start EMP reset to activate new firmware
> * @devlink: pointer to the devlink instance to reload
> @@ -589,6 +666,19 @@ static const struct devlink_param ice_devlink_params[] = {
> ice_devlink_enable_iw_get,
> ice_devlink_enable_iw_set,
> ice_devlink_enable_iw_validate),
> + DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_FW_MGMT_MINSREV,
> + "fw.mgmt.minsrev",
> + DEVLINK_PARAM_TYPE_U32,
> + BIT(DEVLINK_PARAM_CMODE_PERMANENT),
> + ice_devlink_minsrev_get,
> + ice_devlink_minsrev_set, NULL),
> + DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_FW_UNDI_MINSREV,
> + "fw.undi.minsrev",
> + DEVLINK_PARAM_TYPE_U32,
> + BIT(DEVLINK_PARAM_CMODE_PERMANENT),
> + ice_devlink_minsrev_get,
> + ice_devlink_minsrev_set, NULL),
> +
>
> };
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
> index 13cdb5ea594d..1c3fa733387d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_nvm.c
> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
> @@ -1139,6 +1139,59 @@ int ice_nvm_write_activate(struct ice_hw *hw, u8 cmd_flags, u8 *response_flags)
> return err;
> }
>
> +/**
> + * ice_get_nvm_minsrevs - Get the Minimum Security Revision values from flash
> + * @hw: pointer to the HW struct
> + * @minsrevs: structure to store NVM and OROM minsrev values
> + *
> + * Read the Minimum Security Revision TLV and extract the revision values from
> + * the flash image into a readable structure for processing.
> + */
> +int ice_get_nvm_minsrevs(struct ice_hw *hw, struct ice_minsrev_info *minsrevs)
> +{
> + struct ice_aqc_nvm_minsrev data;
> + int status;
> + u16 valid;
> +
> + status = ice_acquire_nvm(hw, ICE_RES_READ);
> + if (status)
> + return status;
> +
> + status = ice_aq_read_nvm(hw, ICE_AQC_NVM_MINSREV_MOD_ID, 0,
> + sizeof(data), &data, true, false, NULL);
> +
> + ice_release_nvm(hw);
> +
> + if (status)
> + return status;
> +
> + valid = le16_to_cpu(data.validity);
> +
> + /* Extract NVM minimum security revision */
> + if (valid & ICE_AQC_NVM_MINSREV_NVM_VALID) {
> + u16 minsrev_l, minsrev_h;
> +
> + minsrev_l = le16_to_cpu(data.nvm_minsrev_l);
> + minsrev_h = le16_to_cpu(data.nvm_minsrev_h);
> +
> + minsrevs->nvm = minsrev_h << 16 | minsrev_l;
> + minsrevs->nvm_valid = true;
> + }
> +
> + /* Extract the OROM minimum security revision */
> + if (valid & ICE_AQC_NVM_MINSREV_OROM_VALID) {
> + u16 minsrev_l, minsrev_h;
> +
> + minsrev_l = le16_to_cpu(data.orom_minsrev_l);
> + minsrev_h = le16_to_cpu(data.orom_minsrev_h);
> +
> + minsrevs->orom = minsrev_h << 16 | minsrev_l;
> + minsrevs->orom_valid = true;
> + }
> +
> + return 0;
> +}
> +
> /**
> * ice_aq_nvm_update_empr
> * @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 856d1ad4398b..b44ecb8b9341 100644
> --- a/drivers/net/ethernet/intel/ice/ice_nvm.h
> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.h
> @@ -20,6 +20,7 @@ ice_read_flat_nvm(struct ice_hw *hw, u32 offset, u32 *length, u8 *data,
> int
> ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
> u16 module_type);
> +int ice_get_nvm_minsrevs(struct ice_hw *hw, struct ice_minsrev_info *minsrevs);
> int ice_get_inactive_orom_ver(struct ice_hw *hw, struct ice_orom_info *orom);
> int ice_get_inactive_nvm_ver(struct ice_hw *hw, struct ice_nvm_info *nvm);
> int
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
> index 861b64322959..c14fa57b1cb7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -428,6 +428,14 @@ struct ice_nvm_info {
> u8 minor;
> };
>
> +/* Minimum Security Revision information */
> +struct ice_minsrev_info {
> + u32 nvm;
> + u32 orom;
> + u8 nvm_valid : 1;
> + u8 orom_valid : 1;
> +};
> +
> /* netlist version information */
> struct ice_netlist_info {
> u32 major; /* major high/low */
More information about the Intel-wired-lan
mailing list