-----Original Message-----
From: Jiri Pirko <jiri@resnulli.us>
Sent: Wednesday, July 20, 2022 10:56 PM
To: Keller, Jacob E <jacob.e.keller@intel.com>
Cc: netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>
Subject: Re: [net-next PATCH 2/2] ice: support dry run of a flash update to
validate firmware file

Wed, Jul 20, 2022 at 08:34:33PM CEST, jacob.e.keller@intel.com wrote:

quoted

Now that devlink core flash update can handle dry run requests, update
the ice driver to allow validating a PLDM file in dry_run mode.

First, add a new dry_run field to the pldmfw context structure. This
indicates that the PLDM firmware file library should only validate the
file and verify that it has a matching record. Update the pldmfw
documentation to indicate this "dry run" mode.

In the ice driver, let the stack know that we support the dry run
attribute for flash update by setting the appropriate bit in the
.supported_flash_update_params field.

If the dry run is requested, notify the PLDM firmware library by setting
the context bit appropriately. Don't cancel a pending update during
a dry run.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Documentation/driver-api/pldmfw/index.rst      | 10 ++++++++++
drivers/net/ethernet/intel/ice/ice_devlink.c   |  3 ++-
drivers/net/ethernet/intel/ice/ice_fw_update.c | 14 ++++++++++----
include/linux/pldmfw.h                         |  5 +++++
lib/pldmfw/pldmfw.c                            | 12 ++++++++++++
5 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/Documentation/driver-api/pldmfw/index.rst

b/Documentation/driver-api/pldmfw/index.rst

quoted

index ad2c33ece30f..454b3ed6576a 100644

--- a/Documentation/driver-api/pldmfw/index.rst
+++ b/Documentation/driver-api/pldmfw/index.rst

@@ -51,6 +51,16 @@ unaligned access of multi-byte fields, and to properly

convert from Little

quoted

Endian to CPU host format. Additionally the records, descriptors, and
components are stored in linked lists.

+Validating a PLDM firmware file
+===============================
+
+To simply validate a PLDM firmware file, and verify whether it applies to
+the device, set the ``dry_run`` flag in the ``pldmfw`` context structure.
+If this flag is set, the library will parse the file, validating its UUID
+and checking if any record matches the device. Note that in a dry run, the
+library will *not* issue any ops besides ``match_record``. It will not
+attempt to send the component table or package data to the device firmware.
+
Performing a flash update
=========================

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c

b/drivers/net/ethernet/intel/ice/ice_devlink.c

quoted

index 3337314a7b35..18214ea33e2d 100644

--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c

@@ -467,7 +467,8 @@ ice_devlink_reload_empr_finish(struct devlink *devlink,

}

static const struct devlink_ops ice_devlink_ops = {
-	.supported_flash_update_params =

DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,

quoted

+	.supported_flash_update_params =

DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK |

quoted

+

DEVLINK_SUPPORT_FLASH_UPDATE_DRY_RUN,

quoted

	.reload_actions = BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
	/* The ice driver currently does not support driver reinit */
	.reload_down = ice_devlink_reload_empr_start,

diff --git a/drivers/net/ethernet/intel/ice/ice_fw_update.c

b/drivers/net/ethernet/intel/ice/ice_fw_update.c

quoted

index 3dc5662d62a6..63317ae88186 100644

--- a/drivers/net/ethernet/intel/ice/ice_fw_update.c
+++ b/drivers/net/ethernet/intel/ice/ice_fw_update.c

@@ -1015,15 +1015,21 @@ int ice_devlink_flash_update(struct devlink

*devlink,

quoted

	else
		priv.context.ops = &ice_fwu_ops_e810;
	priv.context.dev = dev;
+	priv.context.dry_run = params->dry_run;
	priv.extack = extack;
	priv.pf = pf;
	priv.activate_flags = preservation;

-	devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL,

0, 0);

quoted

+	if (params->dry_run)
+		devlink_flash_update_status_notify(devlink, "Validating flash

binary", NULL, 0, 0);

You do validation of the binary instead of the actual flash. Why is it
called "dry-run" then? Perhaps "validate" would be more suitable?

I had it as dry-run to match the naming of the devlink, but validate might make more sense for what we are able to do with PLDM here.

I don't believe we have  a method to actually load it and have firmware perform any further validation without actually updating.