mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] ACPI: APEI: GHES: Performance improvements for error notification handlers
@ 2026-01-04 12:04 Shuai Xue
  2026-01-04 12:04 ` [PATCH v2 1/3] ACPI: APEI: GHES: Improve ghes_notify_nmi() status check Shuai Xue
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Shuai Xue @ 2026-01-04 12:04 UTC (permalink / raw)
  To: tony.luck, guohanjun, mchehab, yazen.ghannam
  Cc: dave.jiang, Smita.KoralahalliChannabasappa, leitao, pengdonglin,
	xueshuai, baolin.wang, benjamin.cheatham, bp, dan.j.williams,
	james.morse, lenb, linux-acpi, linux-kernel, rafael, zhuo.song

changes since v1:
- add Tested-by and Reviewed-by tags from Tony
- change return value from AE_BAD_ADDRESS to -EINVAL ghes_map_error_status per Hanjun
- remove unnecessary blank lines per Hanjun

This patch series improves the performance of GHES error notification handlers
(NMI and SEA) by optimizing how they check for active error conditions.

Currently, both ghes_notify_nmi() and ghes_notify_sea() perform expensive
operations on each invocation to determine if there are actual error records
to process. This includes mapping/unmapping physical addresses and accessing
hardware registers, which causes significant overhead especially on systems
with many cores.

The optimizations introduced in this series:
1. Pre-map error status registers during initialization
2. Directly check for active errors using mapped virtual addresses
3. Extract common functionality into reusable helper functions
4. Apply the same optimization to both NMI and SEA handlers

These changes significantly reduce the overhead of error checking:
- NMI handler: From ~15,000 TSC cycles to ~900 cycles
- SEA handler: From 8,138.3 ns to a much faster check

The initial idea for this optimization came from Tony Luck [1], who identified
and implemented the approach for the NMI handler. This series extends the
same concept to the SEA handler and refactors common code into shared helpers.

Patch 1 (Tony Luck): Improves ghes_notify_nmi() status check by pre-mapping
                     error status registers and avoiding repeated mappings.

Patch 2 (Shuai Xue): Extracts common helper functions for error status handling
                     to eliminate code duplication.

Patch 3 (Shuai Xue): Applies the same optimization to ghes_notify_sea() to improve
                     ARMv8 system performance.

https://lore.kernel.org/lkml/20251103230547.8715-1-tony.luck@intel.com/

Shuai Xue (2):
  ACPI: APEI: GHES: Extract helper functions for error status handling
  ACPI: APEI: GHES: Improve ghes_notify_sea() status check

Tony Luck (1):
  ACPI: APEI: GHES: Improve ghes_notify_nmi() status check

 drivers/acpi/apei/ghes.c | 111 ++++++++++++++++++++++++++++++++++++---
 include/acpi/ghes.h      |   1 +
 2 files changed, 106 insertions(+), 6 deletions(-)

-- 
2.39.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] ACPI: APEI: GHES: Improve ghes_notify_nmi() status check
  2026-01-04 12:04 [PATCH v2 0/3] ACPI: APEI: GHES: Performance improvements for error notification handlers Shuai Xue
@ 2026-01-04 12:04 ` Shuai Xue
  2026-01-04 12:04 ` [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling Shuai Xue
  2026-01-04 12:04 ` [PATCH v2 3/3] ACPI: APEI: GHES: Improve ghes_notify_sea() status check Shuai Xue
  2 siblings, 0 replies; 8+ messages in thread
From: Shuai Xue @ 2026-01-04 12:04 UTC (permalink / raw)
  To: tony.luck, guohanjun, mchehab, yazen.ghannam
  Cc: dave.jiang, Smita.KoralahalliChannabasappa, leitao, pengdonglin,
	xueshuai, baolin.wang, benjamin.cheatham, bp, dan.j.williams,
	james.morse, lenb, linux-acpi, linux-kernel, rafael, zhuo.song

From: Tony Luck <tony.luck@intel.com>

ghes_notify_nmi() is called for every NMI and must check whether the NMI was
generated because an error was signalled by platform firmware.

This check is very expensive as for each registered GHES NMI source it reads
from the acpi generic address attached to this error source to get the physical
address of the acpi_hest_generic_status block.  It then checks the "block_status"
to see if an error was logged.

The ACPI/APEI code must create virtual mappings for each of those physical
addresses, and tear them down afterwards. On an Icelake system this takes around
15,000 TSC cycles. Enough to disturb efforts to profile system performance.

If that were not bad enough, there are some atomic accesses in the code path
that will cause cache line bounces between CPUs. A problem that gets worse as
the core count increases.

But BIOS changes neither the acpi generic address nor the physical address of
the acpi_hest_generic_status block. So this walk can be done once when the NMI is
registered to save the virtual address (unmapping if the NMI is ever unregistered).
The "block_status" can be checked directly in the NMI handler. This can be done
without any atomic accesses.

Resulting time to check that there is not an error record is around 900 cycles.

Reported-by: Andi Kleen <andi.kleen@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
Tested-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/acpi/apei/ghes.c | 40 +++++++++++++++++++++++++++++++++++++---
 include/acpi/ghes.h      |  1 +
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 0dc767392a6c..0e97d50b0240 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1456,7 +1456,21 @@ static LIST_HEAD(ghes_nmi);
 static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 {
 	static DEFINE_RAW_SPINLOCK(ghes_notify_lock_nmi);
+	bool active_error = false;
 	int ret = NMI_DONE;
+	struct ghes *ghes;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
+		if (ghes->error_status_vaddr && readl(ghes->error_status_vaddr)) {
+			active_error = true;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	if (!active_error)
+		return ret;
 
 	if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
 		return ret;
@@ -1470,13 +1484,27 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 	return ret;
 }
 
-static void ghes_nmi_add(struct ghes *ghes)
+static int ghes_nmi_add(struct ghes *ghes)
 {
+	struct acpi_hest_generic *g = ghes->generic;
+	u64 paddr;
+	int rc;
+
+	rc = apei_read(&paddr, &g->error_status_address);
+	if (rc)
+		return rc;
+
+	ghes->error_status_vaddr = acpi_os_ioremap(paddr, sizeof(ghes->estatus->block_status));
+	if (!ghes->error_status_vaddr)
+		return -EINVAL;
+
 	mutex_lock(&ghes_list_mutex);
 	if (list_empty(&ghes_nmi))
 		register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
 	list_add_rcu(&ghes->list, &ghes_nmi);
 	mutex_unlock(&ghes_list_mutex);
+
+	return 0;
 }
 
 static void ghes_nmi_remove(struct ghes *ghes)
@@ -1486,6 +1514,10 @@ static void ghes_nmi_remove(struct ghes *ghes)
 	if (list_empty(&ghes_nmi))
 		unregister_nmi_handler(NMI_LOCAL, "ghes");
 	mutex_unlock(&ghes_list_mutex);
+
+	if (ghes->error_status_vaddr)
+		iounmap(ghes->error_status_vaddr);
+
 	/*
 	 * To synchronize with NMI handler, ghes can only be
 	 * freed after NMI handler finishes.
@@ -1493,7 +1525,7 @@ static void ghes_nmi_remove(struct ghes *ghes)
 	synchronize_rcu();
 }
 #else /* CONFIG_HAVE_ACPI_APEI_NMI */
-static inline void ghes_nmi_add(struct ghes *ghes) { }
+static inline int ghes_nmi_add(struct ghes *ghes) { return -EINVAL; }
 static inline void ghes_nmi_remove(struct ghes *ghes) { }
 #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
 
@@ -1661,7 +1693,9 @@ static int ghes_probe(struct platform_device *ghes_dev)
 		ghes_sea_add(ghes);
 		break;
 	case ACPI_HEST_NOTIFY_NMI:
-		ghes_nmi_add(ghes);
+		rc = ghes_nmi_add(ghes);
+		if (rc)
+			goto err;
 		break;
 	case ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED:
 		rc = apei_sdei_register_ghes(ghes);
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index ebd21b05fe6e..58655d313a1f 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -29,6 +29,7 @@ struct ghes {
 	};
 	struct device *dev;
 	struct list_head elist;
+	void __iomem *error_status_vaddr;
 };
 
 struct ghes_estatus_node {
-- 
2.39.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling
  2026-01-04 12:04 [PATCH v2 0/3] ACPI: APEI: GHES: Performance improvements for error notification handlers Shuai Xue
  2026-01-04 12:04 ` [PATCH v2 1/3] ACPI: APEI: GHES: Improve ghes_notify_nmi() status check Shuai Xue
@ 2026-01-04 12:04 ` Shuai Xue
  2026-01-05  5:12   ` Donglin Peng
  2026-01-04 12:04 ` [PATCH v2 3/3] ACPI: APEI: GHES: Improve ghes_notify_sea() status check Shuai Xue
  2 siblings, 1 reply; 8+ messages in thread
From: Shuai Xue @ 2026-01-04 12:04 UTC (permalink / raw)
  To: tony.luck, guohanjun, mchehab, yazen.ghannam
  Cc: dave.jiang, Smita.KoralahalliChannabasappa, leitao, pengdonglin,
	xueshuai, baolin.wang, benjamin.cheatham, bp, dan.j.williams,
	james.morse, lenb, linux-acpi, linux-kernel, rafael, zhuo.song

Refactors the GHES driver by extracting common functionality into
reusable helper functions:

1. ghes_has_active_errors() - Checks if any error sources in a given list
   have active errors
2. ghes_map_error_status() - Maps error status address to virtual address
3. ghes_unmap_error_status() - Unmaps error status virtual address

These helpers eliminate code duplication in the NMI path and prepare for
similar usage in the SEA path in a subsequent patch.

No functional change intended.

Tested-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/acpi/apei/ghes.c | 93 +++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 0e97d50b0240..7600063fe263 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1406,6 +1406,75 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
 	return ret;
 }
 
+/**
+ * ghes_has_active_errors - Check if there are active errors in error sources
+ * @ghes_list: List of GHES entries to check for active errors
+ *
+ * This function iterates through all GHES entries in the given list and
+ * checks if any of them has active error status by reading the error
+ * status register.
+ *
+ * Return: true if at least one source has active error, false otherwise.
+ */
+static bool __maybe_unused ghes_has_active_errors(struct list_head *ghes_list)
+{
+	bool active_error = false;
+	struct ghes *ghes;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(ghes, ghes_list, list) {
+		if (ghes->error_status_vaddr &&
+		    readl(ghes->error_status_vaddr)) {
+			active_error = true;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return active_error;
+}
+
+/**
+ * ghes_map_error_status - Map error status address to virtual address
+ * @ghes: pointer to GHES structure
+ *
+ * Reads the error status address from ACPI HEST table and maps it to a virtual
+ * address that can be accessed by the kernel.
+ *
+ * Return: 0 on success, error code on failure.
+ */
+static int __maybe_unused ghes_map_error_status(struct ghes *ghes)
+{
+	struct acpi_hest_generic *g = ghes->generic;
+	u64 paddr;
+	int rc;
+
+	rc = apei_read(&paddr, &g->error_status_address);
+	if (rc)
+		return rc;
+
+	ghes->error_status_vaddr =
+		acpi_os_ioremap(paddr, sizeof(ghes->estatus->block_status));
+	if (!ghes->error_status_vaddr)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * ghes_unmap_error_status - Unmap error status virtual address
+ * @ghes: pointer to GHES structure
+ *
+ * Unmaps the error status address if it was previously mapped.
+ */
+static void __maybe_unused ghes_unmap_error_status(struct ghes *ghes)
+{
+	if (ghes->error_status_vaddr) {
+		iounmap(ghes->error_status_vaddr);
+		ghes->error_status_vaddr = NULL;
+	}
+}
+
 #ifdef CONFIG_ACPI_APEI_SEA
 static LIST_HEAD(ghes_sea);
 
@@ -1456,20 +1525,9 @@ static LIST_HEAD(ghes_nmi);
 static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 {
 	static DEFINE_RAW_SPINLOCK(ghes_notify_lock_nmi);
-	bool active_error = false;
 	int ret = NMI_DONE;
-	struct ghes *ghes;
-
-	rcu_read_lock();
-	list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
-		if (ghes->error_status_vaddr && readl(ghes->error_status_vaddr)) {
-			active_error = true;
-			break;
-		}
-	}
-	rcu_read_unlock();
 
-	if (!active_error)
+	if (!ghes_has_active_errors(&ghes_nmi))
 		return ret;
 
 	if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
@@ -1486,18 +1544,12 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 
 static int ghes_nmi_add(struct ghes *ghes)
 {
-	struct acpi_hest_generic *g = ghes->generic;
-	u64 paddr;
 	int rc;
 
-	rc = apei_read(&paddr, &g->error_status_address);
+	rc = ghes_map_error_status(ghes);
 	if (rc)
 		return rc;
 
-	ghes->error_status_vaddr = acpi_os_ioremap(paddr, sizeof(ghes->estatus->block_status));
-	if (!ghes->error_status_vaddr)
-		return -EINVAL;
-
 	mutex_lock(&ghes_list_mutex);
 	if (list_empty(&ghes_nmi))
 		register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
@@ -1515,8 +1567,7 @@ static void ghes_nmi_remove(struct ghes *ghes)
 		unregister_nmi_handler(NMI_LOCAL, "ghes");
 	mutex_unlock(&ghes_list_mutex);
 
-	if (ghes->error_status_vaddr)
-		iounmap(ghes->error_status_vaddr);
+	ghes_unmap_error_status(ghes);
 
 	/*
 	 * To synchronize with NMI handler, ghes can only be
-- 
2.39.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] ACPI: APEI: GHES: Improve ghes_notify_sea() status check
  2026-01-04 12:04 [PATCH v2 0/3] ACPI: APEI: GHES: Performance improvements for error notification handlers Shuai Xue
  2026-01-04 12:04 ` [PATCH v2 1/3] ACPI: APEI: GHES: Improve ghes_notify_nmi() status check Shuai Xue
  2026-01-04 12:04 ` [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling Shuai Xue
@ 2026-01-04 12:04 ` Shuai Xue
  2 siblings, 0 replies; 8+ messages in thread
From: Shuai Xue @ 2026-01-04 12:04 UTC (permalink / raw)
  To: tony.luck, guohanjun, mchehab, yazen.ghannam
  Cc: dave.jiang, Smita.KoralahalliChannabasappa, leitao, pengdonglin,
	xueshuai, baolin.wang, benjamin.cheatham, bp, dan.j.williams,
	james.morse, lenb, linux-acpi, linux-kernel, rafael, zhuo.song

Performance testing on ARMv8 systems shows significant overhead in error
status handling in SEA error handling.

- ghes_peek_estatus(): 8,138.3 ns (21,160 cycles).
- ghes_clear_estatus(): 2,038.3 ns (5,300 cycles).

Apply the same optimization used in ghes_notify_nmi() to
ghes_notify_sea() by checking for active errors before processing,

Tested-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/acpi/apei/ghes.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 7600063fe263..54a6461d277a 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1487,6 +1487,9 @@ int ghes_notify_sea(void)
 	static DEFINE_RAW_SPINLOCK(ghes_notify_lock_sea);
 	int rv;
 
+	if (!ghes_has_active_errors(&ghes_sea))
+		return -ENOENT;
+
 	raw_spin_lock(&ghes_notify_lock_sea);
 	rv = ghes_in_nmi_spool_from_list(&ghes_sea, FIX_APEI_GHES_SEA);
 	raw_spin_unlock(&ghes_notify_lock_sea);
@@ -1494,11 +1497,19 @@ int ghes_notify_sea(void)
 	return rv;
 }
 
-static void ghes_sea_add(struct ghes *ghes)
+static int ghes_sea_add(struct ghes *ghes)
 {
+	int rc;
+
+	rc = ghes_map_error_status(ghes);
+	if (rc)
+		return rc;
+
 	mutex_lock(&ghes_list_mutex);
 	list_add_rcu(&ghes->list, &ghes_sea);
 	mutex_unlock(&ghes_list_mutex);
+
+	return 0;
 }
 
 static void ghes_sea_remove(struct ghes *ghes)
@@ -1506,10 +1517,11 @@ static void ghes_sea_remove(struct ghes *ghes)
 	mutex_lock(&ghes_list_mutex);
 	list_del_rcu(&ghes->list);
 	mutex_unlock(&ghes_list_mutex);
+	ghes_unmap_error_status(ghes);
 	synchronize_rcu();
 }
 #else /* CONFIG_ACPI_APEI_SEA */
-static inline void ghes_sea_add(struct ghes *ghes) { }
+static inline int ghes_sea_add(struct ghes *ghes) { return -EINVAL; }
 static inline void ghes_sea_remove(struct ghes *ghes) { }
 #endif /* CONFIG_ACPI_APEI_SEA */
 
@@ -1741,7 +1753,9 @@ static int ghes_probe(struct platform_device *ghes_dev)
 		break;
 
 	case ACPI_HEST_NOTIFY_SEA:
-		ghes_sea_add(ghes);
+		rc = ghes_sea_add(ghes);
+		if (rc)
+			goto err;
 		break;
 	case ACPI_HEST_NOTIFY_NMI:
 		rc = ghes_nmi_add(ghes);
-- 
2.39.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling
  2026-01-04 12:04 ` [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling Shuai Xue
@ 2026-01-05  5:12   ` Donglin Peng
  2026-01-05  5:41     ` Shuai Xue
  0 siblings, 1 reply; 8+ messages in thread
From: Donglin Peng @ 2026-01-05  5:12 UTC (permalink / raw)
  To: Shuai Xue
  Cc: tony.luck, guohanjun, mchehab, yazen.ghannam, dave.jiang,
	Smita.KoralahalliChannabasappa, leitao, pengdonglin, baolin.wang,
	benjamin.cheatham, bp, dan.j.williams, james.morse, lenb,
	linux-acpi, linux-kernel, rafael, zhuo.song

On Sun, Jan 4, 2026 at 8:05 PM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>
> Refactors the GHES driver by extracting common functionality into
> reusable helper functions:
>
> 1. ghes_has_active_errors() - Checks if any error sources in a given list
>    have active errors
> 2. ghes_map_error_status() - Maps error status address to virtual address
> 3. ghes_unmap_error_status() - Unmaps error status virtual address
>
> These helpers eliminate code duplication in the NMI path and prepare for
> similar usage in the SEA path in a subsequent patch.
>
> No functional change intended.
>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> ---
>  drivers/acpi/apei/ghes.c | 93 +++++++++++++++++++++++++++++++---------
>  1 file changed, 72 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 0e97d50b0240..7600063fe263 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1406,6 +1406,75 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
>         return ret;
>  }
>
> +/**
> + * ghes_has_active_errors - Check if there are active errors in error sources
> + * @ghes_list: List of GHES entries to check for active errors
> + *
> + * This function iterates through all GHES entries in the given list and
> + * checks if any of them has active error status by reading the error
> + * status register.
> + *
> + * Return: true if at least one source has active error, false otherwise.
> + */
> +static bool __maybe_unused ghes_has_active_errors(struct list_head *ghes_list)
> +{
> +       bool active_error = false;
> +       struct ghes *ghes;
> +
> +       rcu_read_lock();

Nit: Use `guard(rcu)()` instead of explicit
`rcu_read_lock()`/`rcu_read_unlock()`.

Thanks,
Donglin

> +       list_for_each_entry_rcu(ghes, ghes_list, list) {
> +               if (ghes->error_status_vaddr &&
> +                   readl(ghes->error_status_vaddr)) {
> +                       active_error = true;
> +                       break;
> +               }
> +       }
> +       rcu_read_unlock();
> +
> +       return active_error;
> +}
> +
> +/**
> + * ghes_map_error_status - Map error status address to virtual address
> + * @ghes: pointer to GHES structure
> + *
> + * Reads the error status address from ACPI HEST table and maps it to a virtual
> + * address that can be accessed by the kernel.
> + *
> + * Return: 0 on success, error code on failure.
> + */
> +static int __maybe_unused ghes_map_error_status(struct ghes *ghes)
> +{
> +       struct acpi_hest_generic *g = ghes->generic;
> +       u64 paddr;
> +       int rc;
> +
> +       rc = apei_read(&paddr, &g->error_status_address);
> +       if (rc)
> +               return rc;
> +
> +       ghes->error_status_vaddr =
> +               acpi_os_ioremap(paddr, sizeof(ghes->estatus->block_status));
> +       if (!ghes->error_status_vaddr)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +/**
> + * ghes_unmap_error_status - Unmap error status virtual address
> + * @ghes: pointer to GHES structure
> + *
> + * Unmaps the error status address if it was previously mapped.
> + */
> +static void __maybe_unused ghes_unmap_error_status(struct ghes *ghes)
> +{
> +       if (ghes->error_status_vaddr) {
> +               iounmap(ghes->error_status_vaddr);
> +               ghes->error_status_vaddr = NULL;
> +       }
> +}
> +
>  #ifdef CONFIG_ACPI_APEI_SEA
>  static LIST_HEAD(ghes_sea);
>
> @@ -1456,20 +1525,9 @@ static LIST_HEAD(ghes_nmi);
>  static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
>  {
>         static DEFINE_RAW_SPINLOCK(ghes_notify_lock_nmi);
> -       bool active_error = false;
>         int ret = NMI_DONE;
> -       struct ghes *ghes;
> -
> -       rcu_read_lock();
> -       list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
> -               if (ghes->error_status_vaddr && readl(ghes->error_status_vaddr)) {
> -                       active_error = true;
> -                       break;
> -               }
> -       }
> -       rcu_read_unlock();
>
> -       if (!active_error)
> +       if (!ghes_has_active_errors(&ghes_nmi))
>                 return ret;
>
>         if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
> @@ -1486,18 +1544,12 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
>
>  static int ghes_nmi_add(struct ghes *ghes)
>  {
> -       struct acpi_hest_generic *g = ghes->generic;
> -       u64 paddr;
>         int rc;
>
> -       rc = apei_read(&paddr, &g->error_status_address);
> +       rc = ghes_map_error_status(ghes);
>         if (rc)
>                 return rc;
>
> -       ghes->error_status_vaddr = acpi_os_ioremap(paddr, sizeof(ghes->estatus->block_status));
> -       if (!ghes->error_status_vaddr)
> -               return -EINVAL;
> -
>         mutex_lock(&ghes_list_mutex);
>         if (list_empty(&ghes_nmi))
>                 register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
> @@ -1515,8 +1567,7 @@ static void ghes_nmi_remove(struct ghes *ghes)
>                 unregister_nmi_handler(NMI_LOCAL, "ghes");
>         mutex_unlock(&ghes_list_mutex);
>
> -       if (ghes->error_status_vaddr)
> -               iounmap(ghes->error_status_vaddr);
> +       ghes_unmap_error_status(ghes);
>
>         /*
>          * To synchronize with NMI handler, ghes can only be
> --
> 2.39.3
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling
  2026-01-05  5:12   ` Donglin Peng
@ 2026-01-05  5:41     ` Shuai Xue
  2026-01-09 21:22       ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Shuai Xue @ 2026-01-05  5:41 UTC (permalink / raw)
  To: Donglin Peng
  Cc: tony.luck, guohanjun, mchehab, yazen.ghannam, dave.jiang,
	Smita.KoralahalliChannabasappa, leitao, pengdonglin, baolin.wang,
	benjamin.cheatham, bp, dan.j.williams, james.morse, lenb,
	linux-acpi, linux-kernel, rafael, zhuo.song



On 1/5/26 1:12 PM, Donglin Peng wrote:
> On Sun, Jan 4, 2026 at 8:05 PM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>>
>> Refactors the GHES driver by extracting common functionality into
>> reusable helper functions:
>>
>> 1. ghes_has_active_errors() - Checks if any error sources in a given list
>>     have active errors
>> 2. ghes_map_error_status() - Maps error status address to virtual address
>> 3. ghes_unmap_error_status() - Unmaps error status virtual address
>>
>> These helpers eliminate code duplication in the NMI path and prepare for
>> similar usage in the SEA path in a subsequent patch.
>>
>> No functional change intended.
>>
>> Tested-by: Tony Luck <tony.luck@intel.com>
>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> ---
>>   drivers/acpi/apei/ghes.c | 93 +++++++++++++++++++++++++++++++---------
>>   1 file changed, 72 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 0e97d50b0240..7600063fe263 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -1406,6 +1406,75 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
>>          return ret;
>>   }
>>
>> +/**
>> + * ghes_has_active_errors - Check if there are active errors in error sources
>> + * @ghes_list: List of GHES entries to check for active errors
>> + *
>> + * This function iterates through all GHES entries in the given list and
>> + * checks if any of them has active error status by reading the error
>> + * status register.
>> + *
>> + * Return: true if at least one source has active error, false otherwise.
>> + */
>> +static bool __maybe_unused ghes_has_active_errors(struct list_head *ghes_list)
>> +{
>> +       bool active_error = false;
>> +       struct ghes *ghes;
>> +
>> +       rcu_read_lock();
> 
> Nit: Use `guard(rcu)()` instead of explicit
> `rcu_read_lock()`/`rcu_read_unlock()`.
> 
> Thanks,
> Donglin
> 

Hi, Donglin,

Good point. Will fix in next version.

Thanks.
Shuai

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling
  2026-01-05  5:41     ` Shuai Xue
@ 2026-01-09 21:22       ` Rafael J. Wysocki
  2026-01-12  2:57         ` Shuai Xue
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2026-01-09 21:22 UTC (permalink / raw)
  To: Shuai Xue
  Cc: Donglin Peng, tony.luck, guohanjun, mchehab, yazen.ghannam,
	dave.jiang, Smita.KoralahalliChannabasappa, leitao, pengdonglin,
	baolin.wang, benjamin.cheatham, bp, dan.j.williams, james.morse,
	lenb, linux-acpi, linux-kernel, rafael, zhuo.song

On Mon, Jan 5, 2026 at 6:42 AM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>
>
>
> On 1/5/26 1:12 PM, Donglin Peng wrote:
> > On Sun, Jan 4, 2026 at 8:05 PM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> >>
> >> Refactors the GHES driver by extracting common functionality into
> >> reusable helper functions:
> >>
> >> 1. ghes_has_active_errors() - Checks if any error sources in a given list
> >>     have active errors
> >> 2. ghes_map_error_status() - Maps error status address to virtual address
> >> 3. ghes_unmap_error_status() - Unmaps error status virtual address
> >>
> >> These helpers eliminate code duplication in the NMI path and prepare for
> >> similar usage in the SEA path in a subsequent patch.
> >>
> >> No functional change intended.
> >>
> >> Tested-by: Tony Luck <tony.luck@intel.com>
> >> Reviewed-by: Tony Luck <tony.luck@intel.com>
> >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> >> ---
> >>   drivers/acpi/apei/ghes.c | 93 +++++++++++++++++++++++++++++++---------
> >>   1 file changed, 72 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> >> index 0e97d50b0240..7600063fe263 100644
> >> --- a/drivers/acpi/apei/ghes.c
> >> +++ b/drivers/acpi/apei/ghes.c
> >> @@ -1406,6 +1406,75 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
> >>          return ret;
> >>   }
> >>
> >> +/**
> >> + * ghes_has_active_errors - Check if there are active errors in error sources
> >> + * @ghes_list: List of GHES entries to check for active errors
> >> + *
> >> + * This function iterates through all GHES entries in the given list and
> >> + * checks if any of them has active error status by reading the error
> >> + * status register.
> >> + *
> >> + * Return: true if at least one source has active error, false otherwise.
> >> + */
> >> +static bool __maybe_unused ghes_has_active_errors(struct list_head *ghes_list)
> >> +{
> >> +       bool active_error = false;
> >> +       struct ghes *ghes;
> >> +
> >> +       rcu_read_lock();
> >
> > Nit: Use `guard(rcu)()` instead of explicit
> > `rcu_read_lock()`/`rcu_read_unlock()`.
> >
> > Thanks,
> > Donglin
> >
>
> Hi, Donglin,
>
> Good point. Will fix in next version.

If you do so, you may also get rid of the active_error local variable.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling
  2026-01-09 21:22       ` Rafael J. Wysocki
@ 2026-01-12  2:57         ` Shuai Xue
  0 siblings, 0 replies; 8+ messages in thread
From: Shuai Xue @ 2026-01-12  2:57 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Donglin Peng, tony.luck, guohanjun, mchehab, yazen.ghannam,
	dave.jiang, Smita.KoralahalliChannabasappa, leitao, pengdonglin,
	baolin.wang, benjamin.cheatham, bp, dan.j.williams, james.morse,
	lenb, linux-acpi, linux-kernel, zhuo.song



On 1/10/26 5:22 AM, Rafael J. Wysocki wrote:
> On Mon, Jan 5, 2026 at 6:42 AM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>>
>>
>>
>> On 1/5/26 1:12 PM, Donglin Peng wrote:
>>> On Sun, Jan 4, 2026 at 8:05 PM Shuai Xue <xueshuai@linux.alibaba.com> wrote:
>>>>
>>>> Refactors the GHES driver by extracting common functionality into
>>>> reusable helper functions:
>>>>
>>>> 1. ghes_has_active_errors() - Checks if any error sources in a given list
>>>>      have active errors
>>>> 2. ghes_map_error_status() - Maps error status address to virtual address
>>>> 3. ghes_unmap_error_status() - Unmaps error status virtual address
>>>>
>>>> These helpers eliminate code duplication in the NMI path and prepare for
>>>> similar usage in the SEA path in a subsequent patch.
>>>>
>>>> No functional change intended.
>>>>
>>>> Tested-by: Tony Luck <tony.luck@intel.com>
>>>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>>>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>>>> ---
>>>>    drivers/acpi/apei/ghes.c | 93 +++++++++++++++++++++++++++++++---------
>>>>    1 file changed, 72 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>>>> index 0e97d50b0240..7600063fe263 100644
>>>> --- a/drivers/acpi/apei/ghes.c
>>>> +++ b/drivers/acpi/apei/ghes.c
>>>> @@ -1406,6 +1406,75 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
>>>>           return ret;
>>>>    }
>>>>
>>>> +/**
>>>> + * ghes_has_active_errors - Check if there are active errors in error sources
>>>> + * @ghes_list: List of GHES entries to check for active errors
>>>> + *
>>>> + * This function iterates through all GHES entries in the given list and
>>>> + * checks if any of them has active error status by reading the error
>>>> + * status register.
>>>> + *
>>>> + * Return: true if at least one source has active error, false otherwise.
>>>> + */
>>>> +static bool __maybe_unused ghes_has_active_errors(struct list_head *ghes_list)
>>>> +{
>>>> +       bool active_error = false;
>>>> +       struct ghes *ghes;
>>>> +
>>>> +       rcu_read_lock();
>>>
>>> Nit: Use `guard(rcu)()` instead of explicit
>>> `rcu_read_lock()`/`rcu_read_unlock()`.
>>>
>>> Thanks,
>>> Donglin
>>>
>>
>> Hi, Donglin,
>>
>> Good point. Will fix in next version.
> 
> If you do so, you may also get rid of the active_error local variable.

Got it. Thanks for reminding.
Shuai


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-01-12  3:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-04 12:04 [PATCH v2 0/3] ACPI: APEI: GHES: Performance improvements for error notification handlers Shuai Xue
2026-01-04 12:04 ` [PATCH v2 1/3] ACPI: APEI: GHES: Improve ghes_notify_nmi() status check Shuai Xue
2026-01-04 12:04 ` [PATCH v2 2/3] ACPI: APEI: GHES: Extract helper functions for error status handling Shuai Xue
2026-01-05  5:12   ` Donglin Peng
2026-01-05  5:41     ` Shuai Xue
2026-01-09 21:22       ` Rafael J. Wysocki
2026-01-12  2:57         ` Shuai Xue
2026-01-04 12:04 ` [PATCH v2 3/3] ACPI: APEI: GHES: Improve ghes_notify_sea() status check Shuai Xue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®