On 4/11/25 07:06, Borislav Petkov wrote:
On Thu, Apr 10, 2025 at 03:02:02PM -0500, Mario Limonciello wrote:+static __init int print_s5_reset_status_mmio(void) +{ + void __iomem *addr; + unsigned long value; + int bit = -1; + + if (!cpu_feature_enabled(X86_FEATURE_ZEN)) + return 0; + + addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value)); + if (!addr) + return 0;newline.+ value = ioread32(addr); + iounmap(addr); + + do { + bit = find_next_bit(&value, BITS_PER_LONG, bit + 1); + } while (!s5_reset_reason_txt[bit]);What's the idea here? The highest bit is the most fitting one? So why don't you do fls() or so?
The idea was to walk all the bits and pick the first one that has a string associated with it. I was finding that sometimes the reserved bits are set which would get you a NULL pointer deref.
+ pr_info("x86/amd: Previous system reset reason [0x%08lx]: %s\n", + value, s5_reset_reason_txt[bit]);What's guaranteeing that s5_reset_reason_txt[bit] is still set here? I'd suggest you check it again and never trust the hw because we'll be fixing a null ptr here at some point otherwise...
Right; I was worried about that too but find_next_bit() will return the size argument when it doesn't find anything.
So that should be s5_reset_reason_txt[32] which has the "Unknown" string.
