From: Jonathan Cameron <jic23@kernel•org>
To: Ahmed Tiba <ahmed.tiba@arm•com>
Cc: will@kernel•org, xueshuai@linux•alibaba.com,
saket.dumbre@intel•com, mchehab@kernel•org, dave@stgolabs•net,
djbw@kernel•org, bp@alien8•de, tony.luck@intel•com,
guohanjun@huawei•com, lenb@kernel•org, skhan@linuxfoundation•org,
vishal.l.verma@intel•com, rafael@kernel•org, corbet@lwn•net,
ira.weiny@intel•com, dave.jiang@intel•com, krzk+dt@kernel•org,
robh@kernel•org, catalin.marinas@arm•com,
alison.schofield@intel•com, conor+dt@kernel•org,
linux-arm-kernel@lists•infradead.org, Michael.Zhao2@arm•com,
linux-doc@vger•kernel.org, linux-kernel@vger•kernel.org,
linux-cxl@vger•kernel.org, Dmitry.Lamerov@arm•com,
devicetree@vger•kernel.org, linux-acpi@vger•kernel.org,
linux-edac@vger•kernel.org, acpica-devel@lists•linux.dev
Subject: Re: [PATCH v5 02/10] ACPI: APEI: GHES: move CPER read helpers
Date: Fri, 29 May 2026 16:51:37 +0100 [thread overview]
Message-ID: <20260529165137.0cd56e6e@jic23-huawei> (raw)
In-Reply-To: <20260529-topics-ahmtib01-ras_ffh_arm_internal_review-v5-2-2e0500d42642@arm.com>
On Fri, 29 May 2026 10:50:42 +0100
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>
A couple of things I noticed whilst quickly scan through what was moved inline.
Move itself looks fine to me.
Reviewed-by: Jonathan Cameron <jic23@kernel•org>
> diff --git a/drivers/acpi/apei/ghes_cper.c b/drivers/acpi/apei/ghes_cper.c
> new file mode 100644
> index 000000000000..7bb72fe57838
> --- /dev/null
> +++ b/drivers/acpi/apei/ghes_cper.c
> @@ -0,0 +1,195 @@
> +
> +/* 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) {
Obviously this is just a move, but that if !len is silly given that will have failed
the previous check as 0 is definitely less than sizeof(*estatus)
Maybe add a trivial patch to tidy this up and any similar oddities.
> + 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);
Whilst you are here I don't think anyone would mind if you fix the alignment.
Looks like it's one space short.
> + 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;
> +}
next prev parent reply other threads:[~2026-05-29 15:53 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-29 9:50 [PATCH v5 00/10] ACPI: APEI: share GHES CPER helpers and add DT FFH provider Ahmed Tiba
2026-05-29 9:50 ` [PATCH v5 01/10] ACPI: APEI: GHES: share macros via a private header Ahmed Tiba
2026-05-29 15:52 ` Jonathan Cameron
2026-06-01 22:46 ` Borislav Petkov
2026-05-29 9:50 ` [PATCH v5 02/10] ACPI: APEI: GHES: move CPER read helpers Ahmed Tiba
2026-05-29 15:51 ` Jonathan Cameron [this message]
2026-05-29 9:50 ` [PATCH v5 03/10] ACPI: APEI: GHES: move GHESv2 ack and alloc helpers Ahmed Tiba
2026-05-29 15:54 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 04/10] ACPI: APEI: GHES: move estatus cache helpers Ahmed Tiba
2026-05-29 16:03 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 05/10] ACPI: APEI: GHES: move vendor record helpers Ahmed Tiba
2026-05-29 16:10 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 06/10] ACPI: APEI: GHES: move CXL CPER helpers Ahmed Tiba
2026-05-29 16:16 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 07/10] ACPI: APEI: introduce GHES helper Ahmed Tiba
2026-05-29 16:21 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 08/10] ACPI: APEI: share GHES CPER helpers Ahmed Tiba
2026-05-29 16:32 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 09/10] dt-bindings: firmware: add arm,ras-cper Ahmed Tiba
2026-05-29 16:44 ` Jonathan Cameron
2026-05-29 9:50 ` [PATCH v5 10/10] RAS: add firmware-first CPER provider Ahmed Tiba
2026-05-29 17:06 ` Jonathan Cameron
2026-05-29 16:36 ` [PATCH v5 00/10] ACPI: APEI: share GHES CPER helpers and add DT FFH provider Jonathan Cameron
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=20260529165137.0cd56e6e@jic23-huawei \
--to=jic23@kernel$(echo .)org \
--cc=Dmitry.Lamerov@arm$(echo .)com \
--cc=Michael.Zhao2@arm$(echo .)com \
--cc=acpica-devel@lists$(echo .)linux.dev \
--cc=ahmed.tiba@arm$(echo .)com \
--cc=alison.schofield@intel$(echo .)com \
--cc=bp@alien8$(echo .)de \
--cc=catalin.marinas@arm$(echo .)com \
--cc=conor+dt@kernel$(echo .)org \
--cc=corbet@lwn$(echo .)net \
--cc=dave.jiang@intel$(echo .)com \
--cc=dave@stgolabs$(echo .)net \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=djbw@kernel$(echo .)org \
--cc=guohanjun@huawei$(echo .)com \
--cc=ira.weiny@intel$(echo .)com \
--cc=krzk+dt@kernel$(echo .)org \
--cc=lenb@kernel$(echo .)org \
--cc=linux-acpi@vger$(echo .)kernel.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-cxl@vger$(echo .)kernel.org \
--cc=linux-doc@vger$(echo .)kernel.org \
--cc=linux-edac@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=mchehab@kernel$(echo .)org \
--cc=rafael@kernel$(echo .)org \
--cc=robh@kernel$(echo .)org \
--cc=saket.dumbre@intel$(echo .)com \
--cc=skhan@linuxfoundation$(echo .)org \
--cc=tony.luck@intel$(echo .)com \
--cc=vishal.l.verma@intel$(echo .)com \
--cc=will@kernel$(echo .)org \
--cc=xueshuai@linux$(echo .)alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox