tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'devlink-move-common-flash_update-calls-to-core'

Jacob Keller says:

====================
devlink: move common flash_update calls to core

This series moves a couple common pieces done by all drivers of the
->flash_update interface into devlink.c flash update handler. Specifically,
the core code will now request_firmware and
devlink_flash_update_(begin|end)_notify.

This cleanup is intended to simplify driver implementations so that they
have less work to do and are less capable of doing the "wrong" thing.

For request_firmware, this simplification is done as it is not expected that
drivers would do anything else. It also standardizes all drivers so that
they use the same interface (request_firmware, as opposed to
request_firmware_direct), and allows reporting the netlink extended ack with
the file name attribute.

For status notification, this change prevents drivers from sending a status
message without properly sending the status end notification. The current
userspace implementation of devlink relies on this end notification to
properly close the flash update channel. Without this, the flash update
process may hang indefinitely. By moving the begin and end calls into the
core code, it is no longer possible for a driver author to get this wrong.

Changes since v3
* picked up acked-by and reviewed-by comments
* fixed the ionic driver to leave the print statement in place

For the original patch that moved request_firmware, see [1]. For the v2 see
[2]. For further discussion of the issues with devlink flash status see [3].
For v3 see [4].

[1] https://lore.kernel.org/netdev/20201113000142.3563690-1-jacob.e.keller@intel.com/
[2] https://lore.kernel.org/netdev/20201113224559.3910864-1-jacob.e.keller@intel.com/
[3] https://lore.kernel.org/netdev/6352e9d3-02af-721e-3a54-ef99a666be29@intel.com/
[4] https://lore.kernel.org/netdev/20201117200820.854115-1-jacob.e.keller@intel.com/
====================

Link: https://lore.kernel.org/r/20201118190636.1235045-1-jacob.e.keller@intel.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Jakub Kicinski ac75b096 56495a24

+68 -113
+1 -3
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
··· 30 30 return -EPERM; 31 31 } 32 32 33 - devlink_flash_update_begin_notify(dl); 34 33 devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0); 35 - rc = bnxt_flash_package_from_file(bp->dev, params->file_name, 0); 34 + rc = bnxt_flash_package_from_fw_obj(bp->dev, params->fw, 0); 36 35 if (!rc) 37 36 devlink_flash_update_status_notify(dl, "Flashing done", NULL, 0, 0); 38 37 else 39 38 devlink_flash_update_status_notify(dl, "Flashing failed", NULL, 0, 0); 40 - devlink_flash_update_end_notify(dl); 41 39 return rc; 42 40 } 43 41
+22 -11
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
··· 2419 2419 return rc; 2420 2420 } 2421 2421 2422 - int bnxt_flash_package_from_file(struct net_device *dev, const char *filename, 2423 - u32 install_type) 2422 + int bnxt_flash_package_from_fw_obj(struct net_device *dev, const struct firmware *fw, 2423 + u32 install_type) 2424 2424 { 2425 2425 struct bnxt *bp = netdev_priv(dev); 2426 2426 struct hwrm_nvm_install_update_output *resp = bp->hwrm_cmd_resp_addr; 2427 2427 struct hwrm_nvm_install_update_input install = {0}; 2428 - const struct firmware *fw; 2429 2428 u32 item_len; 2430 2429 int rc = 0; 2431 2430 u16 index; ··· 2436 2437 &index, &item_len, NULL); 2437 2438 if (rc) { 2438 2439 netdev_err(dev, "PKG update area not created in nvram\n"); 2439 - return rc; 2440 - } 2441 - 2442 - rc = request_firmware(&fw, filename, &dev->dev); 2443 - if (rc != 0) { 2444 - netdev_err(dev, "PKG error %d requesting file: %s\n", 2445 - rc, filename); 2446 2440 return rc; 2447 2441 } 2448 2442 ··· 2470 2478 dma_handle); 2471 2479 } 2472 2480 } 2473 - release_firmware(fw); 2474 2481 if (rc) 2475 2482 goto err_exit; 2476 2483 ··· 2505 2514 err_exit: 2506 2515 if (rc == -EACCES) 2507 2516 bnxt_print_admin_err(bp); 2517 + return rc; 2518 + } 2519 + 2520 + static int bnxt_flash_package_from_file(struct net_device *dev, const char *filename, 2521 + u32 install_type) 2522 + { 2523 + const struct firmware *fw; 2524 + int rc; 2525 + 2526 + rc = request_firmware(&fw, filename, &dev->dev); 2527 + if (rc != 0) { 2528 + netdev_err(dev, "PKG error %d requesting file: %s\n", 2529 + rc, filename); 2530 + return rc; 2531 + } 2532 + 2533 + rc = bnxt_flash_package_from_fw_obj(dev, fw, install_type); 2534 + 2535 + release_firmware(fw); 2536 + 2508 2537 return rc; 2509 2538 } 2510 2539
+2 -2
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
··· 94 94 u16 bnxt_get_fw_auto_link_speeds(u32); 95 95 int bnxt_hwrm_nvm_get_dev_info(struct bnxt *bp, 96 96 struct hwrm_nvm_get_dev_info_output *nvm_dev_info); 97 - int bnxt_flash_package_from_file(struct net_device *dev, const char *filename, 98 - u32 install_type); 97 + int bnxt_flash_package_from_fw_obj(struct net_device *dev, const struct firmware *fw, 98 + u32 install_type); 99 99 void bnxt_ethtool_init(struct bnxt *bp); 100 100 void bnxt_ethtool_free(struct bnxt *bp); 101 101
+1 -11
drivers/net/ethernet/huawei/hinic/hinic_devlink.c
··· 285 285 struct netlink_ext_ack *extack) 286 286 { 287 287 struct hinic_devlink_priv *priv = devlink_priv(devlink); 288 - const struct firmware *fw; 289 - int err; 290 288 291 - err = request_firmware_direct(&fw, params->file_name, 292 - &priv->hwdev->hwif->pdev->dev); 293 - if (err) 294 - return err; 295 - 296 - err = hinic_firmware_update(priv, fw, extack); 297 - release_firmware(fw); 298 - 299 - return err; 289 + return hinic_firmware_update(priv, params->fw, extack); 300 290 } 301 291 302 292 static const struct devlink_ops hinic_devlink_ops = {
+1 -16
drivers/net/ethernet/intel/ice/ice_devlink.c
··· 247 247 struct netlink_ext_ack *extack) 248 248 { 249 249 struct ice_pf *pf = devlink_priv(devlink); 250 - struct device *dev = &pf->pdev->dev; 251 250 struct ice_hw *hw = &pf->hw; 252 - const struct firmware *fw; 253 251 u8 preservation; 254 252 int err; 255 253 ··· 275 277 if (err) 276 278 return err; 277 279 278 - err = request_firmware(&fw, params->file_name, dev); 279 - if (err) { 280 - NL_SET_ERR_MSG_MOD(extack, "Unable to read file from disk"); 281 - return err; 282 - } 283 - 284 - dev_dbg(dev, "Beginning flash update with file '%s'\n", params->file_name); 285 - 286 - devlink_flash_update_begin_notify(devlink); 287 280 devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL, 0, 0); 288 - err = ice_flash_pldm_image(pf, fw, preservation, extack); 289 - devlink_flash_update_end_notify(devlink); 290 281 291 - release_firmware(fw); 292 - 293 - return err; 282 + return ice_flash_pldm_image(pf, params->fw, preservation, extack); 294 283 } 295 284 296 285 static const struct devlink_ops ice_devlink_ops = {
+1 -10
drivers/net/ethernet/mellanox/mlx5/core/devlink.c
··· 13 13 struct netlink_ext_ack *extack) 14 14 { 15 15 struct mlx5_core_dev *dev = devlink_priv(devlink); 16 - const struct firmware *fw; 17 - int err; 18 16 19 - err = request_firmware_direct(&fw, params->file_name, &dev->pdev->dev); 20 - if (err) 21 - return err; 22 - 23 - err = mlx5_firmware_flash(dev, fw, extack); 24 - release_firmware(fw); 25 - 26 - return err; 17 + return mlx5_firmware_flash(dev, params->fw, extack); 27 18 } 28 19 29 20 static u8 mlx5_fw_ver_major(u32 version)
-3
drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
··· 368 368 } 369 369 370 370 mlxfw_info(mlxfw_dev, "Initialize firmware flash process\n"); 371 - devlink_flash_update_begin_notify(mlxfw_dev->devlink); 372 371 mlxfw_status_notify(mlxfw_dev, "Initializing firmware flash process", 373 372 NULL, 0, 0); 374 373 err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle); ··· 416 417 mlxfw_info(mlxfw_dev, "Firmware flash done\n"); 417 418 mlxfw_status_notify(mlxfw_dev, "Firmware flash done", NULL, 0, 0); 418 419 mlxfw_mfa2_file_fini(mfa2_file); 419 - devlink_flash_update_end_notify(mlxfw_dev->devlink); 420 420 return 0; 421 421 422 422 err_state_wait_activate_to_locked: ··· 427 429 mlxfw_dev->ops->fsm_release(mlxfw_dev, fwhandle); 428 430 err_fsm_lock: 429 431 mlxfw_mfa2_file_fini(mfa2_file); 430 - devlink_flash_update_end_notify(mlxfw_dev->devlink); 431 432 return err; 432 433 } 433 434 EXPORT_SYMBOL(mlxfw_firmware_flash);
+1 -10
drivers/net/ethernet/mellanox/mlxsw/core.c
··· 1117 1117 struct devlink_flash_update_params *params, 1118 1118 struct netlink_ext_ack *extack) 1119 1119 { 1120 - const struct firmware *firmware; 1121 - int err; 1122 - 1123 - err = request_firmware_direct(&firmware, params->file_name, mlxsw_core->bus_info->dev); 1124 - if (err) 1125 - return err; 1126 - err = mlxsw_core_fw_flash(mlxsw_core, firmware, extack); 1127 - release_firmware(firmware); 1128 - 1129 - return err; 1120 + return mlxsw_core_fw_flash(mlxsw_core, params->fw, extack); 1130 1121 } 1131 1122 1132 1123 static int mlxsw_core_devlink_param_fw_load_policy_validate(struct devlink *devlink, u32 id,
+1 -1
drivers/net/ethernet/netronome/nfp/nfp_devlink.c
··· 333 333 struct devlink_flash_update_params *params, 334 334 struct netlink_ext_ack *extack) 335 335 { 336 - return nfp_flash_update_common(devlink_priv(devlink), params->file_name, extack); 336 + return nfp_flash_update_common(devlink_priv(devlink), params->fw, extack); 337 337 } 338 338 339 339 const struct devlink_ops nfp_devlink_ops = {
+2 -15
drivers/net/ethernet/netronome/nfp/nfp_main.c
··· 301 301 return nfp_pcie_sriov_enable(pdev, num_vfs); 302 302 } 303 303 304 - int nfp_flash_update_common(struct nfp_pf *pf, const char *path, 304 + int nfp_flash_update_common(struct nfp_pf *pf, const struct firmware *fw, 305 305 struct netlink_ext_ack *extack) 306 306 { 307 307 struct device *dev = &pf->pdev->dev; 308 - const struct firmware *fw; 309 308 struct nfp_nsp *nsp; 310 309 int err; 311 310 ··· 318 319 return err; 319 320 } 320 321 321 - err = request_firmware_direct(&fw, path, dev); 322 - if (err) { 323 - NL_SET_ERR_MSG_MOD(extack, 324 - "unable to read flash file from disk"); 325 - goto exit_close_nsp; 326 - } 327 - 328 - dev_info(dev, "Please be patient while writing flash image: %s\n", 329 - path); 330 - 331 322 err = nfp_nsp_write_flash(nsp, fw); 332 323 if (err < 0) 333 - goto exit_release_fw; 324 + goto exit_close_nsp; 334 325 dev_info(dev, "Finished writing flash image\n"); 335 326 err = 0; 336 327 337 - exit_release_fw: 338 - release_firmware(fw); 339 328 exit_close_nsp: 340 329 nfp_nsp_close(nsp); 341 330 return err;
+1 -1
drivers/net/ethernet/netronome/nfp/nfp_main.h
··· 166 166 unsigned int min_size, struct nfp_cpp_area **area); 167 167 int nfp_mbox_cmd(struct nfp_pf *pf, u32 cmd, void *in_data, u64 in_length, 168 168 void *out_data, u64 out_length); 169 - int nfp_flash_update_common(struct nfp_pf *pf, const char *path, 169 + int nfp_flash_update_common(struct nfp_pf *pf, const struct firmware *fw, 170 170 struct netlink_ext_ack *extack); 171 171 172 172 enum nfp_dump_diag {
+1 -1
drivers/net/ethernet/pensando/ionic/ionic_devlink.c
··· 15 15 { 16 16 struct ionic *ionic = devlink_priv(dl); 17 17 18 - return ionic_firmware_update(ionic->lif, params->file_name, extack); 18 + return ionic_firmware_update(ionic->lif, params->fw, extack); 19 19 } 20 20 21 21 static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
+1 -1
drivers/net/ethernet/pensando/ionic/ionic_devlink.h
··· 6 6 7 7 #include <net/devlink.h> 8 8 9 - int ionic_firmware_update(struct ionic_lif *lif, const char *fw_name, 9 + int ionic_firmware_update(struct ionic_lif *lif, const struct firmware *fw, 10 10 struct netlink_ext_ack *extack); 11 11 12 12 struct ionic *ionic_devlink_alloc(struct device *dev);
+2 -12
drivers/net/ethernet/pensando/ionic/ionic_fw.c
··· 91 91 return err; 92 92 } 93 93 94 - int ionic_firmware_update(struct ionic_lif *lif, const char *fw_name, 94 + int ionic_firmware_update(struct ionic_lif *lif, const struct firmware *fw, 95 95 struct netlink_ext_ack *extack) 96 96 { 97 97 struct ionic_dev *idev = &lif->ionic->idev; ··· 99 99 struct ionic *ionic = lif->ionic; 100 100 union ionic_dev_cmd_comp comp; 101 101 u32 buf_sz, copy_sz, offset; 102 - const struct firmware *fw; 103 102 struct devlink *dl; 104 103 int next_interval; 105 104 int err = 0; 106 105 u8 fw_slot; 107 106 108 - netdev_info(netdev, "Installing firmware %s\n", fw_name); 107 + netdev_info(netdev, "Installing firmware\n"); 109 108 110 109 dl = priv_to_devlink(ionic); 111 - devlink_flash_update_begin_notify(dl); 112 110 devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0); 113 - 114 - err = request_firmware(&fw, fw_name, ionic->dev); 115 - if (err) { 116 - NL_SET_ERR_MSG_MOD(extack, "Unable to find firmware file"); 117 - goto err_out; 118 - } 119 111 120 112 buf_sz = sizeof(idev->dev_cmd_regs->data); 121 113 ··· 192 200 devlink_flash_update_status_notify(dl, "Flash failed", NULL, 0, 0); 193 201 else 194 202 devlink_flash_update_status_notify(dl, "Flash done", NULL, 0, 0); 195 - release_firmware(fw); 196 - devlink_flash_update_end_notify(dl); 197 203 return err; 198 204 }
-2
drivers/net/netdevsim/dev.c
··· 766 766 return -EOPNOTSUPP; 767 767 768 768 if (nsim_dev->fw_update_status) { 769 - devlink_flash_update_begin_notify(devlink); 770 769 devlink_flash_update_status_notify(devlink, 771 770 "Preparing to flash", 772 771 params->component, 0, 0); ··· 789 790 params->component, 81); 790 791 devlink_flash_update_status_notify(devlink, "Flashing done", 791 792 params->component, 0, 0); 792 - devlink_flash_update_end_notify(devlink); 793 793 } 794 794 795 795 return 0;
+4 -5
include/net/devlink.h
··· 19 19 #include <net/flow_offload.h> 20 20 #include <uapi/linux/devlink.h> 21 21 #include <linux/xarray.h> 22 + #include <linux/firmware.h> 22 23 23 24 #define DEVLINK_RELOAD_STATS_ARRAY_SIZE \ 24 25 (__DEVLINK_RELOAD_LIMIT_MAX * __DEVLINK_RELOAD_ACTION_MAX) ··· 567 566 568 567 /** 569 568 * struct devlink_flash_update_params - Flash Update parameters 570 - * @file_name: the name of the flash firmware file to update from 569 + * @fw: pointer to the firmware data to update from 571 570 * @component: the flash component to update 572 571 * 573 - * With the exception of file_name, drivers must opt-in to parameters by 572 + * With the exception of fw, drivers must opt-in to parameters by 574 573 * setting the appropriate bit in the supported_flash_update_params field in 575 574 * their devlink_ops structure. 576 575 */ 577 576 struct devlink_flash_update_params { 578 - const char *file_name; 577 + const struct firmware *fw; 579 578 const char *component; 580 579 u32 overwrite_mask; 581 580 }; ··· 1577 1576 enum devlink_reload_limit limit, 1578 1577 u32 actions_performed); 1579 1578 1580 - void devlink_flash_update_begin_notify(struct devlink *devlink); 1581 - void devlink_flash_update_end_notify(struct devlink *devlink); 1582 1579 void devlink_flash_update_status_notify(struct devlink *devlink, 1583 1580 const char *status_msg, 1584 1581 const char *component,
+27 -9
net/core/devlink.c
··· 3372 3372 nlmsg_free(msg); 3373 3373 } 3374 3374 3375 - void devlink_flash_update_begin_notify(struct devlink *devlink) 3375 + static void devlink_flash_update_begin_notify(struct devlink *devlink) 3376 3376 { 3377 3377 struct devlink_flash_notify params = { 0 }; 3378 3378 ··· 3380 3380 DEVLINK_CMD_FLASH_UPDATE, 3381 3381 &params); 3382 3382 } 3383 - EXPORT_SYMBOL_GPL(devlink_flash_update_begin_notify); 3384 3383 3385 - void devlink_flash_update_end_notify(struct devlink *devlink) 3384 + static void devlink_flash_update_end_notify(struct devlink *devlink) 3386 3385 { 3387 3386 struct devlink_flash_notify params = { 0 }; 3388 3387 ··· 3389 3390 DEVLINK_CMD_FLASH_UPDATE_END, 3390 3391 &params); 3391 3392 } 3392 - EXPORT_SYMBOL_GPL(devlink_flash_update_end_notify); 3393 3393 3394 3394 void devlink_flash_update_status_notify(struct devlink *devlink, 3395 3395 const char *status_msg, ··· 3429 3431 static int devlink_nl_cmd_flash_update(struct sk_buff *skb, 3430 3432 struct genl_info *info) 3431 3433 { 3432 - struct nlattr *nla_component, *nla_overwrite_mask; 3434 + struct nlattr *nla_component, *nla_overwrite_mask, *nla_file_name; 3433 3435 struct devlink_flash_update_params params = {}; 3434 3436 struct devlink *devlink = info->user_ptr[0]; 3437 + const char *file_name; 3435 3438 u32 supported_params; 3439 + int ret; 3436 3440 3437 3441 if (!devlink->ops->flash_update) 3438 3442 return -EOPNOTSUPP; ··· 3443 3443 return -EINVAL; 3444 3444 3445 3445 supported_params = devlink->ops->supported_flash_update_params; 3446 - 3447 - params.file_name = nla_data(info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME]); 3448 3446 3449 3447 nla_component = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT]; 3450 3448 if (nla_component) { ··· 3467 3469 params.overwrite_mask = sections.value & sections.selector; 3468 3470 } 3469 3471 3470 - return devlink->ops->flash_update(devlink, &params, info->extack); 3472 + nla_file_name = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME]; 3473 + file_name = nla_data(nla_file_name); 3474 + ret = request_firmware(&params.fw, file_name, devlink->dev); 3475 + if (ret) { 3476 + NL_SET_ERR_MSG_ATTR(info->extack, nla_file_name, "failed to locate the requested firmware file"); 3477 + return ret; 3478 + } 3479 + 3480 + devlink_flash_update_begin_notify(devlink); 3481 + ret = devlink->ops->flash_update(devlink, &params, info->extack); 3482 + devlink_flash_update_end_notify(devlink); 3483 + 3484 + release_firmware(params.fw); 3485 + 3486 + return ret; 3471 3487 } 3472 3488 3473 3489 static const struct devlink_param devlink_param_generic[] = { ··· 10239 10227 goto out; 10240 10228 } 10241 10229 10242 - params.file_name = file_name; 10230 + ret = request_firmware(&params.fw, file_name, devlink->dev); 10231 + if (ret) 10232 + goto out; 10243 10233 10244 10234 mutex_lock(&devlink->lock); 10235 + devlink_flash_update_begin_notify(devlink); 10245 10236 ret = devlink->ops->flash_update(devlink, &params, NULL); 10237 + devlink_flash_update_end_notify(devlink); 10246 10238 mutex_unlock(&devlink->lock); 10239 + 10240 + release_firmware(params.fw); 10247 10241 10248 10242 out: 10249 10243 rtnl_lock();