Re: [PATCH kernel v3 4/4] crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)
From: Alexey Kardashevskiy
Date: Tue Dec 02 2025 - 17:30:46 EST
On 3/12/25 01:52, Tom Lendacky wrote:
On 12/1/25 20:44, Alexey Kardashevskiy wrote:
Implement the SEV-TIO (Trusted I/O) firmware interface for PCIe TDISP
(Trust Domain In-Socket Protocol). This enables secure communication
between trusted domains and PCIe devices through the PSP (Platform
Security Processor).
The implementation includes:
- Device Security Manager (DSM) operations for establishing secure links
- SPDM (Security Protocol and Data Model) over DOE (Data Object Exchange)
- IDE (Integrity Data Encryption) stream management for secure PCIe
This module bridges the SEV firmware stack with the generic PCIe TSM
framework.
This is phase1 as described in Documentation/driver-api/pci/tsm.rst.
On AMD SEV, the AMD PSP firmware acts as TSM (manages the security/trust).
The CCP driver provides the interface to it and registers in the TSM
subsystem.
Detect the PSP support (reported via FEATURE_INFO + SNP_PLATFORM_STATUS)
and enable SEV-TIO in the SNP_INIT_EX call if the hardware supports TIO.
Implement SEV TIO PSP command wrappers in sev-dev-tio.c and store
the data in the SEV-TIO-specific structs.
Implement TSM hooks and IDE setup in sev-dev-tsm.c.
Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
Just some minor comments below. After those are addressed:
For the ccp related changes in the whole series:
Acked-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
Dan did it right (thanks Dan!).
---
Changes:
v2:
* moved declarations from sev-dev-tio.h to sev-dev.h
* removed include "sev-dev-tio.h" from sev-dev.c to fight errors when TSM is disabled
* converted /** to /* as these are part of any external API and trigger unwanted kerneldoc warnings
* got rid of ifdefs
* "select PCI_TSM" moved under CRYPTO_DEV_SP_PSP
* open coded SNP_SEV_TIO_SUPPORTED
* renamed tio_present to tio_supp to match the flag name
* merged "crypto: ccp: Enable SEV-TIO feature in the PSP when supported" to this one
---
drivers/crypto/ccp/Kconfig | 1 +
drivers/crypto/ccp/Makefile | 4 +
drivers/crypto/ccp/sev-dev-tio.h | 123 +++
drivers/crypto/ccp/sev-dev.h | 9 +
include/linux/psp-sev.h | 11 +-
drivers/crypto/ccp/sev-dev-tio.c | 864 ++++++++++++++++++++
drivers/crypto/ccp/sev-dev-tsm.c | 405 +++++++++
drivers/crypto/ccp/sev-dev.c | 51 +-
8 files changed, 1465 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 9e0c16b36f9c..d6095d1467b3 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -75,6 +75,10 @@ static bool psp_init_on_probe = true;
module_param(psp_init_on_probe, bool, 0444);
MODULE_PARM_DESC(psp_init_on_probe, " if true, the PSP will be initialized on module init. Else the PSP will be initialized on the first command requiring it");
+static bool sev_tio_enabled = IS_ENABLED(CONFIG_PCI_TSM);
+module_param_named(tio, sev_tio_enabled, bool, 0444);
+MODULE_PARM_DESC(tio, "Enables TIO in SNP_INIT_EX");
Hmmm... I thought you said you wanted to hide the module parameter if
CONFIG_PCI_TSM isn't enabled. Either way, it's fine.
I did but you did not and I do not care that much :)
+
MODULE_FIRMWARE("amd/amd_sev_fam17h_model0xh.sbin"); /* 1st gen EPYC */
MODULE_FIRMWARE("amd/amd_sev_fam17h_model3xh.sbin"); /* 2nd gen EPYC */
MODULE_FIRMWARE("amd/amd_sev_fam19h_model0xh.sbin"); /* 3rd gen EPYC */
@@ -251,7 +255,7 @@ static int sev_cmd_buffer_len(int cmd)
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);
- default: return 0;
+ default: return sev_tio_cmd_buffer_len(cmd);
}
return 0;
@@ -1434,6 +1438,19 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
data.init_rmp = 1;
data.list_paddr_en = 1;
data.list_paddr = __psp_pa(snp_range_list);
+
+ bool tio_supp = !!(sev->snp_feat_info_0.ebx & SNP_SEV_TIO_SUPPORTED);
Please put the variable definition at the top of the "if" block instead
of in the middle of the code.
+
+ data.tio_en = tio_supp && sev_tio_enabled && amd_iommu_sev_tio_supported();
Don't you still want to take CONFIG_PCI_TSM into account?
data.tio_en = IS_ENABLED(CONFIG_PCI_TSM) && tio_supp && sev_tio_enabled && amd_iommu_sev_tio_supported();
or
if (IS_ENABLED(CONFIG_PCI_TSM)
data.tio_en = tio_supp && sev_tio_enabled && amd_iommu_sev_tio_supported();
But if you change back to #ifdef the module parameter, then you won't
need the IS_ENABLED() check here because sev_tio_enabled will be set
based on CONFIG_PCI_TSM and will be false and not changeable if
CONFIG_PCI_TSM is not y.
Ah true. I thought sev_tio_enabled=IS_ENABLED(CONFIG_PCI_TSM) does it but missed that sev_tio_enabled is exported as a parameter so not a constant at compile time.
+
+ /*
+ * When psp_init_on_probe is disabled, the userspace calling
+ * SEV ioctl can inadvertently shut down SNP and SEV-TIO causing
+ * unexpected state loss.
+ */
After this is merged, lets see if sev_move_to_init_state() can be
cleaned up to avoid this situation.
Do we want to keep psp_init_on_probe, why? Thanks,
Thanks,
Tom
+ if (data.tio_en && !psp_init_on_probe)
+ dev_warn(sev->dev, "SEV-TIO as incompatible with psp_init_on_probe=0\n");
+
cmd = SEV_CMD_SNP_INIT_EX;
} else {
cmd = SEV_CMD_SNP_INIT;
@@ -1471,7 +1488,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
snp_hv_fixed_pages_state_update(sev, HV_FIXED);
sev->snp_initialized = true;
- dev_dbg(sev->dev, "SEV-SNP firmware initialized\n");
+ dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
+ data.tio_en ? "enabled" : "disabled");
dev_info(sev->dev, "SEV-SNP API:%d.%d build:%d\n", sev->api_major,
sev->api_minor, sev->build);
@@ -1479,6 +1497,23 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
atomic_notifier_chain_register(&panic_notifier_list,
&snp_panic_notifier);
+ if (data.tio_en) {
+ /*
+ * This executes with the sev_cmd_mutex held so down the stack
+ * snp_reclaim_pages(locked=false) might be needed (which is extremely
+ * unlikely) but will cause a deadlock.
+ * Instead of exporting __snp_alloc_firmware_pages(), allocate a page
+ * for this one call here.
+ */
+ void *tio_status = page_address(__snp_alloc_firmware_pages(
+ GFP_KERNEL_ACCOUNT | __GFP_ZERO, 0, true));
+
+ if (tio_status) {
+ sev_tsm_init_locked(sev, tio_status);
+ __snp_free_firmware_pages(virt_to_page(tio_status), 0, true);
+ }
+ }
+
sev_es_tmr_size = SNP_TMR_SIZE;
return 0;
@@ -2758,8 +2793,20 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic)
static void sev_firmware_shutdown(struct sev_device *sev)
{
+ /*
+ * Calling without sev_cmd_mutex held as TSM will likely try disconnecting
+ * IDE and this ends up calling sev_do_cmd() which locks sev_cmd_mutex.
+ */
+ if (sev->tio_status)
+ sev_tsm_uninit(sev);
+
mutex_lock(&sev_cmd_mutex);
+
__sev_firmware_shutdown(sev, false);
+
+ kfree(sev->tio_status);
+ sev->tio_status = NULL;
+
mutex_unlock(&sev_cmd_mutex);
}
--
Alexey