[RFC PATCH 0/3] ACPI, x86/cpu/amd: Parse S5_REST_STATUS from ACPI PHAT on supported AMD platforms

[RFC PATCH 0/3] ACPI, x86/cpu/amd: Parse S5_REST_STATUS from ACPI PHAT on supported AMD platforms

[K Prateek Nayak]

x86/cpu/amd bits uses the FCH register to read the S5_RESET_STATUS which
is known to be inaccurate on some configurations where the firmware can
consume the status before the OS boots.

Vilas and Yazen noted that the ACPI PHAT table has a read-only shadow
copy of this state and is the preferred way for OS to consume the S5
Reset Status.

Introduce generic helpers to parse ACPI PHAT records and use the
platform specific helpers to interpret the vendor specific reset reason
records.

Since these records are read-only, the status cannot be cleared once
consumed and the last reset reason will persist across kxec until next
reset which reintroduces the case prevented by commit e6416c2dfe23c
("x86/CPU/AMD: Prevent reset reasons from being retained across reboot")
by clearing the FCH register once consumed.

This can be avoided with a boolean flags passed over as setup data
during kexec but this scheme adds unnecessary complexity in
implementation that has been left out for now.

Patches are based on:

  git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi

at commit 3109f9f38800 ("ACPI: button: Add missing device class clearing
on probe failures") (tag: 'acpi-7.1-rc6')

Series has been tested on both, a Zen4 platform that does not contain
the PHAT record, and a Zen5 platform that contains the PHAT record.
The Zen5 platform was also tested with CONFIG_ACPI_PHAT disabled to
confirm that logic correctly falls back to reading the FCH register.

x86 folks have been Cc'd only on the Cover letter and the x86 bits. ACPI
folks, my fellow AMD colleagues, and the lists should have received the
entire series. If you would like to be Cc'd on the full series for
future posting, please let me know. Sorry in advance for any oversight!
---
K Prateek Nayak (3):
  ACPICA: actbl2.h: ACPI 6.5: PHAT: Add more struct definitions
  ACPI: PHAT: Add generic helper to parse PHAT records
  x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported

 arch/x86/kernel/cpu/amd.c |  34 ++++++++++
 drivers/acpi/Kconfig      |  11 ++++
 drivers/acpi/Makefile     |   1 +
 drivers/acpi/acpi_phat.c  | 133 ++++++++++++++++++++++++++++++++++++++
 include/acpi/actbl2.h     |  14 ++++
 include/linux/acpi.h      |  14 ++++
 6 files changed, 207 insertions(+)
 create mode 100644 drivers/acpi/acpi_phat.c


base-commit: 3109f9f38800841e46769e95e1ba11f1f8c7b230
-- 
2.34.1

[RFC PATCH 1/3] ACPICA: actbl2.h: ACPI 6.5: PHAT: Add more struct definitions

[K Prateek Nayak]

The ACPI Specification v6.5 [1] under Sec. 5.2.31 "Platform Health
Assessment Table (PHAT)" listed additional definition for
"Device-specific Data" and "Vendor Specific Reset Reason Entry" in
Table 5.168 "Reset Reason Health Record Structure" and
Table 5.169: "Reset Reason Health Record Vendor Data Entry".

Add these definitions as part actbl2.h. The struct definitions will be
used in the subsequent commit to implement a generic PHAT parser for
vendor entries.

Link: https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf [1]
Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
---
 include/acpi/actbl2.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 5c0b55e7b3e4..bff0b92d0c82 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -2844,6 +2844,20 @@ struct acpi_phat_health_data {
 	u32 device_specific_offset;	/* Zero if no Device-specific data */
 };
 
+struct acpi_phat_device_data {
+	u8 supported_sources;
+	u8 source;
+	u8 sub_source;
+	u8 reason;
+	u16 vendor_count;		/* Zero if no Vendor specific data */
+};
+
+struct acpi_phat_vendor_element {
+	u8 vendor_guid[16];
+	u16 length;
+	u16 revision;
+};
+
 /* Values for Health field above */
 
 #define ACPI_PHAT_ERRORS_FOUND          0
-- 
2.34.1

[RFC PATCH 2/3] ACPI: PHAT: Add generic helper to parse PHAT records

[K Prateek Nayak]

The ACPI Specification v6.5 [1] under Sec. 5.2.31 "Platform Health
Assessment Table (PHAT)" provides a well defined data format for reset
reason records, including the ones that are vendor specific.

Provide a generic helper to locate the vendor specific records and parse
them into a buffer that can be consumed by the users.

The first user for the interface is added in the subsequent commit with
AMD processors using it to parse the s5_RESET_STATUS from PHAT records.

Make the Kconfig depend on ACPI && CPU_SUP_AMD to prevent the need for
unnecessarily compiling in ACPI PHAT support when not needed.

Link: https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf [1]
Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
---
 drivers/acpi/Kconfig     |  11 ++++
 drivers/acpi/Makefile    |   1 +
 drivers/acpi/acpi_phat.c | 133 +++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h     |  14 +++++
 4 files changed, 159 insertions(+)
 create mode 100644 drivers/acpi/acpi_phat.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index f165d14cf61a..494c757a62d7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -607,6 +607,17 @@ config ACPI_PRMT
 	  substantially increase computational overhead related to the
 	  initialization of some server systems.
 
+config ACPI_PHAT
+	bool "ACPI PHAT Record parsing support"
+	depends on X86 && ACPI && CPU_SUP_AMD
+	default y
+	help
+	  The Platform Health Assessment Table (PHAT) allows platforms to
+	  expose read-only data for diagnostics and health assessments.
+
+	  Enable this feature to access generic helpers for parsing PHAT
+	  data records.
+
 endif	# ACPI
 
 config X86_PM_TIMER
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index d1b0affb844f..d6cf252ca4ea 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
 obj-$(CONFIG_ACPI_PPTT) 	+= pptt.o
 obj-$(CONFIG_ACPI_PFRUT)	+= pfr_update.o pfr_telemetry.o
+obj-$(CONFIG_ACPI_PHAT) 	+= acpi_phat.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o processor_thermal.o
diff --git a/drivers/acpi/acpi_phat.c b/drivers/acpi/acpi_phat.c
new file mode 100644
index 000000000000..de612ae018e0
--- /dev/null
+++ b/drivers/acpi/acpi_phat.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Helpers for parsing Platform Health Assessment Table (PHAT) records.
+ */
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+
+#include <acpi/actbl2.h>
+
+/*
+ * __phat_get_firmware_health_data() - Internal helper to locate the
+ * "Reset Reason Health Record" within the Platform Health Assessment
+ * Table (PHAT).
+ *
+ * @table: Pointer to the beginning of PHAT.
+ *
+ * Return: A pointer within the @table if "Reset Reason Health Record"
+ * is found; NULL otherwise.
+ */
+static struct acpi_phat_health_data *
+__phat_get_firmware_health_data(struct acpi_table_phat *table)
+{
+	unsigned int length = table->header.length;
+	void *header = table;
+
+	/* No records. */
+	if (length <= sizeof(struct acpi_table_header))
+		return NULL;
+
+	/*
+	 * Advance to the end of the table header
+	 * where the first record starts.
+	 */
+	header += sizeof(struct acpi_table_header);
+	length -= sizeof(struct acpi_table_header);
+
+	/*
+	 * Search for PHAT firmware health data record header
+	 * with type == ACPI_PHAT_TYPE_FW_HEALTH_DATA.
+	 */
+	while (length) {
+		struct acpi_phat_health_data *data = header;
+
+		if (data->header.type == ACPI_PHAT_TYPE_FW_HEALTH_DATA)
+			return header;
+
+		/* Move to the next header */
+		header += data->header.length;
+		length -= data->header.length;
+	}
+
+	return NULL;
+}
+
+/**
+ * acpi_phat_get_vendor_reset_reason - Find a "Vendor Specific Reset Reason
+ * Entry" with the matching @guid from the "Firmware Health Data Record". If
+ * successfully located, the function will allocate an object of the size
+ * "acpi_phat_vendor_element.length" and return a pointer populated with the
+ * content of the record.
+ *
+ * @guid: The "Vendor Data ID" of the reset reason record.
+ *
+ * Return: A valid pointer to an allocated "acpi_phat_vendor_element" populated
+ * with the data from the record with matching @guid; an ERR_PTR() otherwise if
+ * no matching records were found, or if the element could not be allocated.
+ * If a valid pointer was returned, the user must call
+ * acpi_phat_put_vendor_reset_reason() for the object once done to reclaim the
+ * allocated memory.
+ */
+struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid)
+{
+	struct acpi_table_header *phat_tbl __free(acpi_put_table) = NULL;
+	struct acpi_phat_health_data *fw_health_data;
+	struct acpi_phat_device_data *dev_data;
+	acpi_status status;
+	void *data;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_PHAT, 0, &phat_tbl);
+	if (ACPI_FAILURE(status))
+		return ERR_PTR(-ENODEV);
+
+	fw_health_data =
+		__phat_get_firmware_health_data((struct acpi_table_phat *)phat_tbl);
+	if (!fw_health_data)
+		return ERR_PTR(-ENODEV);
+
+	/* Check if Device-specific data record is present. */
+	if (!fw_health_data->device_specific_offset)
+		return ERR_PTR(-ENODEV);
+
+	dev_data = (void *)fw_health_data + fw_health_data->device_specific_offset;
+	if (!dev_data->vendor_count)
+		return ERR_PTR(-ENODEV);
+
+	/* Vendor data starts after Device-specific data */
+	data = (void *)dev_data + sizeof(*dev_data);
+
+	for (i = 0; i <= dev_data->vendor_count; ++i) {
+		struct acpi_phat_vendor_element *vendor_data = data;
+		int length = vendor_data->length;
+
+		/*
+		 * Move to the next Vendor specific entry if
+		 * the GUID of entry doesn't match.
+		 */
+		if (!guid_equal(guid, (guid_t *)vendor_data->vendor_guid)) {
+			data += vendor_data->length;
+			continue;
+		}
+
+		vendor_data = kmalloc(length, GFP_KERNEL);
+		if (!vendor_data)
+			return ERR_PTR(-ENOMEM);
+
+		memcpy(vendor_data, data, length);
+		return vendor_data;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+/**
+ * acpi_phat_put_vendor_reset_reason - Reclaim the object allocated by
+ * acpi_phat_get_vendor_reset_reason().
+ *
+ * @reason: A valid pointer returned by acpi_phat_get_vendor_reset_reason()
+ */
+void acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason)
+{
+	kfree(reason);
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 67effb91fa98..daed1d66ca43 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1627,6 +1627,20 @@ extern int acpi_ffh_address_space_arch_handler(acpi_integer *value,
 static inline void acpi_init_ffh(void) { }
 #endif
 
+#ifdef CONFIG_ACPI_PHAT
+struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid);
+void acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason);
+#else
+static inline struct acpi_phat_vendor_element *
+acpi_phat_get_vendor_reset_reason(guid_t *guid)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void
+acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason) { }
+#endif
+
 #ifdef CONFIG_ACPI
 extern void acpi_device_notify(struct device *dev);
 extern void acpi_device_notify_remove(struct device *dev);
-- 
2.34.1

[RFC PATCH 3/3] x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported

[K Prateek Nayak]

Firmware can consume the S5_RESET_STATUS from the FCH register and the
OS may see an incorrect value at the time of boot on certain platforms.

Vilas, Yazen noted that S5_RESET_STATUS is also populated in ACPI PHAT
on newer AMD platforms which is more reliable than the status from the
FCH register.

Use the S5_RESET_STATUS from the ACPI PHAT which is described in the
"Platform Health Assessment Table (PHAT)" section of "AMD Family 1Ah
Models 00h–0Fh and Models 10h–1Fh ACPI v6.5 Porting Guide" [1] on
supported platform and fallback to the legacy FCH path if the PHAT
record is not found.

Unlike the FCH register, PHAT only provides a read-only value which
cannot be cleared once consumed and will persist across kexec boots.

Link: https://docs.amd.com/v/u/en-US/58088_0.90_PUB [1]
Suggested-by: Vilas Sridharan <Vilas.Sridharan@amd.com>
Suggested-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
---
 arch/x86/kernel/cpu/amd.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 2f8e8ff2d000..d3dee7fd4c51 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 #include <linux/export.h>
+#include <linux/acpi.h>
 #include <linux/bitops.h>
 #include <linux/dmi.h>
 #include <linux/elf.h>
@@ -11,6 +12,8 @@
 #include <linux/random.h>
 #include <linux/topology.h>
 #include <linux/platform_data/x86/amd-fch.h>
+
+#include <acpi/actbl2.h>
 #include <asm/processor.h>
 #include <asm/apic.h>
 #include <asm/cacheinfo.h>
@@ -1342,8 +1345,21 @@ static const char * const s5_reset_reason_txt[] = {
 	[31] = "a software sync flood event occurred",
 };
 
+/*
+ * "AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh ACPI v6.5 Porting Guide"
+ * specifies the vendor-specific GUID of FCH::PM::S5_RESET_STATUS record as
+ * "1f425831-da46-4f65-9296-3c4d44c387ab" alongside the format of reset reason.
+ */
+#define S5_RESET_STATUS_GUID GUID_INIT(0x1f425831, 0xda46, 0x4f65, 0x92, 0x96,\
+				       0x3c, 0x4d, 0x44, 0xc3, 0x87, 0xab)
+
 static __init int print_s5_reset_status_mmio(void)
 {
+	struct reset_status_record {
+		struct acpi_phat_vendor_element		header;
+		u32					s5_reset_status;
+	} __packed *record;
+	guid_t guid = S5_RESET_STATUS_GUID;
 	void __iomem *addr;
 	u32 value;
 	int i;
@@ -1351,6 +1367,23 @@ static __init int print_s5_reset_status_mmio(void)
 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
 		return 0;
 
+	record = (void *)acpi_phat_get_vendor_reset_reason(&guid);
+	if (!IS_ERR(record)) {
+		bool record_valid = false;
+
+		/* Sanity check before using the parsed record. */
+		if (record->header.length == sizeof(*record)) {
+			pr_debug("Using ACPI PHAT record for S5_RESET_STATUS\n");
+			value = record->s5_reset_status;
+			record_valid = true;
+		}
+
+		acpi_phat_put_vendor_reset_reason((void *)record);
+
+		if (record_valid)
+			goto parse_status;
+	}
+
 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
 	if (!addr)
 		return 0;
@@ -1374,6 +1407,7 @@ static __init int print_s5_reset_status_mmio(void)
 	iowrite32(value, addr);
 	iounmap(addr);
 
+parse_status:
 	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
 		if (!(value & BIT(i)))
 			continue;
-- 
2.34.1

Re: [RFC PATCH 2/3] ACPI: PHAT: Add generic helper to parse PHAT records

[Mario Limonciello]

On 6/3/26 01:33, K Prateek Nayak wrote:
> The ACPI Specification v6.5 [1] under Sec. 5.2.31 "Platform Health
> Assessment Table (PHAT)" provides a well defined data format for reset
> reason records, including the ones that are vendor specific.
> 
> Provide a generic helper to locate the vendor specific records and parse
> them into a buffer that can be consumed by the users.
> 
> The first user for the interface is added in the subsequent commit with
> AMD processors using it to parse the s5_RESET_STATUS from PHAT records.
> 
> Make the Kconfig depend on ACPI && CPU_SUP_AMD to prevent the need for
> unnecessarily compiling in ACPI PHAT support when not needed.
> 
> Link: https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf [1]
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
>   drivers/acpi/Kconfig     |  11 ++++
>   drivers/acpi/Makefile    |   1 +
>   drivers/acpi/acpi_phat.c | 133 +++++++++++++++++++++++++++++++++++++++
>   include/linux/acpi.h     |  14 +++++
>   4 files changed, 159 insertions(+)
>   create mode 100644 drivers/acpi/acpi_phat.c
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index f165d14cf61a..494c757a62d7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -607,6 +607,17 @@ config ACPI_PRMT
>   	  substantially increase computational overhead related to the
>   	  initialization of some server systems.
>   
> +config ACPI_PHAT
> +	bool "ACPI PHAT Record parsing support"
> +	depends on X86 && ACPI && CPU_SUP_AMD
> +	default y
> +	help
> +	  The Platform Health Assessment Table (PHAT) allows platforms to
> +	  expose read-only data for diagnostics and health assessments.
> +
> +	  Enable this feature to access generic helpers for parsing PHAT
> +	  data records.
> +
>   endif	# ACPI
>   
>   config X86_PM_TIMER
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index d1b0affb844f..d6cf252ca4ea 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -108,6 +108,7 @@ obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
>   obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>   obj-$(CONFIG_ACPI_PPTT) 	+= pptt.o
>   obj-$(CONFIG_ACPI_PFRUT)	+= pfr_update.o pfr_telemetry.o
> +obj-$(CONFIG_ACPI_PHAT) 	+= acpi_phat.o
>   
>   # processor has its own "processor." module_param namespace
>   processor-y			:= processor_driver.o processor_thermal.o
> diff --git a/drivers/acpi/acpi_phat.c b/drivers/acpi/acpi_phat.c
> new file mode 100644
> index 000000000000..de612ae018e0
> --- /dev/null
> +++ b/drivers/acpi/acpi_phat.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Helpers for parsing Platform Health Assessment Table (PHAT) records.
> + */
> +#include <linux/kernel.h>
> +#include <linux/acpi.h>
> +
> +#include <acpi/actbl2.h>
> +
> +/*
> + * __phat_get_firmware_health_data() - Internal helper to locate the
> + * "Reset Reason Health Record" within the Platform Health Assessment
> + * Table (PHAT).
> + *
> + * @table: Pointer to the beginning of PHAT.
> + *
> + * Return: A pointer within the @table if "Reset Reason Health Record"
> + * is found; NULL otherwise.
> + */
> +static struct acpi_phat_health_data *
> +__phat_get_firmware_health_data(struct acpi_table_phat *table)
> +{
> +	unsigned int length = table->header.length;
> +	void *header = table;
> +
> +	/* No records. */
> +	if (length <= sizeof(struct acpi_table_header))
> +		return NULL;
> +
> +	/*
> +	 * Advance to the end of the table header
> +	 * where the first record starts.
> +	 */
> +	header += sizeof(struct acpi_table_header);
> +	length -= sizeof(struct acpi_table_header);
> +
> +	/*
> +	 * Search for PHAT firmware health data record header
> +	 * with type == ACPI_PHAT_TYPE_FW_HEALTH_DATA.
> +	 */
> +	while (length) {
> +		struct acpi_phat_health_data *data = header;
> +
> +		if (data->header.type == ACPI_PHAT_TYPE_FW_HEALTH_DATA)
> +			return header;
> +
> +		/* Move to the next header */
> +		header += data->header.length;
> +		length -= data->header.length;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * acpi_phat_get_vendor_reset_reason - Find a "Vendor Specific Reset Reason
> + * Entry" with the matching @guid from the "Firmware Health Data Record". If
> + * successfully located, the function will allocate an object of the size
> + * "acpi_phat_vendor_element.length" and return a pointer populated with the
> + * content of the record.
> + *
> + * @guid: The "Vendor Data ID" of the reset reason record.
> + *
> + * Return: A valid pointer to an allocated "acpi_phat_vendor_element" populated
> + * with the data from the record with matching @guid; an ERR_PTR() otherwise if
> + * no matching records were found, or if the element could not be allocated.
> + * If a valid pointer was returned, the user must call
> + * acpi_phat_put_vendor_reset_reason() for the object once done to reclaim the
> + * allocated memory.
> + */
> +struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid)
> +{
> +	struct acpi_table_header *phat_tbl __free(acpi_put_table) = NULL;
> +	struct acpi_phat_health_data *fw_health_data;
> +	struct acpi_phat_device_data *dev_data;
> +	acpi_status status;
> +	void *data;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_PHAT, 0, &phat_tbl);
> +	if (ACPI_FAILURE(status))
> +		return ERR_PTR(-ENODEV);

For some further sanity checking, should you look at the PHAT revision 
here matches 2 as well?  Table 55 in the linked spec.

> +
> +	fw_health_data =
> +		__phat_get_firmware_health_data((struct acpi_table_phat *)phat_tbl);
> +	if (!fw_health_data)
> +		return ERR_PTR(-ENODEV);
> +
> +	/* Check if Device-specific data record is present. */
> +	if (!fw_health_data->device_specific_offset)
> +		return ERR_PTR(-ENODEV);
> +
> +	dev_data = (void *)fw_health_data + fw_health_data->device_specific_offset;
> +	if (!dev_data->vendor_count)
> +		return ERR_PTR(-ENODEV);
> +
> +	/* Vendor data starts after Device-specific data */
> +	data = (void *)dev_data + sizeof(*dev_data);
> +
> +	for (i = 0; i <= dev_data->vendor_count; ++i) {
> +		struct acpi_phat_vendor_element *vendor_data = data;
> +		int length = vendor_data->length;
> +
> +		/*
> +		 * Move to the next Vendor specific entry if
> +		 * the GUID of entry doesn't match.
> +		 */
> +		if (!guid_equal(guid, (guid_t *)vendor_data->vendor_guid)) {
> +			data += vendor_data->length;
> +			continue;
> +		}
> +
> +		vendor_data = kmalloc(length, GFP_KERNEL);
> +		if (!vendor_data)
> +			return ERR_PTR(-ENOMEM);
> +
> +		memcpy(vendor_data, data, length);
> +		return vendor_data;
> +	}
> +
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +/**
> + * acpi_phat_put_vendor_reset_reason - Reclaim the object allocated by
> + * acpi_phat_get_vendor_reset_reason().
> + *
> + * @reason: A valid pointer returned by acpi_phat_get_vendor_reset_reason()
> + */
> +void acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason)
> +{
> +	kfree(reason);
> +}
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 67effb91fa98..daed1d66ca43 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1627,6 +1627,20 @@ extern int acpi_ffh_address_space_arch_handler(acpi_integer *value,
>   static inline void acpi_init_ffh(void) { }
>   #endif
>   
> +#ifdef CONFIG_ACPI_PHAT
> +struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid);
> +void acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason);
> +#else
> +static inline struct acpi_phat_vendor_element *
> +acpi_phat_get_vendor_reset_reason(guid_t *guid)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void
> +acpi_phat_put_vendor_reset_reason(struct acpi_phat_vendor_element *reason) { }
> +#endif
> +
>   #ifdef CONFIG_ACPI
>   extern void acpi_device_notify(struct device *dev);
>   extern void acpi_device_notify_remove(struct device *dev);

Re: [RFC PATCH 3/3] x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported

[Mario Limonciello]

On 6/3/26 01:33, K Prateek Nayak wrote:
> Firmware can consume the S5_RESET_STATUS from the FCH register and the
> OS may see an incorrect value at the time of boot on certain platforms.
> 
> Vilas, Yazen noted that S5_RESET_STATUS is also populated in ACPI PHAT
> on newer AMD platforms which is more reliable than the status from the
> FCH register.
> 
> Use the S5_RESET_STATUS from the ACPI PHAT which is described in the
> "Platform Health Assessment Table (PHAT)" section of "AMD Family 1Ah
> Models 00h–0Fh and Models 10h–1Fh ACPI v6.5 Porting Guide" [1] on
> supported platform and fallback to the legacy FCH path if the PHAT
> record is not found.
> 
> Unlike the FCH register, PHAT only provides a read-only value which
> cannot be cleared once consumed and will persist across kexec boots.
> 
> Link: https://docs.amd.com/v/u/en-US/58088_0.90_PUB [1]
> Suggested-by: Vilas Sridharan <Vilas.Sridharan@amd.com>
> Suggested-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
>   arch/x86/kernel/cpu/amd.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 2f8e8ff2d000..d3dee7fd4c51 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1,5 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   #include <linux/export.h>
> +#include <linux/acpi.h>
>   #include <linux/bitops.h>
>   #include <linux/dmi.h>
>   #include <linux/elf.h>
> @@ -11,6 +12,8 @@
>   #include <linux/random.h>
>   #include <linux/topology.h>
>   #include <linux/platform_data/x86/amd-fch.h>
> +
> +#include <acpi/actbl2.h>
>   #include <asm/processor.h>
>   #include <asm/apic.h>
>   #include <asm/cacheinfo.h>
> @@ -1342,8 +1345,21 @@ static const char * const s5_reset_reason_txt[] = {
>   	[31] = "a software sync flood event occurred",
>   };
>   
> +/*
> + * "AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh ACPI v6.5 Porting Guide"
> + * specifies the vendor-specific GUID of FCH::PM::S5_RESET_STATUS record as
> + * "1f425831-da46-4f65-9296-3c4d44c387ab" alongside the format of reset reason.
> + */
> +#define S5_RESET_STATUS_GUID GUID_INIT(0x1f425831, 0xda46, 0x4f65, 0x92, 0x96,\
> +				       0x3c, 0x4d, 0x44, 0xc3, 0x87, 0xab)
> +
>   static __init int print_s5_reset_status_mmio(void)
>   {
> +	struct reset_status_record {
> +		struct acpi_phat_vendor_element		header;
> +		u32					s5_reset_status;
> +	} __packed *record;
> +	guid_t guid = S5_RESET_STATUS_GUID;
>   	void __iomem *addr;
>   	u32 value;
>   	int i;
> @@ -1351,6 +1367,23 @@ static __init int print_s5_reset_status_mmio(void)
>   	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>   		return 0;
>   
> +	record = (void *)acpi_phat_get_vendor_reset_reason(&guid);
> +	if (!IS_ERR(record)) {
> +		bool record_valid = false;
> +
> +		/* Sanity check before using the parsed record. */
> +		if (record->header.length == sizeof(*record)) {

Why not do this sanity check in acpi_phat_get_vendor_reset_reason() 
directly?  Then we can always assume if something is returned it's valid 
and it's simpler here (and any other potential callers in future).

> +			pr_debug("Using ACPI PHAT record for S5_RESET_STATUS\n");
> +			value = record->s5_reset_status;
> +			record_valid = true;
> +		}
> +
> +		acpi_phat_put_vendor_reset_reason((void *)record);
> +
> +		if (record_valid)
> +			goto parse_status;
> +	}
> +
>   	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
>   	if (!addr)
>   		return 0;
> @@ -1374,6 +1407,7 @@ static __init int print_s5_reset_status_mmio(void)
>   	iowrite32(value, addr);
>   	iounmap(addr);
>   
> +parse_status:
>   	for (i = 0; i < ARRAY_SIZE(s5_reset_reason_txt); i++) {
>   		if (!(value & BIT(i)))
>   			continue;

Re: [RFC PATCH 0/3] ACPI, x86/cpu/amd: Parse S5_REST_STATUS from ACPI PHAT on supported AMD platforms

[Rong Zhang]

Hi K Prateek,

On Wed, 2026-06-03 at 06:33 +0000, K Prateek Nayak wrote:
> x86/cpu/amd bits uses the FCH register to read the S5_RESET_STATUS which
> is known to be inaccurate on some configurations where the firmware can
> consume the status before the OS boots.
> 
> Vilas and Yazen noted that the ACPI PHAT table has a read-only shadow
> copy of this state and is the preferred way for OS to consume the S5
> Reset Status.
> 
> Introduce generic helpers to parse ACPI PHAT records and use the
> platform specific helpers to interpret the vendor specific reset reason
> records.
> 
> Since these records are read-only, the status cannot be cleared once
> consumed and the last reset reason will persist across kxec until next
> reset which reintroduces the case prevented by commit e6416c2dfe23c
> ("x86/CPU/AMD: Prevent reset reasons from being retained across reboot")
> by clearing the FCH register once consumed.

It's not only about kexec.

The last reset reason register could remain unchanged when the reset is
caused by a voltage drop on a power rail, probably due to a broken power
supply or VRM. This wasted me a lot of frustrating time debugging
irrelevant reset reasons.

Eventually, I RMA'd the broken device and submitted the patch to save
other's time.

Also, some bits are not cleared by hardware as per [1]. If the firmware
doesn't clear them either they will persist anyway.

So I'd prefer clearing the register in any cases.

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

Thanks,
Rong

> 
> This can be avoided with a boolean flags passed over as setup data
> during kexec but this scheme adds unnecessary complexity in
> implementation that has been left out for now.
> 
> Patches are based on:
> 
>   git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi
> 
> at commit 3109f9f38800 ("ACPI: button: Add missing device class clearing
> on probe failures") (tag: 'acpi-7.1-rc6')
> 
> Series has been tested on both, a Zen4 platform that does not contain
> the PHAT record, and a Zen5 platform that contains the PHAT record.
> The Zen5 platform was also tested with CONFIG_ACPI_PHAT disabled to
> confirm that logic correctly falls back to reading the FCH register.
> 
> x86 folks have been Cc'd only on the Cover letter and the x86 bits. ACPI
> folks, my fellow AMD colleagues, and the lists should have received the
> entire series. If you would like to be Cc'd on the full series for
> future posting, please let me know. Sorry in advance for any oversight!
> ---
> K Prateek Nayak (3):
>   ACPICA: actbl2.h: ACPI 6.5: PHAT: Add more struct definitions
>   ACPI: PHAT: Add generic helper to parse PHAT records
>   x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported
> 
>  arch/x86/kernel/cpu/amd.c |  34 ++++++++++
>  drivers/acpi/Kconfig      |  11 ++++
>  drivers/acpi/Makefile     |   1 +
>  drivers/acpi/acpi_phat.c  | 133 ++++++++++++++++++++++++++++++++++++++
>  include/acpi/actbl2.h     |  14 ++++
>  include/linux/acpi.h      |  14 ++++
>  6 files changed, 207 insertions(+)
>  create mode 100644 drivers/acpi/acpi_phat.c
> 
> 
> base-commit: 3109f9f38800841e46769e95e1ba11f1f8c7b230

Re: [RFC PATCH 0/3] ACPI, x86/cpu/amd: Parse S5_REST_STATUS from ACPI PHAT on supported AMD platforms

[K Prateek Nayak]

Hello Rong,

On 6/3/2026 8:07 PM, Rong Zhang wrote:
>> Since these records are read-only, the status cannot be cleared once
>> consumed and the last reset reason will persist across kxec until next
>> reset which reintroduces the case prevented by commit e6416c2dfe23c
>> ("x86/CPU/AMD: Prevent reset reasons from being retained across reboot")
>> by clearing the FCH register once consumed.
> 
> It's not only about kexec.
> 
> The last reset reason register could remain unchanged when the reset is
> caused by a voltage drop on a power rail, probably due to a broken power
> supply or VRM. This wasted me a lot of frustrating time debugging
> irrelevant reset reasons.
> 
> Eventually, I RMA'd the broken device and submitted the patch to save
> other's time.
> 
> Also, some bits are not cleared by hardware as per [1]. If the firmware
> doesn't clear them either they will persist anyway.
> 
> So I'd prefer clearing the register in any cases.
> 
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=206537#attach_303991

Thanks a ton for the additional context. I'm checking with the BIOS
folks if this PHAT record behaves any differently or if it just mirrors
the FCH::PM::S5_RESET_STATUS at the time the firmware loads.

If it is the latter, yes, I agree with you that we need to clear the
FCH register regardless. I'll adjust the v2 accordingly based on what
the BIOS folks come back with.

-- 
Thanks and Regards,
Prateek

Re: [RFC PATCH 2/3] ACPI: PHAT: Add generic helper to parse PHAT records

[K Prateek Nayak]

Hello Mario,

On 6/3/2026 7:55 PM, Mario Limonciello wrote:
>> +/**
>> + * acpi_phat_get_vendor_reset_reason - Find a "Vendor Specific Reset Reason
>> + * Entry" with the matching @guid from the "Firmware Health Data Record". If
>> + * successfully located, the function will allocate an object of the size
>> + * "acpi_phat_vendor_element.length" and return a pointer populated with the
>> + * content of the record.
>> + *
>> + * @guid: The "Vendor Data ID" of the reset reason record.
>> + *
>> + * Return: A valid pointer to an allocated "acpi_phat_vendor_element" populated
>> + * with the data from the record with matching @guid; an ERR_PTR() otherwise if
>> + * no matching records were found, or if the element could not be allocated.
>> + * If a valid pointer was returned, the user must call
>> + * acpi_phat_put_vendor_reset_reason() for the object once done to reclaim the
>> + * allocated memory.
>> + */
>> +struct acpi_phat_vendor_element *acpi_phat_get_vendor_reset_reason(guid_t *guid)
>> +{
>> +    struct acpi_table_header *phat_tbl __free(acpi_put_table) = NULL;
>> +    struct acpi_phat_health_data *fw_health_data;
>> +    struct acpi_phat_device_data *dev_data;
>> +    acpi_status status;
>> +    void *data;
>> +    int i;
>> +
>> +    status = acpi_get_table(ACPI_SIG_PHAT, 0, &phat_tbl);
>> +    if (ACPI_FAILURE(status))
>> +        return ERR_PTR(-ENODEV);
> 
> For some further sanity checking, should you look at the PHAT revision
>  here matches 2 as well?  Table 55 in the linked spec.

Good catch! I'll add in that check in the next version.

-- 
Thanks and Regards,
Prateek

Re: [RFC PATCH 3/3] x86/cpu/amd: Fetch S5_RESET_STATUS from PHAT when supported

[K Prateek Nayak]

Hello Mario,

On 6/3/2026 7:56 PM, Mario Limonciello wrote:
>> +    record = (void *)acpi_phat_get_vendor_reset_reason(&guid);
>> +    if (!IS_ERR(record)) {
>> +        bool record_valid = false;
>> +
>> +        /* Sanity check before using the parsed record. */
>> +        if (record->header.length == sizeof(*record)) {
> 
> Why not do this sanity check in acpi_phat_get_vendor_reset_reason()
> directly?  Then we can always assume if something is returned it's
> valid and it's simpler here (and any other potential callers in future).

My intention here was to catch any changes to the PHAT record format
for this GUID. If we can reasonably assume that isn't going to happen,
we can skip this check entirely.

-- 
Thanks and Regards,
Prateek