From: Jiri Pirko <jiri@resnulli•us>
To: Jacob Keller <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
Date: Thu, 21 Jul 2022 07:56:04 +0200 [thread overview]
Message-ID: <YtjqdJcGpulWsBHs@nanopsycho> (raw)
In-Reply-To: <20220720183433.2070122-3-jacob.e.keller@intel.com>
Wed, Jul 20, 2022 at 08:34:33PM CEST, jacob.e.keller@intel•com wrote:
>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
>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
> 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
>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,
>+ .supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK |
>+ DEVLINK_SUPPORT_FLASH_UPDATE_DRY_RUN,
> .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
>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,
> 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);
>+ 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?
>+ else
>+ devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL, 0, 0);
>
>- err = ice_cancel_pending_update(pf, NULL, extack);
>- if (err)
>- return err;
>+ if (!params->dry_run) {
>+ err = ice_cancel_pending_update(pf, NULL, extack);
>+ if (err)
>+ return err;
>+ }
>
> err = ice_acquire_nvm(hw, ICE_RES_WRITE);
> if (err) {
>diff --git a/include/linux/pldmfw.h b/include/linux/pldmfw.h
>index 0fc831338226..d9add301582b 100644
>--- a/include/linux/pldmfw.h
>+++ b/include/linux/pldmfw.h
>@@ -124,10 +124,15 @@ struct pldmfw_ops;
> * should embed this in a private structure and use container_of to obtain
> * a pointer to their own data, used to implement the device specific
> * operations.
>+ *
>+ * @ops: function pointers used as callbacks from the PLDMFW library
>+ * @dev: pointer to the device being updated
>+ * @dry_run: if true, only validate the file, do not perform an update.
> */
> struct pldmfw {
> const struct pldmfw_ops *ops;
> struct device *dev;
>+ bool dry_run;
> };
>
> bool pldmfw_op_pci_match_record(struct pldmfw *context, struct pldmfw_record *record);
>diff --git a/lib/pldmfw/pldmfw.c b/lib/pldmfw/pldmfw.c
>index 6e77eb6d8e72..29a132a39876 100644
>--- a/lib/pldmfw/pldmfw.c
>+++ b/lib/pldmfw/pldmfw.c
>@@ -827,6 +827,10 @@ static int pldm_finalize_update(struct pldmfw_priv *data)
> * to the device firmware. Extract and write the flash data for each of the
> * components indicated in the firmware file.
> *
>+ * If the context->dry_run is set, this is a request for a dry run, i.e. to
>+ * only validate the PLDM firmware file. In this case, stop and exit after we
>+ * find a valid matching record.
>+ *
> * Returns: zero on success, or a negative error code on failure.
> */
> int pldmfw_flash_image(struct pldmfw *context, const struct firmware *fw)
>@@ -844,14 +848,22 @@ int pldmfw_flash_image(struct pldmfw *context, const struct firmware *fw)
> data->fw = fw;
> data->context = context;
>
>+ /* Parse the image and make sure it is a valid PLDM firmware binary */
> err = pldm_parse_image(data);
> if (err)
> goto out_release_data;
>
>+ /* Search for a record matching the device */
> err = pldm_find_matching_record(data);
> if (err)
> goto out_release_data;
>
>+ /* If this is a dry run, do not perform an update */
>+ if (context->dry_run)
>+ goto out_release_data;
>+
>+ /* Perform the device update */
>+
> err = pldm_send_package_data(data);
> if (err)
> goto out_release_data;
>--
>2.35.1.456.ga9c7032d4631
>
next prev parent reply other threads:[~2022-07-21 5:56 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-20 18:34 [net-next PATCH 0/2] devlink: implement dry run support for flash update Jacob Keller
2022-07-20 18:34 ` [net-next PATCH 1/2] devlink: add dry run attribute to " Jacob Keller
2022-07-21 5:54 ` Jiri Pirko
2022-07-21 20:32 ` Keller, Jacob E
2022-07-22 6:18 ` Jiri Pirko
2022-07-22 21:12 ` Keller, Jacob E
2022-07-23 15:42 ` Jiri Pirko
2022-07-25 19:15 ` Keller, Jacob E
2022-07-25 19:39 ` Jakub Kicinski
2022-07-25 20:27 ` Keller, Jacob E
2022-07-25 20:32 ` Jakub Kicinski
2022-07-25 20:46 ` Keller, Jacob E
2022-07-26 1:13 ` Jakub Kicinski
2022-07-26 17:23 ` Keller, Jacob E
2022-07-26 18:48 ` Jakub Kicinski
2022-07-26 18:49 ` Keller, Jacob E
2022-07-26 18:21 ` Keller, Jacob E
2022-08-05 16:32 ` Keller, Jacob E
2022-08-05 18:51 ` Jakub Kicinski
2022-08-05 19:50 ` Keller, Jacob E
2022-07-25 20:33 ` Keller, Jacob E
2022-07-21 16:47 ` Jakub Kicinski
2022-07-21 18:57 ` Keller, Jacob E
2022-07-21 16:48 ` Jakub Kicinski
2022-07-21 18:57 ` Keller, Jacob E
2022-07-20 18:34 ` [net-next PATCH 2/2] ice: support dry run of a flash update to validate firmware file Jacob Keller
2022-07-21 5:56 ` Jiri Pirko [this message]
2022-07-21 18:57 ` Keller, Jacob E
2022-07-21 7:53 ` Leon Romanovsky
2022-07-21 5:57 ` [net-next PATCH 0/2] devlink: implement dry run support for flash update Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YtjqdJcGpulWsBHs@nanopsycho \
--to=jiri@resnulli$(echo .)us \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox