[PATCH v2] x86/CPU/AMD: Ignore invalid reset reason value

Yazen Ghannam posted 1 patch 2 months, 1 week ago
arch/x86/kernel/cpu/amd.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[PATCH v2] x86/CPU/AMD: Ignore invalid reset reason value
Posted by Yazen Ghannam 2 months, 1 week ago
The reset reason value may be "all bits set", e.g. 0xFFFFFFFF. This is a
commonly used error response from hardware. This may occur due to a real
hardware issue or when running in a VM.

The user will see all reset reasons reported in this case.

Return early if running in a VM as this register is not emulated.

Check for an error response value and return early to avoid decoding
invalid data.

Also, adjust the data variable type to match the hardware register size.

Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
Reported-by: Libing He <libhe@redhat.com>
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Cc: David Arcari <darcari@redhat.com>
Cc: stable@vger.kernel.org
---
Link:
https://lore.kernel.org/r/20250721181155.3536023-1-yazen.ghannam@amd.com

v1->v2:
* Include Reviewed-by tag from Mario.
* Include hypervisor check suggested by Boris.

 arch/x86/kernel/cpu/amd.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 50f88fe51816..7a10fe426104 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1274,10 +1274,13 @@ static const char * const s5_reset_reason_txt[] = {
 
 static __init int print_s5_reset_status_mmio(void)
 {
-	unsigned long value;
 	void __iomem *addr;
+	u32 value;
 	int i;
 
+	if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
+		return 0;
+
 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
 		return 0;
 
@@ -1288,12 +1291,16 @@ static __init int print_s5_reset_status_mmio(void)
 	value = ioread32(addr);
 	iounmap(addr);
 
+	/* Value with "all bits set" is an error response and should be ignored. */
+	if (value == U32_MAX)
+		return 0;
+
 	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
 		if (!(value & BIT(i)))
 			continue;
 
 		if (s5_reset_reason_txt[i]) {
-			pr_info("x86/amd: Previous system reset reason [0x%08lx]: %s\n",
+			pr_info("x86/amd: Previous system reset reason [0x%08x]: %s\n",
 				value, s5_reset_reason_txt[i]);
 		}
 	}

base-commit: 65f55a30176662ee37fe18b47430ee30b57bfc98
-- 
2.50.1
Re: [PATCH v2] x86/CPU/AMD: Ignore invalid reset reason value
Posted by Yazen Ghannam 1 month, 3 weeks ago
On Wed, Jul 23, 2025 at 08:07:52PM +0000, Yazen Ghannam wrote:
> The reset reason value may be "all bits set", e.g. 0xFFFFFFFF. This is a
> commonly used error response from hardware. This may occur due to a real
> hardware issue or when running in a VM.
> 
> The user will see all reset reasons reported in this case.
> 
> Return early if running in a VM as this register is not emulated.
> 
> Check for an error response value and return early to avoid decoding
> invalid data.
> 
> Also, adjust the data variable type to match the hardware register size.
> 
> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> Reported-by: Libing He <libhe@redhat.com>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Cc: David Arcari <darcari@redhat.com>
> Cc: stable@vger.kernel.org
> ---

Hi all,

Any more feedback on this?

Thanks,
Yazen
Re: [PATCH v2] x86/CPU/AMD: Ignore invalid reset reason value
Posted by Borislav Petkov 2 months, 1 week ago
On July 23, 2025 11:07:52 PM GMT+03:00, Yazen Ghannam <yazen.ghannam@amd.com> wrote:
>The reset reason value may be "all bits set", e.g. 0xFFFFFFFF. This is a
>commonly used error response from hardware. This may occur due to a real
>hardware issue or when running in a VM.
>
>The user will see all reset reasons reported in this case.
>
>Return early if running in a VM as this register is not emulated.
>
>Check for an error response value and return early to avoid decoding
>invalid data.
>
>Also, adjust the data variable type to match the hardware register size.
>
>Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
>Reported-by: Libing He <libhe@redhat.com>
>Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
>Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>Cc: David Arcari <darcari@redhat.com>
>Cc: stable@vger.kernel.org
>---
>Link:
>https://lore.kernel.org/r/20250721181155.3536023-1-yazen.ghannam@amd.com
>
>v1->v2:
>* Include Reviewed-by tag from Mario.
>* Include hypervisor check suggested by Boris.
>
> arch/x86/kernel/cpu/amd.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>index 50f88fe51816..7a10fe426104 100644
>--- a/arch/x86/kernel/cpu/amd.c
>+++ b/arch/x86/kernel/cpu/amd.c
>@@ -1274,10 +1274,13 @@ static const char * const s5_reset_reason_txt[] = {
> 
> static __init int print_s5_reset_status_mmio(void)
> {
>-	unsigned long value;
> 	void __iomem *addr;
>+	u32 value;
> 	int i;
> 
>+	if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
>+		return 0;
>+
> 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> 		return 0;
> 
>@@ -1288,12 +1291,16 @@ static __init int print_s5_reset_status_mmio(void)
> 	value = ioread32(addr);
> 	iounmap(addr);
> 
>+	/* Value with "all bits set" is an error response and should be ignored. */
>+	if (value == U32_MAX)
>+		return 0;
>+
> 	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
> 		if (!(value & BIT(i)))
> 			continue;
> 
> 		if (s5_reset_reason_txt[i]) {
>-			pr_info("x86/amd: Previous system reset reason [0x%08lx]: %s\n",
>+			pr_info("x86/amd: Previous system reset reason [0x%08x]: %s\n",
> 				value, s5_reset_reason_txt[i]);
> 		}
> 	}
>
>base-commit: 65f55a30176662ee37fe18b47430ee30b57bfc98

Ack.
-- 
Sent from a small device: formatting sucks and brevity is inevitable.