[v2] x86/CPU/AMD: Prevent reset reasons from being retained among boots

[PATCH v2] x86/CPU/AMD: Prevent reset reasons from being retained among boots

Posted by Rong Zhang 2 months, 1 week ago

The S5_RESET_STATUS register is parsed on boot and printed to kmsg.
However, this could sometimes be misleading and lead to users wasting a
lot of time on meaningless debugging for two reasons:

* Some bits are never cleared by hardware. It's the software's
responsibility to clear them as per the Processor Programming Reference
(see Link:).

* Some rare hardware-initiated platform resets do not update the
register at all.

In both cases, a previous reboot could leave its trace in the register,
resulting in users seeing unrelated reboot reasons while debugging
random reboots afterward.

Clearing all reason bits solves the issue. Since all reason bits are
write-1-to-clear and we must preserve all other bits, this is done by
writing the read value back to the register.

Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537#attach_303991
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v2:
- Remove a debug message (suggested by Borislav Petkov)
- No longer mention this behavior in the documentation
- Link to v1: https://lore.kernel.org/r/20250913144245.23237-1-i@rong.moe/
---
 arch/x86/kernel/cpu/amd.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 5398db4dedb4a..ccaa51ce63f6e 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1355,11 +1355,23 @@ static __init int print_s5_reset_status_mmio(void)
 		return 0;
 
 	value = ioread32(addr);
-	iounmap(addr);
 
 	/* Value with "all bits set" is an error response and should be ignored. */
-	if (value == U32_MAX)
+	if (value == U32_MAX) {
+		iounmap(addr);
 		return 0;
+	}
+
+	/*
+	 * Clear all reason bits so they won't be retained if the next reset
+	 * does not update the register. Besides, some bits are never cleared by
+	 * hardware so it's software's responsibility to clear them.
+	 *
+	 * Writing the value back effectively clears all reason bits as they are
+	 * write-1-to-clear.
+	 */
+	iowrite32(value, addr);
+	iounmap(addr);
 
 	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
 		if (!(value & BIT(i)))

base-commit: 7d24c651ce163bc04e7683ec75bf976b6ff042e2
-- 
2.51.0

Re: [PATCH v2] x86/CPU/AMD: Prevent reset reasons from being retained among boots

Posted by Mario Limonciello 2 months ago

On 10/10/25 11:59 AM, Rong Zhang wrote:
> The S5_RESET_STATUS register is parsed on boot and printed to kmsg.
> However, this could sometimes be misleading and lead to users wasting a
> lot of time on meaningless debugging for two reasons:
> 
> * Some bits are never cleared by hardware. It's the software's
> responsibility to clear them as per the Processor Programming Reference
> (see Link:).
> 
> * Some rare hardware-initiated platform resets do not update the
> register at all.
> 
> In both cases, a previous reboot could leave its trace in the register,
> resulting in users seeing unrelated reboot reasons while debugging
> random reboots afterward.
> 
> Clearing all reason bits solves the issue. Since all reason bits are
> write-1-to-clear and we must preserve all other bits, this is done by
> writing the read value back to the register.
> 
> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537#attach_303991
> Signed-off-by: Rong Zhang <i@rong.moe>

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

> ---
> Changes in v2:
> - Remove a debug message (suggested by Borislav Petkov)
> - No longer mention this behavior in the documentation
> - Link to v1: https://lore.kernel.org/r/20250913144245.23237-1-i@rong.moe/
> ---
>   arch/x86/kernel/cpu/amd.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 5398db4dedb4a..ccaa51ce63f6e 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1355,11 +1355,23 @@ static __init int print_s5_reset_status_mmio(void)
>   		return 0;
>   
>   	value = ioread32(addr);
> -	iounmap(addr);
>   
>   	/* Value with "all bits set" is an error response and should be ignored. */
> -	if (value == U32_MAX)
> +	if (value == U32_MAX) {
> +		iounmap(addr);
>   		return 0;
> +	}
> +
> +	/*
> +	 * Clear all reason bits so they won't be retained if the next reset
> +	 * does not update the register. Besides, some bits are never cleared by
> +	 * hardware so it's software's responsibility to clear them.
> +	 *
> +	 * Writing the value back effectively clears all reason bits as they are
> +	 * write-1-to-clear.
> +	 */
> +	iowrite32(value, addr);
> +	iounmap(addr);
>   
>   	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
>   		if (!(value & BIT(i)))
> 
> base-commit: 7d24c651ce163bc04e7683ec75bf976b6ff042e2

Re: [PATCH v2] x86/CPU/AMD: Prevent reset reasons from being retained among boots

Posted by Yazen Ghannam 2 months ago

On Sat, Oct 11, 2025 at 12:59:58AM +0800, Rong Zhang wrote:
> The S5_RESET_STATUS register is parsed on boot and printed to kmsg.
> However, this could sometimes be misleading and lead to users wasting a
> lot of time on meaningless debugging for two reasons:
> 
> * Some bits are never cleared by hardware. It's the software's
> responsibility to clear them as per the Processor Programming Reference
> (see Link:).
> 
> * Some rare hardware-initiated platform resets do not update the
> register at all.
> 
> In both cases, a previous reboot could leave its trace in the register,
> resulting in users seeing unrelated reboot reasons while debugging
> random reboots afterward.
> 
> Clearing all reason bits solves the issue. Since all reason bits are
> write-1-to-clear and we must preserve all other bits, this is done by
> writing the read value back to the register.
> 
> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537#attach_303991
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen

[tip: x86/urgent] x86/CPU/AMD: Prevent reset reasons from being retained across reboot

Posted by tip-bot2 for Rong Zhang 2 months ago

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     e6416c2dfe23c9a6fec881fda22ebb9ae486cfc5
Gitweb:        https://git.kernel.org/tip/e6416c2dfe23c9a6fec881fda22ebb9ae486cfc5
Author:        Rong Zhang <i@rong.moe>
AuthorDate:    Sat, 11 Oct 2025 00:59:58 +08:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Wed, 15 Oct 2025 21:38:06 +02:00

x86/CPU/AMD: Prevent reset reasons from being retained across reboot

The S5_RESET_STATUS register is parsed on boot and printed to kmsg.
However, this could sometimes be misleading and lead to users wasting a
lot of time on meaningless debugging for two reasons:

* Some bits are never cleared by hardware. It's the software's
responsibility to clear them as per the Processor Programming Reference
(see [1]).

* Some rare hardware-initiated platform resets do not update the
register at all.

In both cases, a previous reboot could leave its trace in the register,
resulting in users seeing unrelated reboot reasons while debugging random
reboots afterward.

Write the read value back to the register in order to clear all reason bits
since they are write-1-to-clear while the others must be preserved.

  [1]: https://bugzilla.kernel.org/show_bug.cgi?id=206537#attach_303991

  [ bp: Massage commit message. ]

Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
Signed-off-by: Rong Zhang <i@rong.moe>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: <stable@kernel.org>
Link: https://lore.kernel.org/all/20250913144245.23237-1-i@rong.moe/
---
 arch/x86/kernel/cpu/amd.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 5398db4..ccaa51c 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1355,11 +1355,23 @@ static __init int print_s5_reset_status_mmio(void)
 		return 0;
 
 	value = ioread32(addr);
-	iounmap(addr);
 
 	/* Value with "all bits set" is an error response and should be ignored. */
-	if (value == U32_MAX)
+	if (value == U32_MAX) {
+		iounmap(addr);
 		return 0;
+	}
+
+	/*
+	 * Clear all reason bits so they won't be retained if the next reset
+	 * does not update the register. Besides, some bits are never cleared by
+	 * hardware so it's software's responsibility to clear them.
+	 *
+	 * Writing the value back effectively clears all reason bits as they are
+	 * write-1-to-clear.
+	 */
+	iowrite32(value, addr);
+	iounmap(addr);
 
 	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
 		if (!(value & BIT(i)))