* [PATCH 1/7] firmware_loader: Stop pinning modules on registration
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
@ 2026-09-10 17:01 ` Pratik R. Sampat
2026-09-10 17:01 ` [PATCH 2/7] firmware_loader: Stop pinning parent device per workqueue invocation Pratik R. Sampat
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:01 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
From: Dan Williams <djbw@kernel.org>
The module reference counting can result in callers pinning themselves in a
circular loop. The module reference counting is unnecessary.
firmware_upload_unregister() must be able to guarantee that all ops are
idle at return.
All ops are either called from sysfs or the workqueue, so unregister sysfs
to stop submissions, cancel any started transfers, flush cancelled
transfers, and then release the device.
This also solves a theoretical race of new submissions starting between
flush_work() and device_unregister(). The module reference was not
protecting against that race.
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Russ Weight <russ.weight@linux.dev>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Reported-by: Chao Gao <chao.gao@intel.com>
Tested-by: Chao Gao <chao.gao@intel.com>
Closes: https://sashiko.dev/#/patchset/20260326084448.29947-1-chao.gao%40intel.com?patch=10705
Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support")
Signed-off-by: Dan Williams <djbw@kernel.org>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
drivers/base/firmware_loader/sysfs_upload.c | 31 ++++++++++-----------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index efc33294212f..6f2123538682 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -312,14 +312,9 @@ firmware_upload_register(struct module *module, struct device *parent,
return ERR_PTR(-EINVAL);
}
- if (!try_module_get(module))
- return ERR_PTR(-EFAULT);
-
fw_upload = kzalloc_obj(*fw_upload);
- if (!fw_upload) {
- ret = -ENOMEM;
- goto exit_module_put;
- }
+ if (!fw_upload)
+ return ERR_PTR(-ENOMEM);
fw_upload_priv = kzalloc_obj(*fw_upload_priv);
if (!fw_upload_priv) {
@@ -361,7 +356,7 @@ firmware_upload_register(struct module *module, struct device *parent,
if (ret) {
dev_err(fw_dev, "%s: device_register failed\n", __func__);
put_device(fw_dev);
- goto exit_module_put;
+ return ERR_PTR(ret);
}
return fw_upload;
@@ -372,9 +367,6 @@ firmware_upload_register(struct module *module, struct device *parent,
free_fw_upload:
kfree(fw_upload);
-exit_module_put:
- module_put(module);
-
return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(firmware_upload_register);
@@ -386,23 +378,28 @@ EXPORT_SYMBOL_GPL(firmware_upload_register);
void firmware_upload_unregister(struct fw_upload *fw_upload)
{
struct fw_sysfs *fw_sysfs = fw_upload->priv;
+ struct device *parent = fw_sysfs->dev.parent;
struct fw_upload_priv *fw_upload_priv = fw_sysfs->fw_upload_priv;
- struct module *module = fw_upload_priv->module;
+
+ /* hold a parent reference while child is unregistered */
+ get_device(parent);
+
+ /* shutdown the sysfs interface to block new requests */
+ device_del(&fw_sysfs->dev);
mutex_lock(&fw_upload_priv->lock);
if (fw_upload_priv->progress == FW_UPLOAD_PROG_IDLE) {
mutex_unlock(&fw_upload_priv->lock);
- goto unregister;
+ goto release;
}
fw_upload_priv->ops->cancel(fw_upload);
mutex_unlock(&fw_upload_priv->lock);
+release:
/* Ensure lower-level device-driver is finished */
flush_work(&fw_upload_priv->work);
-
-unregister:
- device_unregister(&fw_sysfs->dev);
- module_put(module);
+ put_device(&fw_sysfs->dev);
+ put_device(parent);
}
EXPORT_SYMBOL_GPL(firmware_upload_unregister);
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 2/7] firmware_loader: Stop pinning parent device per workqueue invocation
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
2026-09-10 17:01 ` [PATCH 1/7] firmware_loader: Stop pinning modules on registration Pratik R. Sampat
@ 2026-09-10 17:01 ` Pratik R. Sampat
2026-09-10 17:01 ` [PATCH 3/7] treewide: firmware_loader: Drop the unused @module argument Pratik R. Sampat
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:01 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
From: Dan Williams <djbw@kernel.org>
The device core pins parent devices while children are registered. As long
as all usage of the parent device by the firmware_loader ends at
firmware_upload_unregister(), no per queue_work() reference is needed.
Now that firmware_upload_unregister() holds its own parent device reference
over the child device_del() and flush_work() events, the per queue_work()
reference can be deleted.
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Russ Weight <russ.weight@linux.dev>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Chao Gao <chao.gao@intel.com>
Signed-off-by: Dan Williams <djbw@kernel.org>
[chao.gao@intel.com: drop unused fw_dev in fw_upload_start()]
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
drivers/base/firmware_loader/sysfs_upload.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index 6f2123538682..890a242a7c20 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -176,7 +176,7 @@ static void fw_upload_main(struct work_struct *work)
ret = fwlp->ops->prepare(fwl, fwlp->data, fwlp->remaining_size);
if (ret != FW_UPLOAD_ERR_NONE) {
fw_upload_set_error(fwlp, ret);
- goto putdev_exit;
+ goto out;
}
fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_TRANSFERRING);
@@ -205,9 +205,7 @@ static void fw_upload_main(struct work_struct *work)
if (fwlp->ops->cleanup)
fwlp->ops->cleanup(fwl);
-putdev_exit:
- put_device(fw_dev->parent);
-
+out:
/*
* Note: fwlp->remaining_size is left unmodified here to provide
* additional information on errors. It will be reinitialized when
@@ -228,7 +226,6 @@ static void fw_upload_main(struct work_struct *work)
int fw_upload_start(struct fw_sysfs *fw_sysfs)
{
struct fw_priv *fw_priv = fw_sysfs->fw_priv;
- struct device *fw_dev = &fw_sysfs->dev;
struct fw_upload_priv *fwlp;
if (!fw_sysfs->fw_upload_priv)
@@ -249,8 +246,6 @@ int fw_upload_start(struct fw_sysfs *fw_sysfs)
return -EBUSY;
}
- get_device(fw_dev->parent); /* released in fw_upload_main */
-
fwlp->progress = FW_UPLOAD_PROG_RECEIVING;
fwlp->err_code = 0;
fwlp->remaining_size = fw_priv->size;
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 3/7] treewide: firmware_loader: Drop the unused @module argument
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
2026-09-10 17:01 ` [PATCH 1/7] firmware_loader: Stop pinning modules on registration Pratik R. Sampat
2026-09-10 17:01 ` [PATCH 2/7] firmware_loader: Stop pinning parent device per workqueue invocation Pratik R. Sampat
@ 2026-09-10 17:01 ` Pratik R. Sampat
2026-09-10 17:01 ` [PATCH 4/7] crypto: ccp - Factor out the release of the SEV firmware buffers Pratik R. Sampat
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:01 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
From: Dan Williams <djbw@kernel.org>
Now that the firmware loader properly ceases all operations at
firmware_upload_unregister() and no longer takes module references, clean
up the unused parameter.
Cc: Chao Gao <chao.gao@intel.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Russ Weight <russ.weight@linux.dev>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Dan Williams <djbw@kernel.org>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[chao.gao@intel.com: drop leftover fw_upload_priv->module assignment
and the unused @module field from struct fw_upload_priv]
[prsampat@amd.com: convert the tdx-host caller added since the original
posting]
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
Documentation/driver-api/firmware/fw_upload.rst | 2 +-
drivers/base/firmware_loader/sysfs_upload.c | 10 ++++------
drivers/base/firmware_loader/sysfs_upload.h | 1 -
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 +++++++--------
lib/test_firmware.c | 3 +--
12 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/Documentation/driver-api/firmware/fw_upload.rst b/Documentation/driver-api/firmware/fw_upload.rst
index edf1d0c5e7c3..702b1ed77551 100644
--- a/Documentation/driver-api/firmware/fw_upload.rst
+++ b/Documentation/driver-api/firmware/fw_upload.rst
@@ -57,7 +57,7 @@ function calls firmware_upload_unregister() such as::
len = (truncate) ? truncate - fw_name : strlen(fw_name);
sec->fw_name = kmemdup_nul(fw_name, len, GFP_KERNEL);
- fwl = firmware_upload_register(THIS_MODULE, sec->dev, sec->fw_name,
+ fwl = firmware_upload_register(sec->dev, sec->fw_name,
&m10bmc_ops, sec);
if (IS_ERR(fwl)) {
dev_err(sec->dev, "Firmware Upload driver failed to start\n");
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index 890a242a7c20..920c4b640f96 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -273,7 +273,6 @@ void fw_upload_free(struct fw_sysfs *fw_sysfs)
/**
* firmware_upload_register() - register for the firmware upload sysfs API
- * @module: kernel module of this device
* @parent: parent device instantiating firmware upload
* @name: firmware name to be associated with this device
* @ops: pointer to structure of firmware upload ops
@@ -285,10 +284,10 @@ void fw_upload_free(struct fw_sysfs *fw_sysfs)
* Return: struct fw_upload pointer or ERR_PTR()
*
**/
-struct fw_upload *
-firmware_upload_register(struct module *module, struct device *parent,
- const char *name, const struct fw_upload_ops *ops,
- void *dd_handle)
+struct fw_upload *firmware_upload_register(struct device *parent,
+ const char *name,
+ const struct fw_upload_ops *ops,
+ void *dd_handle)
{
u32 opt_flags = FW_OPT_NOCACHE;
struct fw_upload *fw_upload;
@@ -320,7 +319,6 @@ firmware_upload_register(struct module *module, struct device *parent,
fw_upload_priv->fw_upload = fw_upload;
fw_upload_priv->ops = ops;
mutex_init(&fw_upload_priv->lock);
- fw_upload_priv->module = module;
fw_upload_priv->name = name;
fw_upload_priv->err_code = 0;
fw_upload_priv->progress = FW_UPLOAD_PROG_IDLE;
diff --git a/drivers/base/firmware_loader/sysfs_upload.h b/drivers/base/firmware_loader/sysfs_upload.h
index 31931ff7808a..dc7ccdceb96f 100644
--- a/drivers/base/firmware_loader/sysfs_upload.h
+++ b/drivers/base/firmware_loader/sysfs_upload.h
@@ -26,7 +26,6 @@ enum fw_upload_prog {
struct fw_upload_priv {
struct fw_upload *fw_upload;
- struct module *module;
const char *name;
const struct fw_upload_ops *ops;
struct mutex lock; /* protect data structure contents */
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index b3419df586b9..ee7a97b8b2c8 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -1151,8 +1151,8 @@ int devm_cxl_setup_fw_upload(struct device *host, struct cxl_memdev_state *mds)
if (!test_bit(CXL_MEM_COMMAND_ID_GET_FW_INFO, cxl_mbox->enabled_cmds))
return 0;
- fwl = firmware_upload_register(THIS_MODULE, dev, dev_name(dev),
- &cxl_memdev_fw_ops, mds);
+ fwl = firmware_upload_register(dev, dev_name(dev), &cxl_memdev_fw_ops,
+ mds);
if (IS_ERR(fwl))
return PTR_ERR(fwl);
return devm_add_action_or_reset(host, cxl_remove_fw_upload, fwl);
diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
index 1211fd8d0463..992088606204 100644
--- a/drivers/firmware/microchip/mpfs-auto-update.c
+++ b/drivers/firmware/microchip/mpfs-auto-update.c
@@ -437,7 +437,7 @@ static int mpfs_auto_update_probe(struct platform_device *pdev)
return dev_err_probe(dev, ret,
"The current bitstream does not support auto-update\n");
- fw_uploader = firmware_upload_register(THIS_MODULE, dev, "mpfs-auto-update",
+ fw_uploader = firmware_upload_register(dev, "mpfs-auto-update",
&mpfs_auto_update_ops, priv);
if (IS_ERR(fw_uploader))
return dev_err_probe(dev, PTR_ERR(fw_uploader),
diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index 7d23d914df3f..3d318570faba 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -712,8 +712,8 @@ static int m10bmc_sec_probe(struct platform_device *pdev)
goto fw_name_fail;
}
- fwl = firmware_upload_register(THIS_MODULE, sec->dev, sec->fw_name,
- &m10bmc_ops, sec);
+ fwl = firmware_upload_register(sec->dev, sec->fw_name, &m10bmc_ops,
+ sec);
if (IS_ERR(fwl)) {
dev_err(sec->dev, "Firmware Upload driver failed to start\n");
ret = PTR_ERR(fwl);
diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c
index 244966d56c9b..a097a3b10552 100644
--- a/drivers/greybus/gb-beagleplay.c
+++ b/drivers/greybus/gb-beagleplay.c
@@ -1172,7 +1172,7 @@ static int gb_fw_init(struct gb_beagleplay *bg)
return PTR_ERR(desc);
bg->rst_gpio = desc;
- fwl = firmware_upload_register(THIS_MODULE, &bg->sd->dev, "cc1352p7",
+ fwl = firmware_upload_register(&bg->sd->dev, "cc1352p7",
&cc1352_bootloader_ops, bg);
if (IS_ERR(fwl))
return PTR_ERR(fwl);
diff --git a/drivers/media/i2c/thp7312.c b/drivers/media/i2c/thp7312.c
index 775cfba188d8..ebbe14393c9f 100644
--- a/drivers/media/i2c/thp7312.c
+++ b/drivers/media/i2c/thp7312.c
@@ -1909,7 +1909,7 @@ static int thp7312_register_flash_mode(struct thp7312_device *thp7312)
goto error;
}
- fwl = firmware_upload_register(THIS_MODULE, dev, "thp7312-firmware",
+ fwl = firmware_upload_register(dev, "thp7312-firmware",
&thp7312_fw_upload_ops, thp7312);
if (IS_ERR(fwl)) {
ret = PTR_ERR(fwl);
diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 209de9cec849..5bb91a0f4438 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -1841,8 +1841,8 @@ static int pd692x0_i2c_probe(struct i2c_client *client)
return dev_err_probe(dev, ret,
"failed to register PSE controller\n");
- fwl = firmware_upload_register(THIS_MODULE, dev, dev_name(dev),
- &pd692x0_fw_ops, priv);
+ fwl = firmware_upload_register(dev, dev_name(dev), &pd692x0_fw_ops,
+ priv);
if (IS_ERR(fwl))
return dev_err_probe(dev, PTR_ERR(fwl),
"failed to register to the Firmware Upload API\n");
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index e8ed8dfa526e..f11a545d40cf 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -194,8 +194,8 @@ static int seamldr_init(struct device *dev)
if (!supports_runtime_update())
return 0;
- tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
- &tdx_fw_ops, NULL);
+ tdx_fwl = firmware_upload_register(dev, "tdx_module", &tdx_fw_ops,
+ NULL);
if (IS_ERR(tdx_fwl))
return PTR_ERR(tdx_fwl);
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 0fa3b027f02f..aa0333095c10 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -197,20 +197,19 @@ static inline int request_partial_firmware_into_buf
#ifdef CONFIG_FW_UPLOAD
-struct fw_upload *
-firmware_upload_register(struct module *module, struct device *parent,
- const char *name, const struct fw_upload_ops *ops,
- void *dd_handle);
+struct fw_upload *firmware_upload_register(struct device *parent,
+ const char *name,
+ const struct fw_upload_ops *ops,
+ void *dd_handle);
void firmware_upload_unregister(struct fw_upload *fw_upload);
#else
static inline struct fw_upload *
-firmware_upload_register(struct module *module, struct device *parent,
- const char *name, const struct fw_upload_ops *ops,
- void *dd_handle)
+firmware_upload_register(struct device *parent, const char *name,
+ const struct fw_upload_ops *ops, void *dd_handle)
{
- return ERR_PTR(-EINVAL);
+ return ERR_PTR(-EINVAL);
}
static inline void firmware_upload_unregister(struct fw_upload *fw_upload)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 7459bba65444..a77a99404fd3 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -1322,8 +1322,7 @@ static ssize_t upload_register_store(struct device *dev,
goto free_tst;
}
- fwl = firmware_upload_register(THIS_MODULE, dev, tst->name,
- &upload_test_ops, tst);
+ fwl = firmware_upload_register(dev, tst->name, &upload_test_ops, tst);
if (IS_ERR(fwl)) {
ret = PTR_ERR(fwl);
goto free_buf;
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 4/7] crypto: ccp - Factor out the release of the SEV firmware buffers
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
` (2 preceding siblings ...)
2026-09-10 17:01 ` [PATCH 3/7] treewide: firmware_loader: Drop the unused @module argument Pratik R. Sampat
@ 2026-09-10 17:01 ` Pratik R. Sampat
2026-09-10 17:02 ` [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT Pratik R. Sampat
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:01 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
__sev_firmware_shutdown() reclaims and frees the TMR and the INIT_EX NV
area once the platform has been taken down. Move that into a helper so
that other paths which de-initialize the platform can reuse the same
path.
No functional change intended.
Suggested-by: Shantanu Sinha <shansinha@google.com>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
drivers/crypto/ccp/sev-dev.c | 55 ++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index f833cb7e4da3..1ed9e61a95cc 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1881,6 +1881,35 @@ static int __sev_platform_shutdown_locked(int *error)
return ret;
}
+static void __sev_release_firmware_buffers(bool panic)
+{
+ if (sev_es_tmr) {
+ /*
+ * The TMR area was encrypted, flush it from the cache.
+ *
+ * If invoked during panic handling, local interrupts are
+ * disabled and all CPUs are stopped, so wbinvd_on_all_cpus()
+ * can't be used. In that case, wbinvd() is done on remote CPUs
+ * via the NMI callback, and done for this CPU later during
+ * SNP shutdown, so wbinvd_on_all_cpus() can be skipped.
+ */
+ if (!panic)
+ wbinvd_on_all_cpus();
+
+ __snp_free_firmware_pages(virt_to_page(sev_es_tmr),
+ get_order(sev_es_tmr_size),
+ true);
+ sev_es_tmr = NULL;
+ }
+
+ if (sev_init_ex_buffer) {
+ __snp_free_firmware_pages(virt_to_page(sev_init_ex_buffer),
+ get_order(NV_LENGTH),
+ true);
+ sev_init_ex_buffer = NULL;
+ }
+}
+
static int sev_get_platform_state(int *state, int *error)
{
struct sev_user_data_status data;
@@ -2906,31 +2935,7 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic)
__sev_platform_shutdown_locked(&error);
- if (sev_es_tmr) {
- /*
- * The TMR area was encrypted, flush it from the cache.
- *
- * If invoked during panic handling, local interrupts are
- * disabled and all CPUs are stopped, so wbinvd_on_all_cpus()
- * can't be used. In that case, wbinvd() is done on remote CPUs
- * via the NMI callback, and done for this CPU later during
- * SNP shutdown, so wbinvd_on_all_cpus() can be skipped.
- */
- if (!panic)
- wbinvd_on_all_cpus();
-
- __snp_free_firmware_pages(virt_to_page(sev_es_tmr),
- get_order(sev_es_tmr_size),
- true);
- sev_es_tmr = NULL;
- }
-
- if (sev_init_ex_buffer) {
- __snp_free_firmware_pages(virt_to_page(sev_init_ex_buffer),
- get_order(NV_LENGTH),
- true);
- sev_init_ex_buffer = NULL;
- }
+ __sev_release_firmware_buffers(panic);
__sev_snp_shutdown_locked(&error, panic);
}
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
` (3 preceding siblings ...)
2026-09-10 17:01 ` [PATCH 4/7] crypto: ccp - Factor out the release of the SEV firmware buffers Pratik R. Sampat
@ 2026-09-10 17:02 ` Pratik R. Sampat
2026-09-10 20:09 ` Tom Lendacky
2026-09-10 17:02 ` [PATCH 6/7] crypto/ccp: Register with fw_uploader and always fail Pratik R. Sampat
2026-09-10 17:02 ` [PATCH 7/7] crypto/ccp: Implement SNP Download Firmware EX Pratik R. Sampat
6 siblings, 1 reply; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:02 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
In preparation for refreshing the cached SNP platform status and feature
information after a successful firmware live update from
DOWNLOAD_FIRMWARE_EX, allow snp_get_platform_data() to be called while
the SNP firmware is in the INIT state.
Once SNP is initialized the firmware requires the output page of both
commands to be firmware-owned. sev->snp_plat_status cannot satisfy that
as it is embedded in struct sev_device, so use
__sev_do_snp_platform_status(), which stages the output through a
dedicated page, and mark/reclaim the SNP_FEATURE_INFO page around the
command.
Co-developed-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
drivers/crypto/ccp/sev-dev.c | 44 ++++++++++++++++++++++++++++--------
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 1ed9e61a95cc..e891d6d1c6f0 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -131,6 +131,8 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic);
static int snp_shutdown_on_panic(struct notifier_block *nb,
unsigned long reason, void *arg);
+static int __sev_do_snp_platform_status(struct sev_user_data_snp_status *status,
+ int *error);
static struct notifier_block snp_panic_notifier = {
.notifier_call = snp_shutdown_on_panic,
@@ -1261,19 +1263,12 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
{
struct sev_data_snp_feature_info snp_feat_info;
struct snp_feature_info *feat_info;
- struct sev_data_snp_addr buf;
struct page *page;
int rc;
- /*
- * This function is expected to be called before SNP is
- * initialized.
- */
- if (sev->snp_initialized)
- return -EINVAL;
-
- buf.address = __psp_pa(&sev->snp_plat_status);
- rc = sev_do_cmd(SEV_CMD_SNP_PLATFORM_STATUS, &buf, error);
+ mutex_lock(&sev_cmd_mutex);
+ rc = __sev_do_snp_platform_status(&sev->snp_plat_status, error);
+ mutex_unlock(&sev_cmd_mutex);
if (rc) {
dev_err(sev->dev, "SNP PLATFORM_STATUS command failed, ret = %d, error = %#x\n",
rc, *error);
@@ -1302,17 +1297,46 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
return -ENOMEM;
feat_info = page_address(page);
+
+ /* If SNP is initialized, transition to use a firmware-owned page */
+ if (sev->snp_initialized) {
+ if (rmp_mark_pages_firmware(__pa(feat_info), 1, false)) {
+ *error = SEV_RET_NO_FW_CALL;
+ rc = -EFAULT;
+ goto free_page;
+ }
+ }
+
snp_feat_info.length = sizeof(snp_feat_info);
snp_feat_info.ecx_in = 0;
snp_feat_info.feature_info_paddr = __psp_pa(feat_info);
rc = sev_do_cmd(SEV_CMD_SNP_FEATURE_INFO, &snp_feat_info, error);
+
+ /*
+ * The feature_info page will be in reclaim state on success, or left
+ * in firmware state on failure. Transition the pages back to
+ * Hypervisor-owned state.
+ *
+ * snp_reclaim_pages() has already pinned the page via snp_leak_pages()
+ * if it could not do so, which keeps it away from the allocator. The
+ * reference taken here is dropped either way.
+ */
+ if (sev->snp_initialized) {
+ if (snp_reclaim_pages(__pa(feat_info), 1, false)) {
+ *error = SEV_RET_NO_FW_CALL;
+ rc = -EFAULT;
+ goto free_page;
+ }
+ }
+
if (!rc)
sev->snp_feat_info_0 = *feat_info;
else
dev_err(sev->dev, "SNP FEATURE_INFO command failed, ret = %d, error = %#x\n",
rc, *error);
+free_page:
__free_page(page);
return rc;
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT
2026-09-10 17:02 ` [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT Pratik R. Sampat
@ 2026-09-10 20:09 ` Tom Lendacky
2026-09-10 21:05 ` Pratik R. Sampat
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lendacky @ 2026-09-10 20:09 UTC (permalink / raw)
To: Pratik R. Sampat, mcgrof, russ.weight, dakr, ashish.kalra,
herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha
On 9/10/26 12:02, Pratik R. Sampat wrote:
> In preparation for refreshing the cached SNP platform status and feature
> information after a successful firmware live update from
> DOWNLOAD_FIRMWARE_EX, allow snp_get_platform_data() to be called while
> the SNP firmware is in the INIT state.
>
> Once SNP is initialized the firmware requires the output page of both
> commands to be firmware-owned. sev->snp_plat_status cannot satisfy that
> as it is embedded in struct sev_device, so use
> __sev_do_snp_platform_status(), which stages the output through a
> dedicated page, and mark/reclaim the SNP_FEATURE_INFO page around the
> command.
>
> Co-developed-by: Tycho Andersen (AMD) <tycho@kernel.org>
> Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
> ---
> drivers/crypto/ccp/sev-dev.c | 44 ++++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 1ed9e61a95cc..e891d6d1c6f0 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -131,6 +131,8 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic);
>
> static int snp_shutdown_on_panic(struct notifier_block *nb,
> unsigned long reason, void *arg);
> +static int __sev_do_snp_platform_status(struct sev_user_data_snp_status *status,
> + int *error);
>
> static struct notifier_block snp_panic_notifier = {
> .notifier_call = snp_shutdown_on_panic,
> @@ -1261,19 +1263,12 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
> {
> struct sev_data_snp_feature_info snp_feat_info;
> struct snp_feature_info *feat_info;
> - struct sev_data_snp_addr buf;
> struct page *page;
> int rc;
>
> - /*
> - * This function is expected to be called before SNP is
> - * initialized.
> - */
> - if (sev->snp_initialized)
> - return -EINVAL;
> -
> - buf.address = __psp_pa(&sev->snp_plat_status);
> - rc = sev_do_cmd(SEV_CMD_SNP_PLATFORM_STATUS, &buf, error);
> + mutex_lock(&sev_cmd_mutex);
> + rc = __sev_do_snp_platform_status(&sev->snp_plat_status, error);
> + mutex_unlock(&sev_cmd_mutex);
> if (rc) {
> dev_err(sev->dev, "SNP PLATFORM_STATUS command failed, ret = %d, error = %#x\n",
> rc, *error);
> @@ -1302,17 +1297,46 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
> return -ENOMEM;
>
> feat_info = page_address(page);
> +
> + /* If SNP is initialized, transition to use a firmware-owned page */
> + if (sev->snp_initialized) {
> + if (rmp_mark_pages_firmware(__pa(feat_info), 1, false)) {
> + *error = SEV_RET_NO_FW_CALL;
> + rc = -EFAULT;
> + goto free_page;
> + }
> + }
> +
You could change to use snp_alloc_firmware_page() which will allocate
the page and place it in the proper state for you.
> snp_feat_info.length = sizeof(snp_feat_info);
> snp_feat_info.ecx_in = 0;
> snp_feat_info.feature_info_paddr = __psp_pa(feat_info);
>
> rc = sev_do_cmd(SEV_CMD_SNP_FEATURE_INFO, &snp_feat_info, error);
> +
> + /*
> + * The feature_info page will be in reclaim state on success, or left
> + * in firmware state on failure. Transition the pages back to
> + * Hypervisor-owned state.
> + *
> + * snp_reclaim_pages() has already pinned the page via snp_leak_pages()
> + * if it could not do so, which keeps it away from the allocator. The
> + * reference taken here is dropped either way.
> + */
> + if (sev->snp_initialized) {
> + if (snp_reclaim_pages(__pa(feat_info), 1, false)) {
> + *error = SEV_RET_NO_FW_CALL;
> + rc = -EFAULT;
> + goto free_page;
> + }
> + }
And then snp_free_firmware_page() here (after you copy the data -
similar to what you did in snp_verify_mitigation()).
(I think the same changes apply to __sev_do_snp_platform_status(), but
don't worry about it for this series)
Thanks,
Tom
> +
> if (!rc)
> sev->snp_feat_info_0 = *feat_info;
> else
> dev_err(sev->dev, "SNP FEATURE_INFO command failed, ret = %d, error = %#x\n",
> rc, *error);
>
> +free_page:
> __free_page(page);
>
> return rc;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT
2026-09-10 20:09 ` Tom Lendacky
@ 2026-09-10 21:05 ` Pratik R. Sampat
0 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 21:05 UTC (permalink / raw)
To: Tom Lendacky, mcgrof, russ.weight, dakr, ashish.kalra, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha
Hi Tom,
Thanks for the review.
On 9/10/26 4:09 PM, Tom Lendacky wrote:
> On 9/10/26 12:02, Pratik R. Sampat wrote:
>> In preparation for refreshing the cached SNP platform status and feature
>> information after a successful firmware live update from
>> DOWNLOAD_FIRMWARE_EX, allow snp_get_platform_data() to be called while
>> the SNP firmware is in the INIT state.
>>
>> Once SNP is initialized the firmware requires the output page of both
>> commands to be firmware-owned. sev->snp_plat_status cannot satisfy that
>> as it is embedded in struct sev_device, so use
>> __sev_do_snp_platform_status(), which stages the output through a
>> dedicated page, and mark/reclaim the SNP_FEATURE_INFO page around the
>> command.
>>
>> Co-developed-by: Tycho Andersen (AMD) <tycho@kernel.org>
>> Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>> ---
>> drivers/crypto/ccp/sev-dev.c | 44 ++++++++++++++++++++++++++++--------
>> 1 file changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index 1ed9e61a95cc..e891d6d1c6f0 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -131,6 +131,8 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic);
>>
>> static int snp_shutdown_on_panic(struct notifier_block *nb,
>> unsigned long reason, void *arg);
>> +static int __sev_do_snp_platform_status(struct sev_user_data_snp_status *status,
>> + int *error);
>>
>> static struct notifier_block snp_panic_notifier = {
>> .notifier_call = snp_shutdown_on_panic,
>> @@ -1261,19 +1263,12 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
>> {
>> struct sev_data_snp_feature_info snp_feat_info;
>> struct snp_feature_info *feat_info;
>> - struct sev_data_snp_addr buf;
>> struct page *page;
>> int rc;
>>
>> - /*
>> - * This function is expected to be called before SNP is
>> - * initialized.
>> - */
>> - if (sev->snp_initialized)
>> - return -EINVAL;
>> -
>> - buf.address = __psp_pa(&sev->snp_plat_status);
>> - rc = sev_do_cmd(SEV_CMD_SNP_PLATFORM_STATUS, &buf, error);
>> + mutex_lock(&sev_cmd_mutex);
>> + rc = __sev_do_snp_platform_status(&sev->snp_plat_status, error);
>> + mutex_unlock(&sev_cmd_mutex);
>> if (rc) {
>> dev_err(sev->dev, "SNP PLATFORM_STATUS command failed, ret = %d, error = %#x\n",
>> rc, *error);
>> @@ -1302,17 +1297,46 @@ static int snp_get_platform_data(struct sev_device *sev, int *error)
>> return -ENOMEM;
>>
>> feat_info = page_address(page);
>> +
>> + /* If SNP is initialized, transition to use a firmware-owned page */
>> + if (sev->snp_initialized) {
>> + if (rmp_mark_pages_firmware(__pa(feat_info), 1, false)) {
>> + *error = SEV_RET_NO_FW_CALL;
>> + rc = -EFAULT;
>> + goto free_page;
>> + }
>> + }
>> +
>
> You could change to use snp_alloc_firmware_page() which will allocate
> the page and place it in the proper state for you.
>
>> snp_feat_info.length = sizeof(snp_feat_info);
>> snp_feat_info.ecx_in = 0;
>> snp_feat_info.feature_info_paddr = __psp_pa(feat_info);
>>
>> rc = sev_do_cmd(SEV_CMD_SNP_FEATURE_INFO, &snp_feat_info, error);
>> +
>> + /*
>> + * The feature_info page will be in reclaim state on success, or left
>> + * in firmware state on failure. Transition the pages back to
>> + * Hypervisor-owned state.
>> + *
>> + * snp_reclaim_pages() has already pinned the page via snp_leak_pages()
>> + * if it could not do so, which keeps it away from the allocator. The
>> + * reference taken here is dropped either way.
>> + */
>> + if (sev->snp_initialized) {
>> + if (snp_reclaim_pages(__pa(feat_info), 1, false)) {
>> + *error = SEV_RET_NO_FW_CALL;
>> + rc = -EFAULT;
>> + goto free_page;
>> + }
>> + }
>
> And then snp_free_firmware_page() here (after you copy the data -
> similar to what you did in snp_verify_mitigation()).
>
Sure, I'll use snp_alloc_firmware_page + snp_free_firmware_page() and make
things cleaner.
> (I think the same changes apply to __sev_do_snp_platform_status(), but
> don't worry about it for this series)
>
Right. The only difference I believe would be to use
__snp_alloc_firmware_pages() and __snp_free_firmware_pages() with locked=true
since that is always called with the sev_cmd_mutex held.
I could send that as a separate clean up patch.
Thanks!
--Pratik
> Thanks,
> Tom
>
>> +
>> if (!rc)
>> sev->snp_feat_info_0 = *feat_info;
>> else
>> dev_err(sev->dev, "SNP FEATURE_INFO command failed, ret = %d, error = %#x\n",
>> rc, *error);
>>
>> +free_page:
>> __free_page(page);
>>
>> return rc;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 6/7] crypto/ccp: Register with fw_uploader and always fail
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
` (4 preceding siblings ...)
2026-09-10 17:02 ` [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT Pratik R. Sampat
@ 2026-09-10 17:02 ` Pratik R. Sampat
2026-09-10 17:02 ` [PATCH 7/7] crypto/ccp: Implement SNP Download Firmware EX Pratik R. Sampat
6 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:02 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
In preparation for SEV-SNP DOWNLOAD_FIRMWARE_EX live firmware update
support, add an 'sev' firmware loader that always fails with EBUSY.
Co-developed-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
drivers/crypto/ccp/sev-dev.c | 81 ++++++++++++++++++++++++++++++++++++
drivers/crypto/ccp/sev-dev.h | 2 +
2 files changed, 83 insertions(+)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index e891d6d1c6f0..397a797d9312 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -2239,6 +2239,81 @@ static int sev_update_firmware(struct device *dev)
return ret;
}
+#ifdef CONFIG_FW_UPLOAD
+static enum fw_upload_err sev_fw_upload_prepare(struct fw_upload *fw_upload,
+ const u8 *data, u32 size)
+{
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err sev_fw_upload_write(struct fw_upload *fw_upload,
+ const u8 *data, u32 offset,
+ u32 size, u32 *written)
+{
+ return FW_UPLOAD_ERR_BUSY;
+}
+
+static enum fw_upload_err sev_fw_upload_poll_complete(struct fw_upload *fw_upload)
+{
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static void sev_fw_upload_cancel(struct fw_upload *fw_upload)
+{
+ /* intentional no-op */
+}
+
+static const struct fw_upload_ops sev_fw_upload_ops = {
+ .prepare = sev_fw_upload_prepare,
+ .write = sev_fw_upload_write,
+ .poll_complete = sev_fw_upload_poll_complete,
+ .cancel = sev_fw_upload_cancel,
+};
+
+static void register_sev_fw_uploader(struct sev_device *sev)
+{
+ struct fw_upload *fwl;
+
+ /*
+ * SNP firmware update is a platform-wide operation; only the master
+ * PSP issues firmware commands. Register a single global interface.
+ */
+ if (sev->psp != psp_master || sev->fwl)
+ return;
+
+ fwl = firmware_upload_register(sev->dev, "sev", &sev_fw_upload_ops,
+ sev);
+ if (IS_ERR(fwl)) {
+ dev_err(sev->dev, "SEV firmware upload registration failure: %ld\n",
+ PTR_ERR(fwl));
+ return;
+ }
+
+ sev->fwl = fwl;
+}
+
+/*
+ * The upload ops issue PSP commands, so the interface has to be gone before
+ * the SEV firmware is shut down and the PSP interrupt handler is cleared.
+ * devm teardown on sev->dev runs after the driver's remove callback and is
+ * therefore too late. Both teardown paths call this, so it has to tolerate
+ * being called twice.
+ */
+static void unregister_sev_fw_uploader(struct sev_device *sev)
+{
+ struct fw_upload *fwl = sev->fwl;
+
+ if (!fwl)
+ return;
+
+ sev->fwl = NULL;
+ firmware_upload_unregister(fwl);
+}
+#else /* CONFIG_FW_UPLOAD */
+static void register_sev_fw_uploader(struct sev_device *sev) { }
+static void unregister_sev_fw_uploader(struct sev_device *sev) { }
+#endif /* CONFIG_FW_UPLOAD */
+
static int __sev_snp_shutdown_locked(int *error, bool panic)
{
struct psp_device *psp = psp_master;
@@ -3045,6 +3120,8 @@ void sev_dev_destroy(struct psp_device *psp)
if (!sev)
return;
+ unregister_sev_fw_uploader(sev);
+
sev_firmware_shutdown(sev);
if (sev->misc)
@@ -3108,6 +3185,8 @@ void sev_pci_init(void)
api_major, api_minor, build,
sev->api_major, sev->api_minor, sev->build);
+ register_sev_fw_uploader(sev);
+
return;
err:
@@ -3123,6 +3202,8 @@ void sev_pci_exit(void)
if (!sev)
return;
+ unregister_sev_fw_uploader(sev);
+
sev_firmware_shutdown(sev);
}
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index d5e596606def..7ec692e2147e 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -69,6 +69,8 @@ struct sev_device {
struct tsm_dev *tsmdev;
struct sev_tio_status *tio_status;
+
+ struct fw_upload *fwl;
};
int sev_dev_init(struct psp_device *psp);
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 7/7] crypto/ccp: Implement SNP Download Firmware EX
2026-09-10 17:01 [PATCH 0/7] Implement SNP live firmware update support Pratik R. Sampat
` (5 preceding siblings ...)
2026-09-10 17:02 ` [PATCH 6/7] crypto/ccp: Register with fw_uploader and always fail Pratik R. Sampat
@ 2026-09-10 17:02 ` Pratik R. Sampat
6 siblings, 0 replies; 10+ messages in thread
From: Pratik R. Sampat @ 2026-09-10 17:02 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr, ashish.kalra, thomas.lendacky, herbert, davem
Cc: linux-crypto, linux-kernel, gregkh, rafael, chao.gao, aik, tycho,
nikunj, michael.roth, shansinha, prsampat
Implement SNP live firmware update using the DOWNLOAD_FIRMWARE_EX
command.
DOWNLOAD_FIRMWARE_EX requires the legacy SEV platform to be UNINIT. If
it is WORKING then legacy guests are running and the update is refused
as busy. If it is INIT, shut it down, release the buffers the firmware
owns across that shutdown, run the update, and bring the platform back
up afterwards. SNP is never taken down, so SNP guests are unaffected.
To test run the following with your sbin file in FW:
echo 1 > /sys/class/firmware/sev/loading
cat <firmware.sbin> > /sys/class/firmware/sev/data
echo 0 > /sys/class/firmware/sev/loading
The COMMIT bit is left clear, so the image is only loaded provisionally
and the admin decides when to make it permanent with ioctl(/dev/sev,
SNP_COMMIT). To roll back, do not commit and upload the previous image
the same way.
Co-developed-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
drivers/crypto/ccp/sev-dev.c | 268 ++++++++++++++++++++++++++++++++++-
drivers/crypto/ccp/sev-dev.h | 2 +
include/linux/psp-sev.h | 19 +++
3 files changed, 288 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 397a797d9312..c95b27275ac2 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -29,6 +29,7 @@
#include <linux/psp.h>
#include <linux/amd-iommu.h>
#include <linux/crash_dump.h>
+#include <linux/sizes.h>
#include <asm/smp.h>
#include <asm/cacheflush.h>
@@ -252,6 +253,7 @@ static int sev_cmd_buffer_len(int cmd)
case SEV_CMD_SNP_PLATFORM_STATUS: return sizeof(struct sev_data_snp_addr);
case SEV_CMD_SNP_GUEST_REQUEST: return sizeof(struct sev_data_snp_guest_request);
case SEV_CMD_SNP_CONFIG: return sizeof(struct sev_user_data_snp_config);
+ case SEV_CMD_SNP_DOWNLOAD_FIRMWARE_EX: return sizeof(struct sev_data_download_firmware_ex);
case SEV_CMD_SNP_COMMIT: return sizeof(struct sev_data_snp_commit);
case SEV_CMD_SNP_FEATURE_INFO: return sizeof(struct sev_data_snp_feature_info);
case SEV_CMD_SNP_VLEK_LOAD: return sizeof(struct sev_user_data_snp_vlek_load);
@@ -2240,17 +2242,274 @@ static int sev_update_firmware(struct device *dev)
}
#ifdef CONFIG_FW_UPLOAD
+/* Largest image the firmware accepts, anything above is rejected */
+#define SEV_FW_IMAGE_MAX_SIZE SZ_512K
+
static enum fw_upload_err sev_fw_upload_prepare(struct fw_upload *fw_upload,
const u8 *data, u32 size)
{
+ struct sev_device *sev = fw_upload->dd_handle;
+
+ if (size > SEV_FW_IMAGE_MAX_SIZE) {
+ dev_err(sev->dev, "DLFW_EX: image of %u bytes exceeds the %u byte maximum\n",
+ size, SEV_FW_IMAGE_MAX_SIZE);
+ return FW_UPLOAD_ERR_INVALID_SIZE;
+ }
+
return FW_UPLOAD_ERR_NONE;
}
+static int sev_download_firmware_ex(const u8 *data, u32 size, int *psp_ret)
+{
+ struct sev_data_download_firmware_ex sev_data = {0};
+ int ret, order;
+ struct page *p;
+ void *fw_blob;
+
+ order = get_order(size);
+ p = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
+ if (!p)
+ return -ENOMEM;
+
+ fw_blob = page_address(p);
+ memcpy(fw_blob, data, size);
+
+ sev_data.len = sizeof(sev_data);
+ sev_data.fw_paddr = __psp_pa(fw_blob);
+ sev_data.fw_len = size;
+ /*
+ * Don't commit to the new firmware immediately, perform an explicit
+ * SNP_COMMIT after
+ */
+ sev_data.commit = 0;
+
+ ret = __sev_do_cmd_locked(SEV_CMD_SNP_DOWNLOAD_FIRMWARE_EX, &sev_data,
+ psp_ret);
+
+ __free_pages(p, order);
+
+ return ret;
+}
+
+static enum fw_upload_err sev_fw_upload_handle_err(struct sev_device *sev,
+ int rc, int psp_ret)
+{
+ enum fw_upload_err ret = FW_UPLOAD_ERR_FW_INVALID;
+
+ if (!rc)
+ return FW_UPLOAD_ERR_NONE;
+
+ switch (psp_ret) {
+ case SEV_RET_INVALID_PARAM:
+ dev_err(sev->dev, "DLFW_EX: Provided image is not well formed\n");
+ break;
+ case SEV_RET_INVALID_LEN:
+ ret = FW_UPLOAD_ERR_INVALID_SIZE;
+ dev_err(sev->dev, "DLFW_EX: Provided image has an unusable length\n");
+ break;
+ case SEV_RET_SHUTDOWN_REQUIRED:
+ dev_err(sev->dev, "DLFW_EX: Provided image cannot be live-updated, shutdown required\n");
+ break;
+ case SEV_RET_BAD_VERSION:
+ dev_err(sev->dev, "DLFW_EX: Provided image < committed version\n");
+ break;
+ case SEV_RET_INVALID_PLATFORM_STATE:
+ dev_err(sev->dev, "DLFW_EX: Platform not in UNINIT state\n");
+ break;
+ case SEV_RET_INVALID_ADDRESS:
+ dev_err(sev->dev, "DLFW_EX: Unaligned address provided\n");
+ break;
+ case SEV_RET_UNSUPPORTED:
+ dev_err(sev->dev, "DLFW_EX: feature not supported\n");
+ break;
+ case SEV_RET_INVALID_CONFIG:
+ dev_err(sev->dev, "DLFW_EX: Image rejected, unsupported configuration\n");
+ break;
+ case SEV_RET_BAD_SIGNATURE:
+ dev_err(sev->dev, "DLFW_EX: Bad firmware signature\n");
+ break;
+ case SEV_RET_UPDATE_FAILED:
+ ret = FW_UPLOAD_ERR_HW_ERROR;
+ dev_err(sev->dev, "DLFW_EX: Upgrade failed, automatically reverted\n");
+ break;
+ case SEV_RET_RESTORE_REQUIRED:
+ dev_err(sev->dev, "DLFW_EX: live upgrade failed, please roll back\n");
+ /*
+ * Firmware requested a roll-back. Declare the PSP dead so
+ * nothing else tries to use it, and let the next upload through
+ * so the admin can restore the previous image.
+ */
+ sev->fwl_rollback_required = true;
+ psp_dead = true;
+ ret = FW_UPLOAD_ERR_HW_ERROR;
+ break;
+ case SEV_RET_HWSEV_RET_UNSAFE:
+ dev_err(sev->dev, "DLFW_EX: SEV firmware no longer safe. Reboot recommended\n");
+ /*
+ * Following a return of HARDWARE_UNSAFE, operation of the SEV
+ * firmware is indeterminate and the recommendation is to reboot
+ * the platform. Declare the PSP dead so the driver stops
+ * issuing commands to it while the reboot is pending.
+ */
+ psp_dead = true;
+ ret = FW_UPLOAD_ERR_HW_ERROR;
+ break;
+ case SEV_RET_NO_FW_CALL:
+ /* The command never reached the firmware. */
+ dev_err(sev->dev, "DLFW_EX: driver error %d\n", rc);
+ ret = FW_UPLOAD_ERR_HW_ERROR;
+ break;
+ default:
+ dev_err(sev->dev, "Unknown SEV firmware err 0x%x\n", psp_ret);
+ ret = FW_UPLOAD_ERR_HW_ERROR;
+ break;
+ }
+
+ return ret;
+}
+
+static int sev_fw_upload_shutdown_platform(struct sev_device *sev)
+{
+ int rc, error = SEV_RET_NO_FW_CALL, sev_plat_state;
+
+ lockdep_assert_held(&sev_cmd_mutex);
+
+ rc = sev_get_platform_state(&sev_plat_state, &error);
+ if (rc) {
+ dev_err(sev->dev, "SEV get platform state failed %d, error %#x\n",
+ rc, error);
+ return rc;
+ }
+
+ switch (sev_plat_state) {
+ case SEV_STATE_UNINIT:
+ return 0;
+ case SEV_STATE_WORKING:
+ /* Legacy guests are running, the update cannot proceed. */
+ return -EBUSY;
+ case SEV_STATE_INIT:
+ break;
+ default:
+ dev_err(sev->dev, "Unknown SEV firmware state %d\n", sev_plat_state);
+ return -EINVAL;
+ }
+
+ rc = __sev_platform_shutdown_locked(&error);
+ if (rc) {
+ dev_err(sev->dev, "SEV platform shutdown failed %d, error %#x\n",
+ rc, error);
+ return rc;
+ }
+
+ __sev_release_firmware_buffers(false);
+
+ sev->fwl_reinit_required = true;
+
+ return 0;
+}
+
+static void sev_fw_upload_reinit_platform(struct sev_device *sev)
+{
+ int rc, error = SEV_RET_NO_FW_CALL;
+
+ lockdep_assert_held(&sev_cmd_mutex);
+
+ if (!sev->fwl_reinit_required)
+ return;
+
+ rc = __sev_platform_init_locked(&error);
+ if (rc) {
+ dev_err(sev->dev, "SEV platform re-init failed %d, error %#x\n",
+ rc, error);
+ return;
+ }
+
+ sev->fwl_reinit_required = false;
+}
+
+static enum fw_upload_err sev_fw_upload_update(struct sev_device *sev,
+ const u8 *data, u32 size,
+ u32 *written)
+{
+ int rc, error = SEV_RET_NO_FW_CALL;
+ enum fw_upload_err ret;
+
+ guard(mutex)(&sev_cmd_mutex);
+
+ /*
+ * A PSP declared dead only executes DOWNLOAD_FIRMWARE_EX if it was the
+ * firmware update that killed it and asked for a rollback. Declared
+ * dead for any other reason it will not answer until the platform is
+ * rebooted.
+ */
+ if (psp_dead && !sev->fwl_rollback_required) {
+ dev_err(sev->dev, "DLFW_EX: PSP is not responding\n");
+ return FW_UPLOAD_ERR_HW_ERROR;
+ }
+
+ /*
+ * If the last firmware update returned RESTORE_REQUIRED, retry DLFW_EX.
+ * We being in this state means that the legacy firmware has previously
+ * been shut down, so no need to do it again.
+ */
+ if (sev->fwl_rollback_required) {
+ psp_dead = false;
+ } else {
+ rc = sev_fw_upload_shutdown_platform(sev);
+ if (rc) {
+ return rc == -EBUSY ? FW_UPLOAD_ERR_BUSY
+ : FW_UPLOAD_ERR_HW_ERROR;
+ }
+ }
+
+ rc = sev_download_firmware_ex(data, size, &error);
+ ret = sev_fw_upload_handle_err(sev, rc, error);
+ if (ret == FW_UPLOAD_ERR_NONE) {
+ *written = size;
+ sev->fwl_rollback_required = false;
+ }
+
+ /* A rollback retry failed. PSP now stays dead. */
+ if (sev->fwl_rollback_required)
+ psp_dead = true;
+
+ if (!sev->fwl_rollback_required && !psp_dead)
+ sev_fw_upload_reinit_platform(sev);
+
+ return ret;
+}
+
static enum fw_upload_err sev_fw_upload_write(struct fw_upload *fw_upload,
const u8 *data, u32 offset,
u32 size, u32 *written)
{
- return FW_UPLOAD_ERR_BUSY;
+ struct sev_device *sev = fw_upload->dd_handle;
+ u8 old_major, old_minor, old_build;
+ enum fw_upload_err ret;
+
+ old_major = sev->api_major;
+ old_minor = sev->api_minor;
+ old_build = sev->build;
+
+ ret = sev_fw_upload_update(sev, data, size, written);
+ if (ret != FW_UPLOAD_ERR_NONE)
+ return ret;
+
+ if (sev_get_api_version()) {
+ dev_err(sev->dev, "SNP platform data refresh after firmware update failed\n");
+ return FW_UPLOAD_ERR_HW_ERROR;
+ }
+
+ if (sev->api_major != old_major || sev->api_minor != old_minor ||
+ sev->build != old_build) {
+ dev_info(sev->dev, "SEV firmware updated to %d.%d build %d\n",
+ sev->api_major, sev->api_minor, sev->build);
+ } else {
+ dev_info(sev->dev, "SEV firmware version unchanged: %d.%d build %d\n",
+ sev->api_major, sev->api_minor, sev->build);
+ }
+
+ return ret;
}
static enum fw_upload_err sev_fw_upload_poll_complete(struct fw_upload *fw_upload)
@@ -2274,6 +2533,13 @@ static void register_sev_fw_uploader(struct sev_device *sev)
{
struct fw_upload *fwl;
+ /*
+ * The interface only drives DOWNLOAD_FIRMWARE_EX, which is an SNP
+ * command, so do not advertise it where SNP is unavailable.
+ */
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+ return;
+
/*
* SNP firmware update is a platform-wide operation; only the master
* PSP issues firmware commands. Register a single global interface.
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index 7ec692e2147e..1e45a08c41da 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -71,6 +71,8 @@ struct sev_device {
struct sev_tio_status *tio_status;
struct fw_upload *fwl;
+ bool fwl_rollback_required;
+ bool fwl_reinit_required;
};
int sev_dev_init(struct psp_device *psp);
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 03a79786df1d..c6adbd46f05a 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -856,6 +856,25 @@ struct sev_platform_init_args {
unsigned int max_snp_asid;
};
+/**
+ * struct sev_data_download_firmware_ex - SNP_DOWNLOAD_FIRMWARE_EX structure
+ *
+ * @len: length of the command buffer read by the PSP
+ * @rsvd0: reserved
+ * @fw_paddr: system physical address of the start of the firmware blob
+ * @fw_len: length of the firmware blob
+ * @commit: whether to immediately commit the firmware update
+ * @rsvd1: reserved
+ */
+struct sev_data_download_firmware_ex {
+ u32 len; /* In */
+ u32 rsvd0;
+ u64 fw_paddr; /* In */
+ u32 fw_len; /* In */
+ u32 commit:1; /* In */
+ u32 rsvd1:31;
+} __packed;
+
/**
* struct sev_data_snp_commit - SNP_COMMIT structure
*
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread