From: John Allen <john.allen@amd.com>
To: Zaid Alali <zaidal@os.amperecomputing.com>
Cc: lenb@kernel.org, james.morse@arm.com, tony.luck@intel.com,
bp@alien8.de, robert.moore@intel.com, Avadhut.Naik@amd.com,
xueshuai@linux.alibaba.com, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, acpica-devel@lists.linux.dev
Subject: Re: [RFC PATCH 1/5] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities
Date: Tue, 14 May 2024 14:10:45 -0500 [thread overview]
Message-ID: <ZkO3NRUV7Kjrf/VS@AUS-L1-JOHALLEN.amd.com> (raw)
In-Reply-To: <20240312212626.29007-2-zaidal@os.amperecomputing.com>
On Tue, Mar 12, 2024 at 02:26:22PM -0700, Zaid Alali wrote:
> EINJv2 capabilities can be discovered by checking the return value
> of get_error_type, where bit 30 set indicates EINJv2 support. This
> commit enables the driver to show all supported error injections
Drop "This commit" and write this using imperative mood (as a command).
For this one, "Enable the driver to show all supported error injections
for EINJ and EINJv2 at the same time".
> for EINJ and EINJv2 at the same time.
>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
> drivers/acpi/apei/einj.c | 37 ++++++++++++++++++++++++++++++-------
> include/acpi/actbl1.h | 1 +
> 2 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
> index 89fb9331c611..90efbcbf6b54 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj.c
> @@ -32,6 +32,7 @@
> #define SLEEP_UNIT_MAX 5000 /* 5ms */
> /* Firmware should respond within 1 seconds */
> #define FIRMWARE_TIMEOUT (1 * USEC_PER_SEC)
> +#define ACPI65_EINJV2_SUPP BIT(30)
> #define ACPI5_VENDOR_BIT BIT(31)
> #define MEM_ERROR_MASK (ACPI_EINJ_MEMORY_CORRECTABLE | \
> ACPI_EINJ_MEMORY_UNCORRECTABLE | \
> @@ -145,13 +146,13 @@ static void einj_exec_ctx_init(struct apei_exec_context *ctx)
> EINJ_TAB_ENTRY(einj_tab), einj_tab->entries);
> }
>
> -static int __einj_get_available_error_type(u32 *type)
> +static int __einj_get_available_error_type(u32 *type, int version)
> {
> struct apei_exec_context ctx;
> int rc;
>
> einj_exec_ctx_init(&ctx);
> - rc = apei_exec_run(&ctx, ACPI_EINJ_GET_ERROR_TYPE);
> + rc = apei_exec_run(&ctx, version);
> if (rc)
> return rc;
> *type = apei_exec_ctx_get_output(&ctx);
> @@ -160,12 +161,12 @@ static int __einj_get_available_error_type(u32 *type)
> }
>
> /* Get error injection capabilities of the platform */
> -static int einj_get_available_error_type(u32 *type)
> +static int einj_get_available_error_type(u32 *type, int version)
> {
> int rc;
>
> mutex_lock(&einj_mutex);
> - rc = __einj_get_available_error_type(type);
> + rc = __einj_get_available_error_type(type, version);
> mutex_unlock(&einj_mutex);
>
> return rc;
> @@ -221,9 +222,14 @@ static void check_vendor_extension(u64 paddr,
>
> static void *einj_get_parameter_address(void)
> {
> - int i;
> + int i, rc;
> u64 pa_v4 = 0, pa_v5 = 0;
> struct acpi_whea_header *entry;
> + u32 error_type = 0;
> +
> + rc = einj_get_available_error_type(&error_type, ACPI_EINJ_GET_ERROR_TYPE);
> + if (rc)
> + return NULL;
>
> entry = EINJ_TAB_ENTRY(einj_tab);
> for (i = 0; i < einj_tab->entries; i++) {
> @@ -615,19 +621,35 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
> { BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
> { BIT(31), "Vendor Defined Error Types" },
> };
> +static struct { u32 mask; const char *str; } const einjv2_error_type_string[] = {
> + { BIT(0), "EINJV2 Processor Error" },
> + { BIT(1), "EINJV2 Memory Error" },
> + { BIT(2), "EINJV2 PCI Express Error" },
> +};
>
> static int available_error_type_show(struct seq_file *m, void *v)
> {
> int rc;
> u32 error_type = 0;
>
> - rc = einj_get_available_error_type(&error_type);
> + rc = einj_get_available_error_type(&error_type, ACPI_EINJ_GET_ERROR_TYPE);
> if (rc)
> return rc;
> for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
> if (error_type & einj_error_type_string[pos].mask)
> seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
> einj_error_type_string[pos].str);
> + if (error_type & ACPI65_EINJV2_SUPP) {
> + rc = einj_get_available_error_type(&error_type, ACPI_EINJV2_GET_ERROR_TYPE);
> + if (rc)
> + return rc;
> + seq_printf(m, "====================\n");
Seems like if we're going to visually split the EINJ and EINJv2 cases,
rather than just splitting them with the above line, it might be better
to be more descriptive. For example:
# cat available_error_type
EINJ error types:
0x00000002 Processor Uncorrectable non-fatal
0x00000008 Memory Correctable
0x00000010 Memory Uncorrectable non-fatal
EINJv2 error types:
0x00000001 EINJV2 Processor Error
0x00000002 EINJV2 Memory Error
> + for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++)
> + if (error_type & einjv2_error_type_string[pos].mask)
> + seq_printf(m, "0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
> + einjv2_error_type_string[pos].str);
When you break a long function call like this, the second line should
align below the first character after the parenthesis. For example:
seq_printf(m, "0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
einjv2_error_type_string[pos].str);
There are a few other places in the series where alignment should be
fixed in this way as well.
Thanks,
John
> +
> + }
>
> return 0;
> }
> @@ -662,7 +684,8 @@ static int error_type_set(void *data, u64 val)
> if (tval & (tval - 1))
> return -EINVAL;
> if (!vendor) {
> - rc = einj_get_available_error_type(&available_error_type);
> + rc = einj_get_available_error_type(&available_error_type,
> + ACPI_EINJ_GET_ERROR_TYPE);
> if (rc)
> return rc;
> if (!(val & available_error_type))
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index a33375e055ad..a07d564b0590 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -1030,6 +1030,7 @@ enum acpi_einj_actions {
> ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
> ACPI_EINJ_GET_EXECUTE_TIMINGS = 9,
> ACPI_EINJ_ACTION_RESERVED = 10, /* 10 and greater are reserved */
> + ACPI_EINJV2_GET_ERROR_TYPE = 0x11,
> ACPI_EINJ_TRIGGER_ERROR = 0xFF /* Except for this value */
> };
>
> --
> 2.34.1
>
next prev parent reply other threads:[~2024-05-14 19:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-12 21:26 [RFC PATCH 0/5] Enable EINJv2 Support Zaid Alali
2024-03-12 21:26 ` [RFC PATCH 1/5] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities Zaid Alali
2024-05-14 19:10 ` John Allen [this message]
2024-03-12 21:26 ` [RFC PATCH 2/5] ACPI: APEI: EINJ: Add einjv2 extension struct Zaid Alali
2024-03-12 21:26 ` [RFC PATCH 3/5] ACPI: APEI: EINJ: Add debugfs files for EINJv2 support Zaid Alali
2024-05-14 18:44 ` John Allen
2024-03-12 21:26 ` [RFC PATCH 4/5] ACPI: APEI: EINJ: Enable EINJv2 error injections Zaid Alali
2024-05-14 19:34 ` John Allen
2024-03-12 21:26 ` [RFC PATCH 5/5] ACPI: APEI: EINJ: Update the documentation for EINJv2 support Zaid Alali
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=ZkO3NRUV7Kjrf/VS@AUS-L1-JOHALLEN.amd.com \
--to=john.allen@amd.com \
--cc=Avadhut.Naik@amd.com \
--cc=acpica-devel@lists.linux.dev \
--cc=bp@alien8.de \
--cc=james.morse@arm.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robert.moore@intel.com \
--cc=tony.luck@intel.com \
--cc=xueshuai@linux.alibaba.com \
--cc=zaidal@os.amperecomputing.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).