[PATCH 1/4] platform/x86/amd/pmc: Fix LPS0 and debugfs leaks when STB init fails

[PATCH 1/4] platform/x86/amd/pmc: Fix LPS0 and debugfs leaks when STB init fails

[Mario Limonciello]

amd_pmc_probe() registers the LPS0 s2idle handler with
acpi_register_lps0_dev() and creates the driver's debugfs directory
before calling amd_stb_s2d_init(), which is the last step in probe that
can fail.

When amd_stb_s2d_init() fails (for example the S2D telemetry region
cannot be ioremapped on a long-running system, or the SMU rejects the
S2D setup) the error path only calls pci_dev_put() and returns.  This
leaves amd_pmc_s2idle_dev_ops on the global lps0_s2idle_devops_head list
and leaks the debugfs directory, while the devm-managed resources
backing the handler are torn down.

Reloading the module then walks the corrupted list in
acpi_register_lps0_dev() and hits:

  list_add corruption. next->prev should be prev, but was NULL.
  kernel BUG at lib/list_debug.c:29!
   acpi_register_lps0_dev+0x44/0x80
   amd_pmc_probe+0x224/0x380 [amd_pmc]
   platform_probe+0x67/0x90

Even without a reload, the stale registration means the next s2idle
transition calls into torn-down driver state.

Unwind the debugfs directory and the LPS0 registration on the
amd_stb_s2d_init() error path.  acpi_unregister_lps0_dev() is safe to
call unconditionally here: it is guarded on the same conditions as
acpi_register_lps0_dev(), which is exactly what amd_pmc_remove() already
relies on.

Assisted-by: Claude:opus
Reported-by: Francis De Brabandere <francisdb@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221759
Tested-by: Francis De Brabandere <francisdb@gmail.com>
Fixes: 83ad6974dd3b ("platform/x86/amd/pmc: Move STB block into amd_pmc_s2d_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc/pmc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index d50ea62fa2f3a..630a664bdd2f4 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -919,13 +919,17 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	amd_pmc_dbgfs_register(dev);
 	err = amd_stb_s2d_init(dev);
 	if (err)
-		goto err_pci_dev_put;
+		goto err_dbgfs_unregister;
 
 	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
 		amd_mp2_stb_init(dev);
 	pm_report_max_hw_sleep(U64_MAX);
 	return 0;
 
+err_dbgfs_unregister:
+	amd_pmc_dbgfs_unregister(dev);
+	if (IS_ENABLED(CONFIG_SUSPEND))
+		acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops);
 err_pci_dev_put:
 	pci_dev_put(rdev);
 	return err;
-- 
2.43.0

Re: [PATCH 1/4] platform/x86/amd/pmc: Fix LPS0 and debugfs leaks when STB init fails

[Ilpo Järvinen]

On Fri, 17 Jul 2026, Mario Limonciello wrote:

> amd_pmc_probe() registers the LPS0 s2idle handler with
> acpi_register_lps0_dev() and creates the driver's debugfs directory
> before calling amd_stb_s2d_init(), which is the last step in probe that
> can fail.
> 
> When amd_stb_s2d_init() fails (for example the S2D telemetry region
> cannot be ioremapped on a long-running system, or the SMU rejects the
> S2D setup) the error path only calls pci_dev_put() and returns.  This
> leaves amd_pmc_s2idle_dev_ops on the global lps0_s2idle_devops_head list
> and leaks the debugfs directory, while the devm-managed resources
> backing the handler are torn down.
> 
> Reloading the module then walks the corrupted list in
> acpi_register_lps0_dev() and hits:
> 
>   list_add corruption. next->prev should be prev, but was NULL.
>   kernel BUG at lib/list_debug.c:29!
>    acpi_register_lps0_dev+0x44/0x80
>    amd_pmc_probe+0x224/0x380 [amd_pmc]
>    platform_probe+0x67/0x90
> 
> Even without a reload, the stale registration means the next s2idle
> transition calls into torn-down driver state.
> 
> Unwind the debugfs directory and the LPS0 registration on the
> amd_stb_s2d_init() error path.  acpi_unregister_lps0_dev() is safe to
> call unconditionally here: it is guarded on the same conditions as
> acpi_register_lps0_dev(), which is exactly what amd_pmc_remove() already
> relies on.
> 
> Assisted-by: Claude:opus
> Reported-by: Francis De Brabandere <francisdb@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221759
> Tested-by: Francis De Brabandere <francisdb@gmail.com>
> Fixes: 83ad6974dd3b ("platform/x86/amd/pmc: Move STB block into amd_pmc_s2d_init()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/platform/x86/amd/pmc/pmc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index d50ea62fa2f3a..630a664bdd2f4 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -919,13 +919,17 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  	amd_pmc_dbgfs_register(dev);
>  	err = amd_stb_s2d_init(dev);
>  	if (err)
> -		goto err_pci_dev_put;
> +		goto err_dbgfs_unregister;
>  
>  	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
>  		amd_mp2_stb_init(dev);
>  	pm_report_max_hw_sleep(U64_MAX);
>  	return 0;
>  
> +err_dbgfs_unregister:
> +	amd_pmc_dbgfs_unregister(dev);
> +	if (IS_ENABLED(CONFIG_SUSPEND))
> +		acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops);
>  err_pci_dev_put:
>  	pci_dev_put(rdev);
>  	return err;
> 

No need to change this patch as this looks to be using reverse from the 
order they were init in, but amd_pmc_remove() is using different order
and should IMO changed to match (there probably isn't any good reason 
for the other order).

Sashiko mentions a few low value pre-existing problems that too would be 
nice to address eventually.

-- 
 i.

[PATCH 2/4] platform/x86/amd/pmc: Fix error handling in amd_stb_s2d_init()

[Mario Limonciello]

amd_stb_s2d_init() has two problems on its error paths:

 - The return value of the S2D_TELEMETRY_SIZE SMU command is discarded.
   When the SMU refuses the command (e.g. "SMU cmd failed. err: 0xff")
   the failure is only noticed indirectly through the telemetry size
   check and reported as -EIO, masking the real error.

 - dev->msg_port is switched to MSG_PORT_S2D before issuing the S2D SMU
   commands but is only restored to MSG_PORT_PMC on the success path.
   The early "return -EIO" leaves the port stuck on MSG_PORT_S2D, so all
   subsequent SMU communication - including the s2idle prepare/restore
   handlers - is directed at the wrong mailbox.

Consolidate the exit path through a single label so the message port is
always restored, and propagate the SMU command error directly instead of
inferring it from the size.

Assisted-by: Claude:opus
Reported-by: Francis De Brabandere <francisdb@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221759
Tested-by: Francis De Brabandere <francisdb@gmail.com>
Fixes: 3d7d407dfb05 ("platform/x86: amd-pmc: Add support for AMD Spill to DRAM STB feature")
Cc: stable@vger.kernel.org
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc/mp1_stb.c | 28 +++++++++++++++-----------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc/mp1_stb.c b/drivers/platform/x86/amd/pmc/mp1_stb.c
index 753d630f3283d..6a048cb2605ec 100644
--- a/drivers/platform/x86/amd/pmc/mp1_stb.c
+++ b/drivers/platform/x86/amd/pmc/mp1_stb.c
@@ -289,7 +289,7 @@ int amd_stb_s2d_init(struct amd_pmc_dev *dev)
 	u32 phys_addr_low, phys_addr_hi;
 	u64 stb_phys_addr;
 	u32 size = 0;
-	int ret;
+	int ret = 0;
 
 	if (!enable_stb)
 		return 0;
@@ -306,13 +306,17 @@ int amd_stb_s2d_init(struct amd_pmc_dev *dev)
 	/* Spill to DRAM feature uses separate SMU message port */
 	dev->msg_port = MSG_PORT_S2D;
 
-	amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
-	if (size != S2D_TELEMETRY_BYTES_MAX)
-		return -EIO;
+	ret = amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
+	if (ret)
+		goto out;
+	if (size != S2D_TELEMETRY_BYTES_MAX) {
+		ret = -EIO;
+		goto out;
+	}
 
-	/* Get DRAM size */
-	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true);
-	if (ret || !dev->dram_size)
+	/* Get DRAM size; fall back to the default if the query fails */
+	if (amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true) ||
+	    !dev->dram_size)
 		dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX;
 
 	/* Get STB DRAM address */
@@ -321,12 +325,12 @@ int amd_stb_s2d_init(struct amd_pmc_dev *dev)
 
 	stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
 
-	/* Clear msg_port for other SMU operation */
-	dev->msg_port = MSG_PORT_PMC;
-
 	dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr, dev->dram_size);
 	if (!dev->stb_virt_addr)
-		return -ENOMEM;
+		ret = -ENOMEM;
 
-	return 0;
+out:
+	/* Restore the default message port for subsequent SMU operations */
+	dev->msg_port = MSG_PORT_PMC;
+	return ret;
 }
-- 
2.43.0

Re: [PATCH 2/4] platform/x86/amd/pmc: Fix error handling in amd_stb_s2d_init()

[Ilpo Järvinen]

On Fri, 17 Jul 2026, Mario Limonciello wrote:

> amd_stb_s2d_init() has two problems on its error paths:
> 
>  - The return value of the S2D_TELEMETRY_SIZE SMU command is discarded.
>    When the SMU refuses the command (e.g. "SMU cmd failed. err: 0xff")
>    the failure is only noticed indirectly through the telemetry size
>    check and reported as -EIO, masking the real error.
> 
>  - dev->msg_port is switched to MSG_PORT_S2D before issuing the S2D SMU
>    commands but is only restored to MSG_PORT_PMC on the success path.
>    The early "return -EIO" leaves the port stuck on MSG_PORT_S2D, so all
>    subsequent SMU communication - including the s2idle prepare/restore
>    handlers - is directed at the wrong mailbox.

If you fix the second one first (see below, it seems another place needs 
similar fix), the first one can be fixed on top of it in own patch.

> Consolidate the exit path through a single label so the message port is
> always restored, and propagate the SMU command error directly instead of
> inferring it from the size.
> 
> Assisted-by: Claude:opus
> Reported-by: Francis De Brabandere <francisdb@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221759
> Tested-by: Francis De Brabandere <francisdb@gmail.com>
> Fixes: 3d7d407dfb05 ("platform/x86: amd-pmc: Add support for AMD Spill to DRAM STB feature")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/platform/x86/amd/pmc/mp1_stb.c | 28 +++++++++++++++-----------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/mp1_stb.c b/drivers/platform/x86/amd/pmc/mp1_stb.c
> index 753d630f3283d..6a048cb2605ec 100644
> --- a/drivers/platform/x86/amd/pmc/mp1_stb.c
> +++ b/drivers/platform/x86/amd/pmc/mp1_stb.c
> @@ -289,7 +289,7 @@ int amd_stb_s2d_init(struct amd_pmc_dev *dev)
>  	u32 phys_addr_low, phys_addr_hi;
>  	u64 stb_phys_addr;
>  	u32 size = 0;
> -	int ret;
> +	int ret = 0;
>  
>  	if (!enable_stb)
>  		return 0;
> @@ -306,13 +306,17 @@ int amd_stb_s2d_init(struct amd_pmc_dev *dev)
>  	/* Spill to DRAM feature uses separate SMU message port */
>  	dev->msg_port = MSG_PORT_S2D;
>  
> -	amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
> -	if (size != S2D_TELEMETRY_BYTES_MAX)
> -		return -EIO;
> +	ret = amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
> +	if (ret)
> +		goto out;
> +	if (size != S2D_TELEMETRY_BYTES_MAX) {
> +		ret = -EIO;
> +		goto out;
> +	}
>  
> -	/* Get DRAM size */
> -	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true);
> -	if (ret || !dev->dram_size)
> +	/* Get DRAM size; fall back to the default if the query fails */
> +	if (amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true) ||
> +	    !dev->dram_size)
>  		dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX;
>  
>  	/* Get STB DRAM address */
> @@ -321,12 +325,12 @@ int amd_stb_s2d_init(struct amd_pmc_dev *dev)
>  
>  	stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
>  
> -	/* Clear msg_port for other SMU operation */
> -	dev->msg_port = MSG_PORT_PMC;
> -
>  	dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr, dev->dram_size);
>  	if (!dev->stb_virt_addr)
> -		return -ENOMEM;
> +		ret = -ENOMEM;
>  
> -	return 0;
> +out:
> +	/* Restore the default message port for subsequent SMU operations */
> +	dev->msg_port = MSG_PORT_PMC;

Sashiko (correctly?) points out amd_stb_debugfs_open_v2() has the same 
problem of leaving from msg_port.

To me it looks like having ->msg_port in dev is design error and it should 
be a parameter to the relevant functions instead or with some wrapping 
given by those functions that actually need to give the non-default port.

-- 
 i.