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

[Mario Limonciello <superm1@kernel.org>]

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