Re: AMD PMF: CCP PSP fails to reinitialize after hibernation causing TEE errors

[Mario Limonciello <superm1@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2954 bytes --]



On 10/26/25 11:49 AM, Shyam Sundar S K wrote:
> + Mario
> 
> On 10/26/2025 22:16, Lars Francke wrote:
>> Hi,
>>
>> thank you Mario for the super quick response.
>> It arrived so fast I missed it.
>>
>> On Sun, Oct 26, 2025 at 3:17 PM Shyam Sundar S K
>> <Shyam-sundar.S-k@amd.com> wrote:
>>>> After resuming from hibernation I get this log line once a second:
>>>>    amd-pmf AMDI0105:00: TEE enact cmd failed. err: ffff000e, ret:0
>>>
>>> Can you please share full dmesg log? Perhaps with both ccp and amd_tee
>>> drivers enabled with dynamic debug.
>>
>> I have (with AI help, but reviewed manually) created a shell script to
>> collect the necessary information. I have attached the full dmesg file
>> to this mail but am not sure if that is proper etiquette here?
>>
>> I put up the script & more debug information in a dedicated
>> repository: https://github.com/lfrancke/amd-pmf-hibernate/tree/main
>>
>>>> Right after hibernation and before the first of those errors I get:
>>>>    ccp 0000:c3:00.2: tee: command 0x5 timed out, disabling PSP
>>>>

This comes from tee_wait_cmd_completion().  Looking at the 
suspend/resume handling code from ccp.ko, I notice that we don't 
actually have any special handling for the TEE ring for S4.  Normally 
that ring is initialized at bootup (by sending PSP_CMD_TEE_RING_INIT to 
PSP).

During the S4 resume flow I don't expect that this has happened and is 
needed explicitly in the restore() call.

Can you try this attached patch which inserts that flow to see if it 
helps?

Note mostly to myself: I need to double check flows but I think restore 
also needs a pm_hibernate_is_recovering() check in this case too if this 
works.

>>>> On boot it looks good:
>>>>    ccp 0000:c3:00.2: psp enabled
>>>
>>> Do you see this message after hibernation exit?
>>
>> Yes. On a fresh boot it works without issues but after the first
>> hibernate I get these.
>>
>>
>>>> Commit 11e298f3548a6fe5e6ad78f811abfba15e6ebbc1 from 2024 has
>>>> more or less exactly my error message but it doesn't seem
>>>> to be fixed for this case.
>>>> https://lore.kernel.org/all/20240216064112.962582-2-Shyam-sundar.S-k@amd.com/
>>>
>>> This was a case fixed for S2Idle cases, but I seem to understand that
>>> you are attempting hibernate here, right?
>>
>> Correct. I use hibernate.
>>
>> The issue I'm really trying to debug is a separate one (I'll report
>> separately): Hibernate doesn't reliably turn off the laptop. Image is
>> being written successfully but I have to press the power button for
>> 4s.

Potential theory on root cause is that it's the same regression reported 
here [1].

If so the root cause is [2], and the fix is [3].

Link:
https://lore.kernel.org/linux-pm/a2563b83-e5d7-4d02-a317-54d51e718d32@kernel.org/T/#m19966ea259837e4b02c6d0a55d67acfe15fc64ae 
[1]
Link: https://git.kernel.org/torvalds/c/469d80a3712c6  [2]
Link: 
https://lore.kernel.org/linux-pm/20251026033115.436448-1-superm1@kernel.org/ 
[3]

[-- Attachment #2: 0001-crypto-ccp-Add-an-S4-restore-flow.patch --]
[-- Type: text/x-patch, Size: 3811 bytes --]

From 1a44b96b4b93d6ec00856dd335302e0a2dccbd96 Mon Sep 17 00:00:00 2001
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
Date: Sun, 26 Oct 2025 12:34:48 -0500
Subject: [PATCH] crypto: ccp - Add an S4 restore flow

The system will have lost power during S4.  The ring used for TEE
communications needs to be initialized before use.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/crypto/ccp/sp-dev.c  | 13 +++++++++++++
 drivers/crypto/ccp/sp-dev.h  |  1 +
 drivers/crypto/ccp/sp-pci.c  | 16 ++++++++++++++++
 drivers/crypto/ccp/tee-dev.c |  8 ++++++++
 drivers/crypto/ccp/tee-dev.h |  1 +
 5 files changed, 39 insertions(+)

diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c
index 3467f6db4f505..e3fa94d14026b 100644
--- a/drivers/crypto/ccp/sp-dev.c
+++ b/drivers/crypto/ccp/sp-dev.c
@@ -21,6 +21,7 @@
 
 #include "sev-dev.h"
 #include "ccp-dev.h"
+#include "tee-dev.h"
 #include "sp-dev.h"
 
 MODULE_AUTHOR("Tom Lendacky <thomas.lendacky@amd.com>");
@@ -230,6 +231,18 @@ int sp_resume(struct sp_device *sp)
 	return 0;
 }
 
+int sp_restore(struct sp_device *sp)
+{
+	if (sp->dev_vdata->psp_vdata->tee) {
+		int r = tee_restore(sp->psp_data);
+
+		if (r)
+			return r;
+	}
+
+	return sp_resume(sp);
+}
+
 struct sp_device *sp_get_psp_master_device(void)
 {
 	struct sp_device *i, *ret = NULL;
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index 6f9d7063257d7..37b38afaab147 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -141,6 +141,7 @@ void sp_destroy(struct sp_device *sp);
 
 int sp_suspend(struct sp_device *sp);
 int sp_resume(struct sp_device *sp);
+int sp_restore(struct sp_device *sp);
 int sp_request_ccp_irq(struct sp_device *sp, irq_handler_t handler,
 		       const char *name, void *data);
 void sp_free_ccp_irq(struct sp_device *sp, void *data);
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index 8891ceee1d7d0..47406a6f150bf 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -353,6 +353,13 @@ static int __maybe_unused sp_pci_resume(struct device *dev)
 	return sp_resume(sp);
 }
 
+static int __maybe_unused sp_pci_restore(struct device *dev)
+{
+	struct sp_device *sp = dev_get_drvdata(dev);
+
+	return sp_restore(sp);
+}
+
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 static const struct sev_vdata sevv1 = {
 	.cmdresp_reg		= 0x10580,	/* C2PMSG_32 */
@@ -565,6 +572,15 @@ MODULE_DEVICE_TABLE(pci, sp_pci_table);
 
 static SIMPLE_DEV_PM_OPS(sp_pci_pm_ops, sp_pci_suspend, sp_pci_resume);
 
+const struct dev_pm_ops i2c_hid_core_pm = {
+	.suspend = pm_sleep_ptr(sp_pci_suspend),
+	.resume = pm_sleep_ptr(sp_pci_resume),
+	.freeze = pm_sleep_ptr(sp_pci_suspend),
+	.thaw = pm_sleep_ptr(sp_pci_resume),
+	.poweroff = pm_sleep_ptr(sp_pci_suspend),
+	.restore = pm_sleep_ptr(sp_pci_restore),
+};
+
 static struct pci_driver sp_pci_driver = {
 	.name = "ccp",
 	.id_table = sp_pci_table,
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5e1d80724678d..a7470477117d6 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -365,3 +365,11 @@ int psp_check_tee_status(void)
 	return 0;
 }
 EXPORT_SYMBOL(psp_check_tee_status);
+
+int tee_restore(struct psp_device *psp)
+{
+	if (!psp || !psp->tee_data)
+		return -ENODEV;
+
+	return tee_init_ring(psp->tee_data);
+}
\ No newline at end of file
diff --git a/drivers/crypto/ccp/tee-dev.h b/drivers/crypto/ccp/tee-dev.h
index ea9a2b7c05f57..c23416cb7bb37 100644
--- a/drivers/crypto/ccp/tee-dev.h
+++ b/drivers/crypto/ccp/tee-dev.h
@@ -111,5 +111,6 @@ struct tee_ring_cmd {
 
 int tee_dev_init(struct psp_device *psp);
 void tee_dev_destroy(struct psp_device *psp);
+int tee_restore(struct psp_device *psp);
 
 #endif /* __TEE_DEV_H__ */
-- 
2.43.0