[PATCH v3 0/5] Fixes for PMF and CCP drivers after S4

[PATCH v3 0/5] Fixes for PMF and CCP drivers after S4

[Mario Limonciello (AMD)]

Lars Francke reported that the PMF driver fails to work afer S4 with:
  ccp 0000:c3:00.2: tee: command 0x5 timed out, disabling PSP

This is because there is a TA loaded to the TEE environment that
is lost during S4.  The TEE rings need to be reinitialized and the
TA needs to be reloaded.

This series adds those flows to the PMF and CCP drivers.

Mario Limonciello (AMD) (4):
  crypto: ccp - Declare PSP dead if PSP_CMD_TEE_RING_INIT fails
  crypto: ccp - Add an S4 restore flow
  crypto: ccp - Factor out ring destroy handling to a helper
  crypto: ccp - Send PSP_CMD_TEE_RING_DESTROY when PSP_CMD_TEE_RING_INIT
    fails

Shyam Sundar S K (1):
  platform/x86/amd/pmf: Prevent TEE errors after hibernate

 drivers/crypto/ccp/psp-dev.c          | 15 +++++++
 drivers/crypto/ccp/sp-dev.h           |  2 +
 drivers/crypto/ccp/sp-pci.c           | 24 ++++++++++-
 drivers/crypto/ccp/tee-dev.c          | 54 +++++++++++++++++------
 drivers/crypto/ccp/tee-dev.h          |  1 +
 drivers/platform/x86/amd/pmf/core.c   | 62 ++++++++++++++++++++++++++-
 drivers/platform/x86/amd/pmf/pmf.h    | 10 +++++
 drivers/platform/x86/amd/pmf/tee-if.c | 12 ++----
 include/linux/psp.h                   |  2 +
 9 files changed, 159 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH v3 1/5] platform/x86/amd/pmf: Prevent TEE errors after hibernate

[Mario Limonciello (AMD)]

From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

After resuming from hibernate, TEE commands can time out and cause PSP
disables. Fix this by reinitializing the Trusted Application (TA) and
cancelling the pb workqueue in the hibernate callbacks to avoid these
errors.

ccp 0000:c4:00.2: tee: command 0x5 timed out, disabling PSP
amd-pmf AMDI0107:00: TEE enact cmd failed. err: ffff000e, ret:0
amd-pmf AMDI0107:00: TEE enact cmd failed. err: ffff000e, ret:0
amd-pmf AMDI0107:00: TEE enact cmd failed. err: ffff000e, ret:0

Fixes: ae82cef7d9c5 ("platform/x86/amd/pmf: Add support for PMF-TA interaction")
Reported-by: Lars Francke <lars.francke@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
[ML: Add more tags]
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/platform/x86/amd/pmf/core.c   | 62 ++++++++++++++++++++++++++-
 drivers/platform/x86/amd/pmf/pmf.h    | 10 +++++
 drivers/platform/x86/amd/pmf/tee-if.c | 12 ++----
 3 files changed, 74 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 8fc293c9c5380..15c27edfb6d85 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -314,6 +314,61 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
 	return 0;
 }
 
+static int amd_pmf_reinit_ta(struct amd_pmf_dev *pdev)
+{
+	bool status;
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(amd_pmf_ta_uuid); i++) {
+		ret = amd_pmf_tee_init(pdev, &amd_pmf_ta_uuid[i]);
+		if (ret) {
+			dev_err(pdev->dev, "TEE init failed for UUID[%d] ret: %d\n", i, ret);
+			return ret;
+		}
+
+		ret = amd_pmf_start_policy_engine(pdev);
+		dev_dbg(pdev->dev, "start policy engine ret: %d (UUID idx: %d)\n", ret, i);
+		status = ret == TA_PMF_TYPE_SUCCESS;
+		if (status)
+			break;
+		amd_pmf_tee_deinit(pdev);
+	}
+
+	return 0;
+}
+
+static int amd_pmf_restore_handler(struct device *dev)
+{
+	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+	int ret;
+
+	if (pdev->buf) {
+		ret = amd_pmf_set_dram_addr(pdev, false);
+		if (ret)
+			return ret;
+	}
+
+	if (pdev->smart_pc_enabled)
+		amd_pmf_reinit_ta(pdev);
+
+	return 0;
+}
+
+static int amd_pmf_freeze_handler(struct device *dev)
+{
+	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+
+	if (!pdev->smart_pc_enabled)
+		return 0;
+
+	cancel_delayed_work_sync(&pdev->pb_work);
+	/* Clear all TEE resources */
+	amd_pmf_tee_deinit(pdev);
+	pdev->session_id = 0;
+
+	return 0;
+}
+
 static int amd_pmf_suspend_handler(struct device *dev)
 {
 	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
@@ -347,7 +402,12 @@ static int amd_pmf_resume_handler(struct device *dev)
 	return 0;
 }
 
-static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, amd_pmf_suspend_handler, amd_pmf_resume_handler);
+static const struct dev_pm_ops amd_pmf_pm = {
+	.suspend = amd_pmf_suspend_handler,
+	.resume = amd_pmf_resume_handler,
+	.freeze = amd_pmf_freeze_handler,
+	.restore = amd_pmf_restore_handler,
+};
 
 static void amd_pmf_init_features(struct amd_pmf_dev *dev)
 {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 9144c8c3bbaf2..513a6309ce130 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -129,6 +129,12 @@ struct cookie_header {
 
 typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
 
+static const uuid_t amd_pmf_ta_uuid[] __used = { UUID_INIT(0xd9b39bf2, 0x66bd, 0x4154, 0xaf, 0xb8,
+							   0x8a, 0xcc, 0x2b, 0x2b, 0x60, 0xd6),
+						 UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d, 0xb1, 0x2d,
+							   0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43),
+					       };
+
 /* APTS PMF BIOS Interface */
 struct amd_pmf_apts_output {
 	u16 table_version;
@@ -895,4 +901,8 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
 void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
 int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev);
 
+int amd_pmf_tee_init(struct amd_pmf_dev *dev, const uuid_t *uuid);
+void amd_pmf_tee_deinit(struct amd_pmf_dev *dev);
+int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev);
+
 #endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 0abce76f89ffe..95ceb906a5f39 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -27,12 +27,6 @@ module_param(pb_side_load, bool, 0444);
 MODULE_PARM_DESC(pb_side_load, "Sideload policy binaries debug policy failures");
 #endif
 
-static const uuid_t amd_pmf_ta_uuid[] = { UUID_INIT(0xd9b39bf2, 0x66bd, 0x4154, 0xaf, 0xb8, 0x8a,
-						    0xcc, 0x2b, 0x2b, 0x60, 0xd6),
-					  UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d, 0xb1, 0x2d, 0xc5,
-						    0x29, 0xb1, 0x3d, 0x85, 0x43),
-					};
-
 static const char *amd_pmf_uevent_as_str(unsigned int state)
 {
 	switch (state) {
@@ -324,7 +318,7 @@ static void amd_pmf_invoke_cmd(struct work_struct *work)
 	schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
 }
 
-static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
+int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
 {
 	struct cookie_header *header;
 	int res;
@@ -480,7 +474,7 @@ static int amd_pmf_register_input_device(struct amd_pmf_dev *dev)
 	return 0;
 }
 
-static int amd_pmf_tee_init(struct amd_pmf_dev *dev, const uuid_t *uuid)
+int amd_pmf_tee_init(struct amd_pmf_dev *dev, const uuid_t *uuid)
 {
 	u32 size;
 	int ret;
@@ -528,7 +522,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev *dev, const uuid_t *uuid)
 	return ret;
 }
 
-static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
+void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
 {
 	if (!dev->tee_ctx)
 		return;
-- 
2.43.0

[PATCH v3 2/5] crypto: ccp - Declare PSP dead if PSP_CMD_TEE_RING_INIT fails

[Mario Limonciello (AMD)]

tee_init_ring() only declares PSP dead if the command times out.
If there is any other failure it is still considered fatal though.
Set psp_dead for other failures as well.

Fixes: 949a0c8dd3c2 ("crypto: ccp - Move direct access to some PSP registers out of TEE")
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/crypto/ccp/tee-dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5e1d80724678d..af881daa5855b 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -125,6 +125,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
 		dev_err(tee->dev, "tee: ring init command failed (%#010lx)\n",
 			FIELD_GET(PSP_CMDRESP_STS, reg));
 		tee_free_ring(tee);
+		psp_dead = true;
 		ret = -EIO;
 	}
 
-- 
2.43.0

[PATCH v3 3/5] crypto: ccp - Add an S4 restore flow

[Mario Limonciello (AMD)]

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

Fixes: f892a21f51162 ("crypto: ccp - use generic power management")
Reported-by: Lars Francke <lars.francke@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v3:
 * Adjust to Tom's suggested flow
---
 drivers/crypto/ccp/psp-dev.c | 15 +++++++++++++++
 drivers/crypto/ccp/sp-dev.h  |  2 ++
 drivers/crypto/ccp/sp-pci.c  | 24 +++++++++++++++++++++++-
 drivers/crypto/ccp/tee-dev.c |  5 +++++
 drivers/crypto/ccp/tee-dev.h |  1 +
 5 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 9e21da0e298ad..e1da4f1a5db99 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -351,6 +351,21 @@ struct psp_device *psp_get_master_device(void)
 	return sp ? sp->psp_data : NULL;
 }
 
+
+int psp_restore(struct sp_device *sp)
+{
+	struct psp_device *psp = sp->psp_data;
+
+	if (psp->tee_data) {
+		int r = tee_restore(psp);
+
+		if (r)
+			return r;
+	}
+
+	return 0;
+}
+
 void psp_pci_init(void)
 {
 	psp_master = psp_get_master_device();
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index 1335a83fe052e..0deea1a399e47 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -174,6 +174,7 @@ int psp_dev_init(struct sp_device *sp);
 void psp_pci_init(void);
 void psp_dev_destroy(struct sp_device *sp);
 void psp_pci_exit(void);
+int psp_restore(struct sp_device *sp);
 
 #else /* !CONFIG_CRYPTO_DEV_SP_PSP */
 
@@ -181,6 +182,7 @@ static inline int psp_dev_init(struct sp_device *sp) { return 0; }
 static inline void psp_pci_init(void) { }
 static inline void psp_dev_destroy(struct sp_device *sp) { }
 static inline void psp_pci_exit(void) { }
+static inline int psp_restore(struct sp_device *sp) { return 0; }
 
 #endif /* CONFIG_CRYPTO_DEV_SP_PSP */
 
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index 8891ceee1d7d0..931ec6bf2cec6 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -353,6 +353,21 @@ 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);
+
+#ifdef CONFIG_CRYPTO_DEV_SP_PSP
+	if (sp->psp_data) {
+		int ret = psp_restore(sp);
+
+		if (ret)
+			return ret;
+	}
+#endif
+	return sp_resume(sp);
+}
+
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 static const struct sev_vdata sevv1 = {
 	.cmdresp_reg		= 0x10580,	/* C2PMSG_32 */
@@ -563,7 +578,14 @@ static const struct pci_device_id sp_pci_table[] = {
 };
 MODULE_DEVICE_TABLE(pci, sp_pci_table);
 
-static SIMPLE_DEV_PM_OPS(sp_pci_pm_ops, sp_pci_suspend, sp_pci_resume);
+static const struct dev_pm_ops sp_pci_pm_ops = {
+	.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_early = pm_sleep_ptr(sp_pci_restore),
+};
 
 static struct pci_driver sp_pci_driver = {
 	.name = "ccp",
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index af881daa5855b..11c4b05e2f3a2 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -366,3 +366,8 @@ int psp_check_tee_status(void)
 	return 0;
 }
 EXPORT_SYMBOL(psp_check_tee_status);
+
+int tee_restore(struct psp_device *psp)
+{
+	return tee_init_ring(psp->tee_data);
+}
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

[PATCH v3 4/5] crypto: ccp - Factor out ring destroy handling to a helper

[Mario Limonciello (AMD)]

The ring destroy command needs to be used in multiple places. Split
out the code to a helper.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/crypto/ccp/tee-dev.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 11c4b05e2f3a2..ef1430f86ad62 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -86,6 +86,29 @@ static inline void tee_free_cmd_buffer(struct tee_init_ring_cmd *cmd)
 	kfree(cmd);
 }
 
+static bool tee_send_destroy_cmd(struct psp_tee_device *tee)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = psp_mailbox_command(tee->psp, PSP_CMD_TEE_RING_DESTROY, NULL,
+				  TEE_DEFAULT_CMD_TIMEOUT, &reg);
+	if (ret) {
+		dev_err(tee->dev, "tee: ring destroy command timed out, disabling TEE support\n");
+		psp_dead = true;
+		return false;
+	}
+
+	if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
+		dev_err(tee->dev, "tee: ring destroy command failed (%#010lx)\n",
+			FIELD_GET(PSP_CMDRESP_STS, reg));
+		psp_dead = true;
+		return false;
+	}
+
+	return true;
+}
+
 static int tee_init_ring(struct psp_tee_device *tee)
 {
 	int ring_size = MAX_RING_BUFFER_ENTRIES * sizeof(struct tee_ring_cmd);
@@ -137,24 +160,13 @@ static int tee_init_ring(struct psp_tee_device *tee)
 
 static void tee_destroy_ring(struct psp_tee_device *tee)
 {
-	unsigned int reg;
-	int ret;
-
 	if (!tee->rb_mgr.ring_start)
 		return;
 
 	if (psp_dead)
 		goto free_ring;
 
-	ret = psp_mailbox_command(tee->psp, PSP_CMD_TEE_RING_DESTROY, NULL,
-				  TEE_DEFAULT_CMD_TIMEOUT, &reg);
-	if (ret) {
-		dev_err(tee->dev, "tee: ring destroy command timed out, disabling TEE support\n");
-		psp_dead = true;
-	} else if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
-		dev_err(tee->dev, "tee: ring destroy command failed (%#010lx)\n",
-			FIELD_GET(PSP_CMDRESP_STS, reg));
-	}
+	tee_send_destroy_cmd(tee);
 
 free_ring:
 	tee_free_ring(tee);
-- 
2.43.0

[PATCH v3 5/5] crypto: ccp - Send PSP_CMD_TEE_RING_DESTROY when PSP_CMD_TEE_RING_INIT fails

[Mario Limonciello (AMD)]

The hibernate resume sequence involves loading a resume kernel that is just
used for loading the hibernate image before shifting back to the existing
kernel.

During that hibernate resume sequence the resume kernel may have loaded
the ccp driver.  If this happens the resume kernel will also have called
PSP_CMD_TEE_RING_INIT but it will never have called
PSP_CMD_TEE_RING_DESTROY.

This is problematic because the existing kernel needs to re-initialize the
ring.  One could argue that the existing kernel should call destroy
as part of restore() but there is no guarantee that the resume kernel did
or didn't load the ccp driver.  There is also no callback opportunity for
the resume kernel to destroy before handing back control to the existing
kernel.

Similar problems could potentially exist with the use of kdump and
crash handling. I actually reproduced this issue like this:

1) rmmod ccp
2) hibernate the system
3) resume the system
4) modprobe ccp

The resume kernel will have loaded ccp but never destroyed and then when
I try to modprobe it fails.

Because of these possible cases add a flow that checks the error code from
the PSP_CMD_TEE_RING_INIT call and tries to call PSP_CMD_TEE_RING_DESTROY
if it failed.  If this succeeds then call PSP_CMD_TEE_RING_INIT again.

Fixes: f892a21f51162 ("crypto: ccp - use generic power management")
Reported-by: Lars Francke <lars.francke@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v3:
 * Add a comment (Tom)
 * Add a define for busy condition (Shyam)
 * Rename label (Shyam)
 * Upgrade message to info (Shyam)
 * Use a helper that validates result for destroy command (Shyam)
---
 drivers/crypto/ccp/tee-dev.c | 12 ++++++++++++
 include/linux/psp.h          |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index ef1430f86ad62..9edb220abbc1a 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -113,6 +113,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
 {
 	int ring_size = MAX_RING_BUFFER_ENTRIES * sizeof(struct tee_ring_cmd);
 	struct tee_init_ring_cmd *cmd;
+	bool retry = false;
 	unsigned int reg;
 	int ret;
 
@@ -135,6 +136,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
 	/* Send command buffer details to Trusted OS by writing to
 	 * CPU-PSP message registers
 	 */
+retry_init:
 	ret = psp_mailbox_command(tee->psp, PSP_CMD_TEE_RING_INIT, cmd,
 				  TEE_DEFAULT_CMD_TIMEOUT, &reg);
 	if (ret) {
@@ -145,6 +147,16 @@ static int tee_init_ring(struct psp_tee_device *tee)
 	}
 
 	if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
+		/*
+		 * During the hibernate resume sequence driver may have gotten loaded
+		 * but the ring not properly destroyed. If the ring doesn't work, try
+		 * to destroy and re-init once.
+		 */
+		if (!retry && FIELD_GET(PSP_CMDRESP_STS, reg) == PSP_TEE_STATUS_RING_BUSY) {
+			dev_info(tee->dev, "tee: ring init command failed with busy status, retrying\n");
+			if (tee_send_destroy_cmd(tee))
+				goto retry_init;
+		}
 		dev_err(tee->dev, "tee: ring init command failed (%#010lx)\n",
 			FIELD_GET(PSP_CMDRESP_STS, reg));
 		tee_free_ring(tee);
diff --git a/include/linux/psp.h b/include/linux/psp.h
index 92e60aeef21e1..a329148e3684b 100644
--- a/include/linux/psp.h
+++ b/include/linux/psp.h
@@ -23,6 +23,8 @@
 #define PSP_CMDRESP_RECOVERY	BIT(30)
 #define PSP_CMDRESP_RESP	BIT(31)
 
+#define PSP_TEE_STATUS_RING_BUSY 0x0000000d  /* Ring already initialized */
+
 #define PSP_DRBL_MSG		PSP_CMDRESP_CMD
 #define PSP_DRBL_RING		BIT(0)
 
-- 
2.43.0

RE: [PATCH v3 0/5] Fixes for PMF and CCP drivers after S4

[Shen, Yijun]

> -----Original Message-----
> From: Mario Limonciello (AMD) <superm1@kernel.org>
> Sent: Monday, December 15, 2025 3:12 AM
> To: Tom Lendacky <thomas.lendacky@amd.com>; Herbert Xu
> <herbert@gondor.apana.org.au>; shyam-sundar.s-k <shyam-sundar.s-
> k@amd.com>; Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>; Rijo Thomas
> <Rijo-john.Thomas@amd.com>
> Cc: John Allen <john.allen@amd.com>; David S . Miller
> <davem@davemloft.net>; Hans de Goede <hansg@kernel.org>; open
> list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER <linux-
> crypto@vger.kernel.org>; open list:AMD PMF DRIVER <platform-driver-
> x86@vger.kernel.org>; Lars Francke <lars.francke@gmail.com>; Shen, Yijun
> <Yijun.Shen@dell.com>; Mario Limonciello (AMD) <superm1@kernel.org>
> Subject: [PATCH v3 0/5] Fixes for PMF and CCP drivers after S4
> 
> 
> [EXTERNAL EMAIL]
> 
> Lars Francke reported that the PMF driver fails to work afer S4 with:
>   ccp 0000:c3:00.2: tee: command 0x5 timed out, disabling PSP
> 
> This is because there is a TA loaded to the TEE environment that is lost during
> S4.  The TEE rings need to be reinitialized and the TA needs to be reloaded.
> 
> This series adds those flows to the PMF and CCP drivers.

Verified this patch series on the issued system.

Tested-By: Yijun Shen <Yijun.Shen@Dell.com>

> 
> Mario Limonciello (AMD) (4):
>   crypto: ccp - Declare PSP dead if PSP_CMD_TEE_RING_INIT fails
>   crypto: ccp - Add an S4 restore flow
>   crypto: ccp - Factor out ring destroy handling to a helper
>   crypto: ccp - Send PSP_CMD_TEE_RING_DESTROY when
> PSP_CMD_TEE_RING_INIT
>     fails
> 
> Shyam Sundar S K (1):
>   platform/x86/amd/pmf: Prevent TEE errors after hibernate
> 
>  drivers/crypto/ccp/psp-dev.c          | 15 +++++++
>  drivers/crypto/ccp/sp-dev.h           |  2 +
>  drivers/crypto/ccp/sp-pci.c           | 24 ++++++++++-
>  drivers/crypto/ccp/tee-dev.c          | 54 +++++++++++++++++------
>  drivers/crypto/ccp/tee-dev.h          |  1 +
>  drivers/platform/x86/amd/pmf/core.c   | 62 ++++++++++++++++++++++++++-
>  drivers/platform/x86/amd/pmf/pmf.h    | 10 +++++
>  drivers/platform/x86/amd/pmf/tee-if.c | 12 ++----
>  include/linux/psp.h                   |  2 +
>  9 files changed, 159 insertions(+), 23 deletions(-)
> 
> --
> 2.43.0

Re: [PATCH v3 5/5] crypto: ccp - Send PSP_CMD_TEE_RING_DESTROY when PSP_CMD_TEE_RING_INIT fails

[Ilpo Järvinen]

On Sun, 14 Dec 2025, Mario Limonciello (AMD) wrote:

> The hibernate resume sequence involves loading a resume kernel that is just
> used for loading the hibernate image before shifting back to the existing
> kernel.
> 
> During that hibernate resume sequence the resume kernel may have loaded
> the ccp driver.  If this happens the resume kernel will also have called
> PSP_CMD_TEE_RING_INIT but it will never have called
> PSP_CMD_TEE_RING_DESTROY.
> 
> This is problematic because the existing kernel needs to re-initialize the
> ring.  One could argue that the existing kernel should call destroy
> as part of restore() but there is no guarantee that the resume kernel did
> or didn't load the ccp driver.  There is also no callback opportunity for
> the resume kernel to destroy before handing back control to the existing
> kernel.
> 
> Similar problems could potentially exist with the use of kdump and
> crash handling. I actually reproduced this issue like this:
> 
> 1) rmmod ccp
> 2) hibernate the system
> 3) resume the system
> 4) modprobe ccp
> 
> The resume kernel will have loaded ccp but never destroyed and then when
> I try to modprobe it fails.
> 
> Because of these possible cases add a flow that checks the error code from
> the PSP_CMD_TEE_RING_INIT call and tries to call PSP_CMD_TEE_RING_DESTROY
> if it failed.  If this succeeds then call PSP_CMD_TEE_RING_INIT again.
> 
> Fixes: f892a21f51162 ("crypto: ccp - use generic power management")
> Reported-by: Lars Francke <lars.francke@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v3:
>  * Add a comment (Tom)
>  * Add a define for busy condition (Shyam)
>  * Rename label (Shyam)
>  * Upgrade message to info (Shyam)
>  * Use a helper that validates result for destroy command (Shyam)
> ---
>  drivers/crypto/ccp/tee-dev.c | 12 ++++++++++++
>  include/linux/psp.h          |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
> index ef1430f86ad62..9edb220abbc1a 100644
> --- a/drivers/crypto/ccp/tee-dev.c
> +++ b/drivers/crypto/ccp/tee-dev.c
> @@ -113,6 +113,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
>  {
>  	int ring_size = MAX_RING_BUFFER_ENTRIES * sizeof(struct tee_ring_cmd);
>  	struct tee_init_ring_cmd *cmd;
> +	bool retry = false;
>  	unsigned int reg;
>  	int ret;
>  
> @@ -135,6 +136,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
>  	/* Send command buffer details to Trusted OS by writing to
>  	 * CPU-PSP message registers
>  	 */
> +retry_init:
>  	ret = psp_mailbox_command(tee->psp, PSP_CMD_TEE_RING_INIT, cmd,
>  				  TEE_DEFAULT_CMD_TIMEOUT, &reg);
>  	if (ret) {
> @@ -145,6 +147,16 @@ static int tee_init_ring(struct psp_tee_device *tee)
>  	}
>  
>  	if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
> +		/*
> +		 * During the hibernate resume sequence driver may have gotten loaded
> +		 * but the ring not properly destroyed. If the ring doesn't work, try
> +		 * to destroy and re-init once.
> +		 */
> +		if (!retry && FIELD_GET(PSP_CMDRESP_STS, reg) == PSP_TEE_STATUS_RING_BUSY) {
> +			dev_info(tee->dev, "tee: ring init command failed with busy status, retrying\n");
> +			if (tee_send_destroy_cmd(tee))
> +				goto retry_init;
> +		}
>  		dev_err(tee->dev, "tee: ring init command failed (%#010lx)\n",
>  			FIELD_GET(PSP_CMDRESP_STS, reg));
>  		tee_free_ring(tee);
> diff --git a/include/linux/psp.h b/include/linux/psp.h
> index 92e60aeef21e1..a329148e3684b 100644
> --- a/include/linux/psp.h
> +++ b/include/linux/psp.h
> @@ -23,6 +23,8 @@
>  #define PSP_CMDRESP_RECOVERY	BIT(30)
>  #define PSP_CMDRESP_RESP	BIT(31)
>  
> +#define PSP_TEE_STATUS_RING_BUSY 0x0000000d  /* Ring already initialized */

It would be better to have this right underneath PSP_CMDRESP_STS (you 
can use one extra space to indent different from the mask and bits).

Also, there's inconsistency in STS vs STATUS in the naming.

> +
>  #define PSP_DRBL_MSG		PSP_CMDRESP_CMD
>  #define PSP_DRBL_RING		BIT(0)
>  
> 

-- 
 i.

Re: [PATCH v3 3/5] crypto: ccp - Add an S4 restore flow

[Ilpo Järvinen]

On Sun, 14 Dec 2025, Mario Limonciello (AMD) wrote:

> The system will have lost power during S4.  The ring used for TEE
> communications needs to be initialized before use.
> 
> Fixes: f892a21f51162 ("crypto: ccp - use generic power management")
> Reported-by: Lars Francke <lars.francke@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v3:
>  * Adjust to Tom's suggested flow
> ---
>  drivers/crypto/ccp/psp-dev.c | 15 +++++++++++++++
>  drivers/crypto/ccp/sp-dev.h  |  2 ++
>  drivers/crypto/ccp/sp-pci.c  | 24 +++++++++++++++++++++++-
>  drivers/crypto/ccp/tee-dev.c |  5 +++++
>  drivers/crypto/ccp/tee-dev.h |  1 +
>  5 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index 9e21da0e298ad..e1da4f1a5db99 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -351,6 +351,21 @@ struct psp_device *psp_get_master_device(void)
>  	return sp ? sp->psp_data : NULL;
>  }
>  
> +

Extra empty line.

> +int psp_restore(struct sp_device *sp)
> +{
> +	struct psp_device *psp = sp->psp_data;
> +
> +	if (psp->tee_data) {
> +		int r = tee_restore(psp);
> +
> +		if (r)

No extra lines in between call and it's error handling. I suggest you 
put int r separately at the top of the function, and not in this block.

The variables should be named ret, no (similar to what the other case in 
this file already do)?

> +			return r;
> +	}
> +
> +	return 0;
> +}
> +
>  void psp_pci_init(void)
>  {
>  	psp_master = psp_get_master_device();
> diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
> index 1335a83fe052e..0deea1a399e47 100644
> --- a/drivers/crypto/ccp/sp-dev.h
> +++ b/drivers/crypto/ccp/sp-dev.h
> @@ -174,6 +174,7 @@ int psp_dev_init(struct sp_device *sp);
>  void psp_pci_init(void);
>  void psp_dev_destroy(struct sp_device *sp);
>  void psp_pci_exit(void);
> +int psp_restore(struct sp_device *sp);
>  
>  #else /* !CONFIG_CRYPTO_DEV_SP_PSP */
>  
> @@ -181,6 +182,7 @@ static inline int psp_dev_init(struct sp_device *sp) { return 0; }
>  static inline void psp_pci_init(void) { }
>  static inline void psp_dev_destroy(struct sp_device *sp) { }
>  static inline void psp_pci_exit(void) { }
> +static inline int psp_restore(struct sp_device *sp) { return 0; }
>  
>  #endif /* CONFIG_CRYPTO_DEV_SP_PSP */
>  
> diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
> index 8891ceee1d7d0..931ec6bf2cec6 100644
> --- a/drivers/crypto/ccp/sp-pci.c
> +++ b/drivers/crypto/ccp/sp-pci.c
> @@ -353,6 +353,21 @@ 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);
> +
> +#ifdef CONFIG_CRYPTO_DEV_SP_PSP
> +	if (sp->psp_data) {
> +		int ret = psp_restore(sp);
> +
> +		if (ret)

		int ret;

		ret = psp_restore(sp);
		if (ret)

> +			return ret;
> +	}
> +#endif
> +	return sp_resume(sp);
> +}
> +
>  #ifdef CONFIG_CRYPTO_DEV_SP_PSP
>  static const struct sev_vdata sevv1 = {
>  	.cmdresp_reg		= 0x10580,	/* C2PMSG_32 */
> @@ -563,7 +578,14 @@ static const struct pci_device_id sp_pci_table[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, sp_pci_table);
>  
> -static SIMPLE_DEV_PM_OPS(sp_pci_pm_ops, sp_pci_suspend, sp_pci_resume);
> +static const struct dev_pm_ops sp_pci_pm_ops = {
> +	.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_early = pm_sleep_ptr(sp_pci_restore),
> +};
>  
>  static struct pci_driver sp_pci_driver = {
>  	.name = "ccp",
> diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
> index af881daa5855b..11c4b05e2f3a2 100644
> --- a/drivers/crypto/ccp/tee-dev.c
> +++ b/drivers/crypto/ccp/tee-dev.c
> @@ -366,3 +366,8 @@ int psp_check_tee_status(void)
>  	return 0;
>  }
>  EXPORT_SYMBOL(psp_check_tee_status);
> +
> +int tee_restore(struct psp_device *psp)
> +{
> +	return tee_init_ring(psp->tee_data);
> +}
> 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__ */
> 

-- 
 i.

Re: [PATCH v3 5/5] crypto: ccp - Send PSP_CMD_TEE_RING_DESTROY when PSP_CMD_TEE_RING_INIT fails

[Mario Limonciello]

On 12/15/25 6:01 AM, Ilpo Järvinen wrote:
> On Sun, 14 Dec 2025, Mario Limonciello (AMD) wrote:
> 
>> The hibernate resume sequence involves loading a resume kernel that is just
>> used for loading the hibernate image before shifting back to the existing
>> kernel.
>>
>> During that hibernate resume sequence the resume kernel may have loaded
>> the ccp driver.  If this happens the resume kernel will also have called
>> PSP_CMD_TEE_RING_INIT but it will never have called
>> PSP_CMD_TEE_RING_DESTROY.
>>
>> This is problematic because the existing kernel needs to re-initialize the
>> ring.  One could argue that the existing kernel should call destroy
>> as part of restore() but there is no guarantee that the resume kernel did
>> or didn't load the ccp driver.  There is also no callback opportunity for
>> the resume kernel to destroy before handing back control to the existing
>> kernel.
>>
>> Similar problems could potentially exist with the use of kdump and
>> crash handling. I actually reproduced this issue like this:
>>
>> 1) rmmod ccp
>> 2) hibernate the system
>> 3) resume the system
>> 4) modprobe ccp
>>
>> The resume kernel will have loaded ccp but never destroyed and then when
>> I try to modprobe it fails.
>>
>> Because of these possible cases add a flow that checks the error code from
>> the PSP_CMD_TEE_RING_INIT call and tries to call PSP_CMD_TEE_RING_DESTROY
>> if it failed.  If this succeeds then call PSP_CMD_TEE_RING_INIT again.
>>
>> Fixes: f892a21f51162 ("crypto: ccp - use generic power management")
>> Reported-by: Lars Francke <lars.francke@gmail.com>
>> Closes: https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> v3:
>>   * Add a comment (Tom)
>>   * Add a define for busy condition (Shyam)
>>   * Rename label (Shyam)
>>   * Upgrade message to info (Shyam)
>>   * Use a helper that validates result for destroy command (Shyam)
>> ---
>>   drivers/crypto/ccp/tee-dev.c | 12 ++++++++++++
>>   include/linux/psp.h          |  2 ++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
>> index ef1430f86ad62..9edb220abbc1a 100644
>> --- a/drivers/crypto/ccp/tee-dev.c
>> +++ b/drivers/crypto/ccp/tee-dev.c
>> @@ -113,6 +113,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
>>   {
>>   	int ring_size = MAX_RING_BUFFER_ENTRIES * sizeof(struct tee_ring_cmd);
>>   	struct tee_init_ring_cmd *cmd;
>> +	bool retry = false;
>>   	unsigned int reg;
>>   	int ret;
>>   
>> @@ -135,6 +136,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
>>   	/* Send command buffer details to Trusted OS by writing to
>>   	 * CPU-PSP message registers
>>   	 */
>> +retry_init:
>>   	ret = psp_mailbox_command(tee->psp, PSP_CMD_TEE_RING_INIT, cmd,
>>   				  TEE_DEFAULT_CMD_TIMEOUT, &reg);
>>   	if (ret) {
>> @@ -145,6 +147,16 @@ static int tee_init_ring(struct psp_tee_device *tee)
>>   	}
>>   
>>   	if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
>> +		/*
>> +		 * During the hibernate resume sequence driver may have gotten loaded
>> +		 * but the ring not properly destroyed. If the ring doesn't work, try
>> +		 * to destroy and re-init once.
>> +		 */
>> +		if (!retry && FIELD_GET(PSP_CMDRESP_STS, reg) == PSP_TEE_STATUS_RING_BUSY) {
>> +			dev_info(tee->dev, "tee: ring init command failed with busy status, retrying\n");
>> +			if (tee_send_destroy_cmd(tee))
>> +				goto retry_init;
>> +		}
>>   		dev_err(tee->dev, "tee: ring init command failed (%#010lx)\n",
>>   			FIELD_GET(PSP_CMDRESP_STS, reg));
>>   		tee_free_ring(tee);
>> diff --git a/include/linux/psp.h b/include/linux/psp.h
>> index 92e60aeef21e1..a329148e3684b 100644
>> --- a/include/linux/psp.h
>> +++ b/include/linux/psp.h
>> @@ -23,6 +23,8 @@
>>   #define PSP_CMDRESP_RECOVERY	BIT(30)
>>   #define PSP_CMDRESP_RESP	BIT(31)
>>   
>> +#define PSP_TEE_STATUS_RING_BUSY 0x0000000d  /* Ring already initialized */
> 
> It would be better to have this right underneath PSP_CMDRESP_STS (you
> can use one extra space to indent different from the mask and bits).
> 
> Also, there's inconsistency in STS vs STATUS in the naming.

OK - to make sure I get it like you are suggesting for next spin, you 
mean like this right?

diff --git a/include/linux/psp.h b/include/linux/psp.h
index 92e60aeef21e..b337dcce1e99 100644
--- a/include/linux/psp.h
+++ b/include/linux/psp.h
@@ -18,6 +18,7 @@
   * and should include an appropriate local definition in their source 
file.
   */
  #define PSP_CMDRESP_STS                GENMASK(15, 0)
+#define  PSP_TEE_STS_RING_BUSY 0x0000000d  /* Ring already initialized */
  #define PSP_CMDRESP_CMD                GENMASK(23, 16)
  #define PSP_CMDRESP_RESERVED   GENMASK(29, 24)
  #define PSP_CMDRESP_RECOVERY   BIT(30)

> 
>> +
>>   #define PSP_DRBL_MSG		PSP_CMDRESP_CMD
>>   #define PSP_DRBL_RING		BIT(0)
>>   
>>
> 

Re: [PATCH v3 5/5] crypto: ccp - Send PSP_CMD_TEE_RING_DESTROY when PSP_CMD_TEE_RING_INIT fails

[Ilpo Järvinen]

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

On Mon, 15 Dec 2025, Mario Limonciello wrote:

> On 12/15/25 6:01 AM, Ilpo Järvinen wrote:
> > On Sun, 14 Dec 2025, Mario Limonciello (AMD) wrote:
> > 
> > > The hibernate resume sequence involves loading a resume kernel that is
> > > just
> > > used for loading the hibernate image before shifting back to the existing
> > > kernel.
> > > 
> > > During that hibernate resume sequence the resume kernel may have loaded
> > > the ccp driver.  If this happens the resume kernel will also have called
> > > PSP_CMD_TEE_RING_INIT but it will never have called
> > > PSP_CMD_TEE_RING_DESTROY.
> > > 
> > > This is problematic because the existing kernel needs to re-initialize the
> > > ring.  One could argue that the existing kernel should call destroy
> > > as part of restore() but there is no guarantee that the resume kernel did
> > > or didn't load the ccp driver.  There is also no callback opportunity for
> > > the resume kernel to destroy before handing back control to the existing
> > > kernel.
> > > 
> > > Similar problems could potentially exist with the use of kdump and
> > > crash handling. I actually reproduced this issue like this:
> > > 
> > > 1) rmmod ccp
> > > 2) hibernate the system
> > > 3) resume the system
> > > 4) modprobe ccp
> > > 
> > > The resume kernel will have loaded ccp but never destroyed and then when
> > > I try to modprobe it fails.
> > > 
> > > Because of these possible cases add a flow that checks the error code from
> > > the PSP_CMD_TEE_RING_INIT call and tries to call PSP_CMD_TEE_RING_DESTROY
> > > if it failed.  If this succeeds then call PSP_CMD_TEE_RING_INIT again.
> > > 
> > > Fixes: f892a21f51162 ("crypto: ccp - use generic power management")
> > > Reported-by: Lars Francke <lars.francke@gmail.com>
> > > Closes:
> > > https://lore.kernel.org/platform-driver-x86/CAD-Ua_gfJnQSo8ucS_7ZwzuhoBRJ14zXP7s8b-zX3ZcxcyWePw@mail.gmail.com/
> > > Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> > > ---
> > > v3:
> > >   * Add a comment (Tom)
> > >   * Add a define for busy condition (Shyam)
> > >   * Rename label (Shyam)
> > >   * Upgrade message to info (Shyam)
> > >   * Use a helper that validates result for destroy command (Shyam)
> > > ---
> > >   drivers/crypto/ccp/tee-dev.c | 12 ++++++++++++
> > >   include/linux/psp.h          |  2 ++
> > >   2 files changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
> > > index ef1430f86ad62..9edb220abbc1a 100644
> > > --- a/drivers/crypto/ccp/tee-dev.c
> > > +++ b/drivers/crypto/ccp/tee-dev.c
> > > @@ -113,6 +113,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
> > >   {
> > >   	int ring_size = MAX_RING_BUFFER_ENTRIES * sizeof(struct tee_ring_cmd);
> > >   	struct tee_init_ring_cmd *cmd;
> > > +	bool retry = false;
> > >   	unsigned int reg;
> > >   	int ret;
> > >   @@ -135,6 +136,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
> > >   	/* Send command buffer details to Trusted OS by writing to
> > >   	 * CPU-PSP message registers
> > >   	 */
> > > +retry_init:
> > >   	ret = psp_mailbox_command(tee->psp, PSP_CMD_TEE_RING_INIT, cmd,
> > >   				  TEE_DEFAULT_CMD_TIMEOUT, &reg);
> > >   	if (ret) {
> > > @@ -145,6 +147,16 @@ static int tee_init_ring(struct psp_tee_device *tee)
> > >   	}
> > >     	if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
> > > +		/*
> > > +		 * During the hibernate resume sequence driver may have gotten
> > > loaded
> > > +		 * but the ring not properly destroyed. If the ring doesn't
> > > work, try
> > > +		 * to destroy and re-init once.
> > > +		 */
> > > +		if (!retry && FIELD_GET(PSP_CMDRESP_STS, reg) ==
> > > PSP_TEE_STATUS_RING_BUSY) {
> > > +			dev_info(tee->dev, "tee: ring init command failed with
> > > busy status, retrying\n");
> > > +			if (tee_send_destroy_cmd(tee))
> > > +				goto retry_init;
> > > +		}
> > >   		dev_err(tee->dev, "tee: ring init command failed (%#010lx)\n",
> > >   			FIELD_GET(PSP_CMDRESP_STS, reg));
> > >   		tee_free_ring(tee);
> > > diff --git a/include/linux/psp.h b/include/linux/psp.h
> > > index 92e60aeef21e1..a329148e3684b 100644
> > > --- a/include/linux/psp.h
> > > +++ b/include/linux/psp.h
> > > @@ -23,6 +23,8 @@
> > >   #define PSP_CMDRESP_RECOVERY	BIT(30)
> > >   #define PSP_CMDRESP_RESP	BIT(31)
> > >   +#define PSP_TEE_STATUS_RING_BUSY 0x0000000d  /* Ring already
> > > initialized */
> > 
> > It would be better to have this right underneath PSP_CMDRESP_STS (you
> > can use one extra space to indent different from the mask and bits).
> > 
> > Also, there's inconsistency in STS vs STATUS in the naming.
> 
> OK - to make sure I get it like you are suggesting for next spin, you mean
> like this right?
>
> diff --git a/include/linux/psp.h b/include/linux/psp.h
> index 92e60aeef21e..b337dcce1e99 100644
> --- a/include/linux/psp.h
> +++ b/include/linux/psp.h
> @@ -18,6 +18,7 @@
>   * and should include an appropriate local definition in their source file.
>   */
>  #define PSP_CMDRESP_STS                GENMASK(15, 0)
> +#define  PSP_TEE_STS_RING_BUSY 0x0000000d  /* Ring already initialized */
>  #define PSP_CMDRESP_CMD                GENMASK(23, 16)
>  #define PSP_CMDRESP_RESERVED   GENMASK(29, 24)
>  #define PSP_CMDRESP_RECOVERY   BIT(30)

Yes.

-- 
 i.