Skip to content

Implement SNP live firmware update support

Pratik R. Sampat linux-crypto lkml 5 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v5 Latest7 patches 2 repliesIn review
  2. v47 patches 6 repliesSuperseded
  3. v37 patches 11 repliesSuperseded
Earlier versions · 2
VersionPostedPatchesRepliesStatus
v27 patches2 repliesSuperseded
v17 patches4 repliesSuperseded

v4 from the cover letter

* Collected reviewed-by tags
* Keep the original firmware error when rollback fails - Tycho, Shantanu

Patches in v5 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/7firmware_loader: Stop pinning modules on registrationdrivers/base/firmware_loader/sysfs_upload.c+14 -170 replies
2/7firmware_loader: Stop pinning parent device per workqueue invocationdrivers/base/firmware_loader/sysfs_upload.c+2 -70 replies
3/7treewide: firmware_loader: Drop the unused @module argumentDocumentation/driver-api/firmware/fw_upload.rst drivers/base/firmware_loader/sysfs_upload.c drivers/base/firmware_loader/sysfs_upload.h +9 more+24 -290 replies
4/7crypto: ccp - Factor out the release of the SEV firmware buffersdrivers/crypto/ccp/sev-dev.c+30 -250 replies
5/7crypto: ccp - Allow SNP platform data to be queried after SNP INITdrivers/crypto/ccp/sev-dev.c+10 -130 replies
6/7crypto: ccp - Register with fw_uploader and always faildrivers/crypto/ccp/Kconfig drivers/crypto/ccp/sev-dev.c drivers/crypto/ccp/sev-dev.h+76 -00 replies
7/7crypto: ccp - Implement SNP Download Firmware EXdrivers/crypto/ccp/sev-dev.c drivers/crypto/ccp/sev-dev.h include/linux/psp-sev.h+317 -12 replies

Reviews trailers per patch

4 of 7 patches have a Reviewed-by or Acked-by.

PatchChao GaoConor DooleyTom LendackyShantanu Sinha
1/7T-b
2/7
3/7A-b
4/7R-b
5/7R-b
6/7R-b
7/7T-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 4 reviewers.

Read the v5 thread

Cover letter [Patch v5 0/7] Implement SNP live firmware update support

Introducing support for live update on AMD SEV-SNP platforms via DOWNLOAD_FIRMWARE_EX.

This patchset is an extension of the RFC patchset from Tycho Andersen.

This series also fronts firmware_loader patches from Dan Williams[1] which majorly cleans up refcount issues so that registering the upload interface no longer pins the module. Without which having live firmware update will cause failure to reload the ccp module as well as break kexec.

Patches based on cryptodev-2.6

v4:
* Collected reviewed-by tags
* Keep the original firmware error when rollback fails - Tycho, Shantanu

Responding to Sashiko comments:
* Does __snp_alloc_firmware_pages leak the allocated page on error?
- This isn't a transient firmware error. rmp_mark_pages_firmware() issues RMPUPDATE, and a failure there indicates an inconsistent RMP. Any pages already converted are reclaimed or placed on the SNP leaked pages list, so not freeing is the conservative choice and used by all the other callers.

* Can reading *feat_info here trigger a host crash?:
- No. Reclaim is only needed before the page is written by the host or returned to the allocator, which __snp_free_firmware_pages() takes care of after the read. tio_status page reading does the same

v4: https://lore.kernel.org/linux-crypto/[email protected]/
* Collected reviewed-by tags
* Cleanup snp_alloc_firmware_pages() error handing - Tom
* Cleanups to comments and code formatting - Tom
* Change dev prints to ratelimited - Tom
* Add comment about releasing sev_release_firmware_buffers() - Tom
* Add FW_UPLOAD and FW_LOADER to CRYPTO_DEV_SP_PSP config and clean up #ifdef-erry - Shantanu
* On failure of sev_get_api_version() return an error only if PSP failed in all other cases DLFW_EX succeeded - Shantanu
* Free TMR if we were already in UNINIT too - Local Sashiko instance

Responding to Sashiko review:
* Possible RMP violation when live fireware update races with snp_get_platform_data()
- Both the dlfw_ex and snp_get_platform_data() are called within a mutex guard so feat_info can only be accessed once it is no longer in the firmware owned state and the mutex is dropped.
* Register-unregister failure
- The ccp PCI driver probes synchronously, so psp_master is set before sev_pci_init() runs. The uploader follows the same lifecycle as the rest of the SEV/SNP initialization, including the TSM and verify-mitigation interfaces. The unregister in sev_dev_destroy() is needed so the upload device doesn't outlive sev.

v3: https://lore.kernel.org/linux-crypto/[email protected]/
* Print psp return codes along with driver returns for easier parsing - Shantanu
* Clean up error codes to map the right firmware/driver error condition to the firmware error codes. - Shantanu
* Report -ETIMEOUT case by checking for psp_dead after DLFW_EX command is called instead of reporting unknown error - Shantanu
* __sev_do_snp_platform_status() call can race and incorrect cached platform states may be supplied. Guard the call under a mutex - Sashiko
* firmware unregister can race when called from sev_dev_destroy() or sev_pci_exit(). Guard the call under a mutex - Sashiko

v2: https://lore.kernel.org/lkml/[email protected]/
* In snp_get_platform_data() use snp_[alloc|free]_firmware_page() to transition pages to and from firmware-owned state, instead of open-coding rmp_mark_pages_firmware()/snp_reclaim_pages() around the SNP_FEATURE_INFO command - Tom, Sashiko
* Claim sev->fwl atomically with xchg() in unregister_sev_fw_uploader() so a concurrent module removal and sysfs unbind cannot both reach firmware_upload_unregister() - Sashiko

Responding to other Sashiko comments:
* __sev_release_firmware_buffers() called with panic=true can cause deadlock as another stopped CPU might currently hold zone->lock or other page allocator locks.
- I think having a panic param within __snp_free_firmware_pages which checks and returns before __free_pages() should resolve this issue. Since this is a pre-existing issue not relevant to this patchset, plan to send this as a seperate patch
* The sev_download_firmware_ex() function is called with the lock held and performs a GFP_KERNEL allocation. Since GFP_KERNEL can trigger direct memory reclaim, could this invoke SEV page reclaim operations that attempt to acquire the already-held sev_cmd_mutex, resulting in a deadlocked state?
- GFP_KERNEL may perform direct MM reclaim, but SNP page reclamation is not registered as an MM shrinker or normal page-reclaim callback. snp_reclaim_pages() acquires sev_cmd_mutex only when explicitly called with locked=false
* "If snp_reclaim_cmd_buf() fails and returns early, the code skips resetting sev->cmd_buf_active and sev->cmd_buf_backup_active. Will this permanently leak the bounce buffer slots and result in future -EBUSY failures?"
- Since the command buffer may still be firmware owned, I think declaring psp_dead = true is the way to ensure we do not make it available for reuse after a failed reclaim. Pre-existing issue, plan to send this as a seperate patch

v1: https://lore.kernel.org/linux-crypto/[email protected]/
* firmware-loader patches are rebased as-is and only incorporates fixes for minor build issues
* Dropped crypto/ccp: Hoist kernel part of SNP_PLATFORM_STATUS as that patch has been merged since
* Dropped crypto/ccp: Reclaim command buffer when the PSP dies and subsequent handling since firmware quirk is now resolved and need not be handled in the OS
* Use guard(mutex) for the locked region, which the split makes possible since sev_get_api_version() acquires sev_cmd_mutex - Maxwell
* Added a patch factoring out the TMR and INIT_EX teardown, so the update path and __sev_firmware_shutdown() share it - Shantanu
* Skip the re-init when the PSP is dead, not just when a rollback is outstanding - Shantanu
* Re-initialize the platform before refreshing the cached status - Shantanu
* Split the monolithic .write into a locked update helper, an error translation helper, and a thin .write that refreshes the cached status once the lock is dropped and clean up various allocations
* Convert rollback required and reinit required globals to reside in sev_dev struct to avoid carrying over states if manually reloaded

RFC: https://lore.kernel.org/all/[email protected]/

[1]: https://lore.kernel.org/lkml/[email protected]/

Dan Williams (3):
firmware_loader: Stop pinning modules on registration
firmware_loader: Stop pinning parent device per workqueue invocation
treewide: firmware_loader: Drop the unused @module argument

Pratik R. Sampat (4):
crypto: ccp - Factor out the release of the SEV firmware buffers
crypto: ccp - Allow SNP platform data to be queried after SNP INIT
crypto: ccp - Register with fw_uploader and always fail
crypto: ccp - Implement SNP Download Firmware EX

 .../driver-api/firmware/fw_upload.rst         |   2 +-
 drivers/base/firmware_loader/sysfs_upload.c   |  50 +-
 drivers/base/firmware_loader/sysfs_upload.h   |   1 -
 drivers/crypto/ccp/Kconfig                    |   2 +
 drivers/crypto/ccp/sev-dev.c                  | 445 ++++++++++++++++--
 drivers/crypto/ccp/sev-dev.h                  |   4 +
 drivers/cxl/core/memdev.c                     |   4 +-
 drivers/firmware/microchip/mpfs-auto-update.c |   2 +-
 drivers/fpga/intel-m10-bmc-sec-update.c       |   4 +-
 drivers/greybus/gb-beagleplay.c               |   2 +-
 drivers/media/i2c/thp7312.c                   |   2 +-
 drivers/net/pse-pd/pd692x0.c                  |   4 +-
 drivers/virt/coco/tdx-host/tdx-host.c         |   4 +-
 include/linux/firmware.h                      |  15 +-
 include/linux/psp-sev.h                       |  19 +
 lib/test_firmware.c                           |   3 +-
 16 files changed, 472 insertions(+), 91 deletions(-)
Signature · 1 line
2.43.0