From: Ahmed Tiba <ahmed.tiba@arm.com>
To: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Cc: devicetree@vger.kernel.org, linux-acpi@vger.kernel.org,
Dmitry.Lamerov@arm.com, catalin.marinas@arm.com, bp@alien8.de,
robh@kernel.org, rafael@kernel.org, will@kernel.org,
conor@kernel.org, linux-arm-kernel@lists.infradead.org,
linux-doc@vger.kernel.org, krzk+dt@kernel.org,
Michael.Zhao2@arm.com, tony.luck@intel.com
Subject: Re: [PATCH v2 03/11] ACPI: APEI: GHES: move CPER read helpers
Date: Wed, 11 Mar 2026 13:18:28 +0000 [thread overview]
Message-ID: <df306958-bb99-4728-b461-e5ef1f731be2@arm.com> (raw)
In-Reply-To: <CA+Ht8=aL8wp0nwH3vnnm8FnRmfp1nUasfXh3HjeWwi8UadBLOg@mail.gmail.com>
On 26/02/2026 05:58, Himanshu Chauhan wrote:
> On Fri, Feb 20, 2026 at 7:13 PM Ahmed Tiba <ahmed.tiba@arm.com> wrote:
>>
>> Relocate the CPER buffer mapping, peek, and clear helpers from ghes.c into
>> ghes_cper.c so they can be shared with other firmware-first providers.
>> This commit only shuffles code; behavior stays the same.
>>
>> Signed-off-by: Ahmed Tiba <ahmed.tiba@arm.com>
>> ---
>> drivers/acpi/apei/ghes.c | 170 +-----------------------------------------
>> drivers/acpi/apei/ghes_cper.c | 170 +++++++++++++++++++++++++++++++++++++++++-
>> include/acpi/ghes_cper.h | 14 ++--
>> 3 files changed, 177 insertions(+), 177 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 07b70bcb8342..b159dbee90ac 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -118,26 +118,6 @@ static struct gen_pool *ghes_estatus_pool;
>> static struct ghes_estatus_cache __rcu *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
>> static atomic_t ghes_estatus_cache_alloced;
>>
>> -static void __iomem *ghes_map(u64 pfn, enum fixed_addresses fixmap_idx)
>> -{
>> - phys_addr_t paddr;
>> - pgprot_t prot;
>> -
>> - paddr = PFN_PHYS(pfn);
>> - prot = arch_apei_get_mem_attribute(paddr);
>> - __set_fixmap(fixmap_idx, paddr, prot);
>> -
>> - return (void __iomem *) __fix_to_virt(fixmap_idx);
>> -}
>> -
>> -static void ghes_unmap(void __iomem *vaddr, enum fixed_addresses fixmap_idx)
>> -{
>> - int _idx = virt_to_fix((unsigned long)vaddr);
>> -
>> - WARN_ON_ONCE(fixmap_idx != _idx);
>> - clear_fixmap(fixmap_idx);
>> -}
>> -
>> int ghes_estatus_pool_init(unsigned int num_ghes)
>> {
>> unsigned long addr, len;
>> @@ -193,22 +173,7 @@ static void unmap_gen_v2(struct ghes *ghes)
>> apei_unmap_generic_address(&ghes->generic_v2->read_ack_register);
>> }
>>
>> -static void ghes_ack_error(struct acpi_hest_generic_v2 *gv2)
>> -{
>> - int rc;
>> - u64 val = 0;
>> -
>> - rc = apei_read(&val, &gv2->read_ack_register);
>> - if (rc)
>> - return;
>> -
>> - val &= gv2->read_ack_preserve << gv2->read_ack_register.bit_offset;
>> - val |= gv2->read_ack_write << gv2->read_ack_register.bit_offset;
>> -
>> - apei_write(val, &gv2->read_ack_register);
>> -}
>> -
>> -static struct ghes *ghes_new(struct acpi_hest_generic *generic)
>> +struct ghes *ghes_new(struct acpi_hest_generic *generic)
>> {
>> struct ghes *ghes;
>> unsigned int error_block_length;
>> @@ -255,7 +220,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
>> return ERR_PTR(rc);
>> }
>>
>> -static void ghes_fini(struct ghes *ghes)
>> +void ghes_fini(struct ghes *ghes)
>> {
>> kfree(ghes->estatus);
>> apei_unmap_generic_address(&ghes->generic->error_status_address);
>> @@ -280,137 +245,6 @@ static inline int ghes_severity(int severity)
>> }
>> }
>
> Can it be "ghes_finish"? We already have "creat" without 'e'.
This is a pure mechanical move from ghes.c. I’m keeping the original
name here to avoid churn. If we want a rename, I can do that separately
with justification.
>>
>> -static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
>> - int from_phys,
>> - enum fixed_addresses fixmap_idx)
>> -{
>> - void __iomem *vaddr;
>> - u64 offset;
>> - u32 trunk;
>> -
>> - while (len > 0) {
>> - offset = paddr - (paddr & PAGE_MASK);
>> - vaddr = ghes_map(PHYS_PFN(paddr), fixmap_idx);
>> - trunk = PAGE_SIZE - offset;
>> - trunk = min(trunk, len);
>> - if (from_phys)
>> - memcpy_fromio(buffer, vaddr + offset, trunk);
>> - else
>> - memcpy_toio(vaddr + offset, buffer, trunk);
>> - len -= trunk;
>> - paddr += trunk;
>> - buffer += trunk;
>> - ghes_unmap(vaddr, fixmap_idx);
>> - }
>> -}
>> -
>> -/* Check the top-level record header has an appropriate size. */
>> -static int __ghes_check_estatus(struct ghes *ghes,
>> - struct acpi_hest_generic_status *estatus)
>> -{
>> - u32 len = cper_estatus_len(estatus);
>> - u32 max_len = min(ghes->generic->error_block_length,
>> - ghes->estatus_length);
>> -
>> - if (len < sizeof(*estatus)) {
>> - pr_warn_ratelimited(FW_WARN GHES_PFX "Truncated error status block!\n");
>> - return -EIO;
>> - }
>> -
>> - if (!len || len > max_len) {
>> - pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid error status block length!\n");
>> - return -EIO;
>> - }
>> -
>> - if (cper_estatus_check_header(estatus)) {
>> - pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid CPER header!\n");
>> - return -EIO;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -/* Read the CPER block, returning its address, and header in estatus. */
>> -static int __ghes_peek_estatus(struct ghes *ghes,
>> - struct acpi_hest_generic_status *estatus,
>> - u64 *buf_paddr, enum fixed_addresses fixmap_idx)
>> -{
>> - struct acpi_hest_generic *g = ghes->generic;
>> - int rc;
>> -
>> - rc = apei_read(buf_paddr, &g->error_status_address);
>> - if (rc) {
>> - *buf_paddr = 0;
>> - pr_warn_ratelimited(FW_WARN GHES_PFX
>> -"Failed to read error status block address for hardware error source: %d.\n",
>> - g->header.source_id);
>> - return -EIO;
>> - }
>> - if (!*buf_paddr)
>> - return -ENOENT;
>> -
>> - ghes_copy_tofrom_phys(estatus, *buf_paddr, sizeof(*estatus), 1,
>> - fixmap_idx);
>> - if (!estatus->block_status) {
>> - *buf_paddr = 0;
>> - return -ENOENT;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -static int __ghes_read_estatus(struct acpi_hest_generic_status *estatus,
>> - u64 buf_paddr, enum fixed_addresses fixmap_idx,
>> - size_t buf_len)
>> -{
>> - ghes_copy_tofrom_phys(estatus, buf_paddr, buf_len, 1, fixmap_idx);
>> - if (cper_estatus_check(estatus)) {
>> - pr_warn_ratelimited(FW_WARN GHES_PFX
>> - "Failed to read error status block!\n");
>> - return -EIO;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -static int ghes_read_estatus(struct ghes *ghes,
>> - struct acpi_hest_generic_status *estatus,
>> - u64 *buf_paddr, enum fixed_addresses fixmap_idx)
>> -{
>> - int rc;
>> -
>> - rc = __ghes_peek_estatus(ghes, estatus, buf_paddr, fixmap_idx);
>> - if (rc)
>> - return rc;
>> -
>> - rc = __ghes_check_estatus(ghes, estatus);
>> - if (rc)
>> - return rc;
>> -
>> - return __ghes_read_estatus(estatus, *buf_paddr, fixmap_idx,
>> - cper_estatus_len(estatus));
>> -}
>> -
>> -static void ghes_clear_estatus(struct ghes *ghes,
>> - struct acpi_hest_generic_status *estatus,
>> - u64 buf_paddr, enum fixed_addresses fixmap_idx)
>> -{
>> - estatus->block_status = 0;
>> -
>> - if (!buf_paddr)
>> - return;
>> -
>> - ghes_copy_tofrom_phys(estatus, buf_paddr,
>> - sizeof(estatus->block_status), 0,
>> - fixmap_idx);
>> -
>> - /*
>> - * GHESv2 type HEST entries introduce support for error acknowledgment,
>> - * so only acknowledge the error if this support is present.
>> - */
>> - if (is_hest_type_generic_v2(ghes))
>> - ghes_ack_error(ghes->generic_v2);
>> -}
>>
>> /**
>> * struct ghes_task_work - for synchronous RAS event
>> diff --git a/drivers/acpi/apei/ghes_cper.c b/drivers/acpi/apei/ghes_cper.c
>> index 63047322a3d9..7e0015e960c1 100644
>> --- a/drivers/acpi/apei/ghes_cper.c
>> +++ b/drivers/acpi/apei/ghes_cper.c
>
> IMO, just "cper.c" would be fine.
"cper.c" and "include/acpi/cper.h" already exist under EFI. This code is
GHES‑specific CPER handling (the GHES view of CPER), not a generic UEFI
CPER API, so I’m keeping the GHES‑scoped naming to avoid ambiguity.
>> @@ -1,7 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> *
>> - * APEI GHES CPER helper translation unit - staging file for helper moves
>> + * APEI GHES CPER helper translation unit - code mechanically moved from ghes.c
>> *
>> * Copyright (C) 2026 ARM Ltd.
>> * Author: Ahmed Tiba <ahmed.tiba@arm.com>
>> @@ -17,10 +17,176 @@
>> #include <linux/slab.h>
>>
>> #include <acpi/apei.h>
>> +#include <acpi/ghes_cper.h>
>>
>> #include <asm/fixmap.h>
>> #include <asm/tlbflush.h>
>>
>> #include "apei-internal.h"
>>
>> -/* Helper bodies will be moved here in follow-up commits. */
>> +static void __iomem *ghes_map(u64 pfn, enum fixed_addresses fixmap_idx)
>> +{
>> + phys_addr_t paddr;
>> + pgprot_t prot;
>> +
>> + paddr = PFN_PHYS(pfn);
>> + prot = arch_apei_get_mem_attribute(paddr);
>> + __set_fixmap(fixmap_idx, paddr, prot);
>> +
>> + return (void __iomem *) __fix_to_virt(fixmap_idx);
>> +}
>> +
>> +static void ghes_unmap(void __iomem *vaddr, enum fixed_addresses fixmap_idx)
>> +{
>> + int _idx = virt_to_fix((unsigned long)vaddr);
>> +
>> + WARN_ON_ONCE(fixmap_idx != _idx);
>> + clear_fixmap(fixmap_idx);
>> +}
>> +
>> +static void ghes_ack_error(struct acpi_hest_generic_v2 *gv2)
>> +{
>> + int rc;
>> + u64 val = 0;
>> +
>> + rc = apei_read(&val, &gv2->read_ack_register);
>> + if (rc)
>> + return;
>> +
>> + val &= gv2->read_ack_preserve << gv2->read_ack_register.bit_offset;
>> + val |= gv2->read_ack_write << gv2->read_ack_register.bit_offset;
>> +
>> + apei_write(val, &gv2->read_ack_register);
>> +}
>> +
>> +static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
>> + int from_phys,
>> + enum fixed_addresses fixmap_idx)
>> +{
>> + void __iomem *vaddr;
>> + u64 offset;
>> + u32 trunk;
>> +
>> + while (len > 0) {
>> + offset = paddr - (paddr & PAGE_MASK);
>> + vaddr = ghes_map(PHYS_PFN(paddr), fixmap_idx);
>> + trunk = PAGE_SIZE - offset;
>> + trunk = min(trunk, len);
>> + if (from_phys)
>> + memcpy_fromio(buffer, vaddr + offset, trunk);
>> + else
>> + memcpy_toio(vaddr + offset, buffer, trunk);
>> + len -= trunk;
>> + paddr += trunk;
>> + buffer += trunk;
>> + ghes_unmap(vaddr, fixmap_idx);
>> + }
>> +}
>> +
>> +/* Check the top-level record header has an appropriate size. */
>> +int __ghes_check_estatus(struct ghes *ghes,
>> + struct acpi_hest_generic_status *estatus)
>> +{
>> + u32 len = cper_estatus_len(estatus);
>> + u32 max_len = min(ghes->generic->error_block_length,
>> + ghes->estatus_length);
>> +
>> + if (len < sizeof(*estatus)) {
>> + pr_warn_ratelimited(FW_WARN GHES_PFX "Truncated error status block!\n");
>> + return -EIO;
>> + }
>> +
>> + if (!len || len > max_len) {
>> + pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid error status block length!\n");
>> + return -EIO;
>> + }
>> +
>> + if (cper_estatus_check_header(estatus)) {
>> + pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid CPER header!\n");
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/* Read the CPER block, returning its address, and header in estatus. */
>> +int __ghes_peek_estatus(struct ghes *ghes,
>> + struct acpi_hest_generic_status *estatus,
>> + u64 *buf_paddr, enum fixed_addresses fixmap_idx)
>> +{
>> + struct acpi_hest_generic *g = ghes->generic;
>> + int rc;
>> +
>> + rc = apei_read(buf_paddr, &g->error_status_address);
>> + if (rc) {
>> + *buf_paddr = 0;
>> + pr_warn_ratelimited(FW_WARN GHES_PFX
>> +"Failed to read error status block address for hardware error source: %d.\n",
>> + g->header.source_id);
>> + return -EIO;
>> + }
>> + if (!*buf_paddr)
>> + return -ENOENT;
>> +
>> + ghes_copy_tofrom_phys(estatus, *buf_paddr, sizeof(*estatus), 1,
>> + fixmap_idx);
>> + if (!estatus->block_status) {
>> + *buf_paddr = 0;
>> + return -ENOENT;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int __ghes_read_estatus(struct acpi_hest_generic_status *estatus,
>> + u64 buf_paddr, enum fixed_addresses fixmap_idx,
>> + size_t buf_len)
>> +{
>> + ghes_copy_tofrom_phys(estatus, buf_paddr, buf_len, 1, fixmap_idx);
>> + if (cper_estatus_check(estatus)) {
>> + pr_warn_ratelimited(FW_WARN GHES_PFX
>> + "Failed to read error status block!\n");
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int ghes_read_estatus(struct ghes *ghes,
>> + struct acpi_hest_generic_status *estatus,
>> + u64 *buf_paddr, enum fixed_addresses fixmap_idx)
>> +{
>> + int rc;
>> +
>> + rc = __ghes_peek_estatus(ghes, estatus, buf_paddr, fixmap_idx);
>> + if (rc)
>> + return rc;
>> +
>> + rc = __ghes_check_estatus(ghes, estatus);
>> + if (rc)
>> + return rc;
>> +
>> + return __ghes_read_estatus(estatus, *buf_paddr, fixmap_idx,
>> + cper_estatus_len(estatus));
>> +}
>> +
>> +void ghes_clear_estatus(struct ghes *ghes,
>> + struct acpi_hest_generic_status *estatus,
>> + u64 buf_paddr, enum fixed_addresses fixmap_idx)
>> +{
>> + estatus->block_status = 0;
>> +
>> + if (!buf_paddr)
>> + return;
>> +
>> + ghes_copy_tofrom_phys(estatus, buf_paddr,
>> + sizeof(estatus->block_status), 0,
>> + fixmap_idx);
>> +
>> + /*
>> + * GHESv2 type HEST entries introduce support for error acknowledgment,
>> + * so only acknowledge the error if this support is present.
>> + */
>> + if (is_hest_type_generic_v2(ghes))
>> + ghes_ack_error(ghes->generic_v2);
>> +}
>> diff --git a/include/acpi/ghes_cper.h b/include/acpi/ghes_cper.h
>> index 2597fbadc4f3..2e3919f0c3e7 100644
>> --- a/include/acpi/ghes_cper.h
>> +++ b/include/acpi/ghes_cper.h
>> @@ -74,21 +74,21 @@ struct ghes_vendor_record_entry {
>> char vendor_record[];
>> };
>>
>
> ditto. "include/acpi/cper.h"
As above.
>> -static struct ghes *ghes_new(struct acpi_hest_generic *generic);
>> -static void ghes_fini(struct ghes *ghes);
>> +struct ghes *ghes_new(struct acpi_hest_generic *generic);
>> +void ghes_fini(struct ghes *ghes);
>>
>> -static int ghes_read_estatus(struct ghes *ghes,
>> +int ghes_read_estatus(struct ghes *ghes,
>> struct acpi_hest_generic_status *estatus,
>> u64 *buf_paddr, enum fixed_addresses fixmap_idx);
>> -static void ghes_clear_estatus(struct ghes *ghes,
>> +void ghes_clear_estatus(struct ghes *ghes,
>> struct acpi_hest_generic_status *estatus,
>> u64 buf_paddr, enum fixed_addresses fixmap_idx);
>> -static int __ghes_peek_estatus(struct ghes *ghes,
>> +int __ghes_peek_estatus(struct ghes *ghes,
>> struct acpi_hest_generic_status *estatus,
>> u64 *buf_paddr, enum fixed_addresses fixmap_idx);
>> -static int __ghes_check_estatus(struct ghes *ghes,
>> +int __ghes_check_estatus(struct ghes *ghes,
>> struct acpi_hest_generic_status *estatus);
>> -static int __ghes_read_estatus(struct acpi_hest_generic_status *estatus,
>> +int __ghes_read_estatus(struct acpi_hest_generic_status *estatus,
>> u64 buf_paddr, enum fixed_addresses fixmap_idx,
>> size_t buf_len);
>>
>>
>> --
>> 2.43.0
>>
>>
next prev parent reply other threads:[~2026-03-11 13:19 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-20 13:42 [PATCH v2 00/11] ACPI: APEI: share GHES CPER helpers and add DT FFH provider Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 01/11] ACPI: APEI: GHES: share macros via a private header Ahmed Tiba
2026-02-24 15:22 ` Jonathan Cameron
2026-03-11 11:39 ` Ahmed Tiba
2026-03-11 12:39 ` Jonathan Cameron
2026-03-11 12:56 ` Ahmed Tiba
2026-02-26 6:44 ` Himanshu Chauhan
2026-03-11 11:55 ` Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 02/11] ACPI: APEI: GHES: add ghes_cper.o stub Ahmed Tiba
2026-02-24 15:25 ` Jonathan Cameron
2026-03-11 12:19 ` Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 03/11] ACPI: APEI: GHES: move CPER read helpers Ahmed Tiba
2026-02-24 15:32 ` Jonathan Cameron
2026-03-11 12:38 ` Ahmed Tiba
2026-02-26 5:58 ` Himanshu Chauhan
2026-03-11 13:18 ` Ahmed Tiba [this message]
2026-02-20 13:42 ` [PATCH v2 04/11] ACPI: APEI: GHES: move GHESv2 ack and alloc helpers Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 05/11] ACPI: APEI: GHES: move estatus cache helpers Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 06/11] ACPI: APEI: GHES: move vendor record helpers Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 07/11] ACPI: APEI: GHES: move CXL CPER helpers Ahmed Tiba
2026-02-24 15:34 ` Jonathan Cameron
2026-02-20 13:42 ` [PATCH v2 08/11] ACPI: APEI: introduce GHES helper Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 09/11] ACPI: APEI: share GHES CPER helpers Ahmed Tiba
2026-02-20 19:19 ` kernel test robot
2026-02-20 19:24 ` kernel test robot
2026-02-20 20:37 ` kernel test robot
2026-02-20 21:16 ` kernel test robot
2026-02-20 13:42 ` [PATCH v2 10/11] dt-bindings: firmware: add arm,ras-ffh Ahmed Tiba
2026-02-26 7:03 ` Himanshu Chauhan
2026-03-11 13:41 ` Ahmed Tiba
2026-02-20 13:42 ` [PATCH v2 11/11] RAS: add DeviceTree firmware-first CPER provider Ahmed Tiba
2026-02-21 9:06 ` Krzysztof Kozlowski
2026-02-23 19:10 ` Ahmed Tiba
2026-02-24 15:55 ` Jonathan Cameron
2026-03-12 12:23 ` Ahmed Tiba
2026-03-12 14:50 ` Jonathan Cameron
2026-02-26 7:01 ` Himanshu Chauhan
2026-02-26 7:05 ` [PATCH v2 00/11] ACPI: APEI: share GHES CPER helpers and add DT FFH provider Himanshu Chauhan
2026-03-11 10:44 ` Ahmed Tiba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=df306958-bb99-4728-b461-e5ef1f731be2@arm.com \
--to=ahmed.tiba@arm.com \
--cc=Dmitry.Lamerov@arm.com \
--cc=Michael.Zhao2@arm.com \
--cc=bp@alien8.de \
--cc=catalin.marinas@arm.com \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=himanshu.chauhan@oss.qualcomm.com \
--cc=krzk+dt@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=tony.luck@intel.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.