| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZzvAMWuOvw4j4C5n@AUSJOHALLEN.amd.com>
Date: Mon, 18 Nov 2024 16:31:13 -0600
From: John Allen <john.allen@....com>
To: Zaid Alali <zaidal@...amperecomputing.com>
Cc: rafael@...nel.org, lenb@...nel.org, james.morse@....com,
tony.luck@...el.com, bp@...en8.de, robert.moore@...el.com,
dan.j.williams@...el.com, Jonathan.Cameron@...wei.com,
Benjamin.Cheatham@....com, Avadhut.Naik@....com,
viro@...iv.linux.org.uk, arnd@...db.de, ira.weiny@...el.com,
dave.jiang@...el.com, sthanneeru.opensrc@...ron.com,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
acpica-devel@...ts.linux.dev
Subject: Re: [PATCH 4/8] ACPI: APEI: EINJ: Enable the discovery of EINJv2
capabilities
On Tue, Oct 22, 2024 at 02:34:25PM -0700, Zaid Alali wrote:
> Enable the driver to show all supported error injections for EINJ
> and EINJv2 at the same time. EINJv2 capabilities can be discovered
> by checking the return value of get_error_type, where bit 30 set
> indicates EINJv2 support.
>
> Signed-off-by: Zaid Alali <zaidal@...amperecomputing.com>
> ---
> drivers/acpi/apei/apei-internal.h | 2 +-
> drivers/acpi/apei/einj-core.c | 33 +++++++++++++++++++++++--------
> drivers/acpi/apei/einj-cxl.c | 2 +-
> 3 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
> index cd2766c69d78..9a3dbaeed39a 100644
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -131,7 +131,7 @@ static inline u32 cper_estatus_len(struct acpi_hest_generic_status *estatus)
>
> int apei_osc_setup(void);
>
> -int einj_get_available_error_type(u32 *type);
> +int einj_get_available_error_type(u32 *type, int version);
> int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> u64 param4);
> int einj_cxl_rch_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 78c5a20115eb..3621f071a735 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -33,6 +33,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 | \
> @@ -84,6 +85,7 @@ static struct debugfs_blob_wrapper vendor_errors;
> static char vendor_dev[64];
>
> static u32 available_error_type;
> +static u32 available_error_type_v2;
>
> /*
> * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
> @@ -159,13 +161,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);
> @@ -174,12 +176,12 @@ static int __einj_get_available_error_type(u32 *type)
> }
>
> /* Get error injection capabilities of the platform */
> -int einj_get_available_error_type(u32 *type)
> +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;
> @@ -647,6 +649,11 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
> { BIT(11), "Platform 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)
> {
> @@ -654,8 +661,13 @@ static int available_error_type_show(struct seq_file *m, void *v)
> for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
> if (available_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);
> -
> + einj_error_type_string[pos].str);
This looks like it may have been unintentional. Alignment of the above
line should be left alone and the newline kept.
> + if (available_error_type & ACPI65_EINJV2_SUPP) {
> + for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++)
> + if (available_error_type_v2 & 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);
Similarly, the above line should be aligned as the other one was
previously:
seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
einjv2_error_type_string[pos].str);
> + }
> return 0;
> }
>
> @@ -692,7 +704,7 @@ int einj_validate_error_type(u64 type)
> if (tval & (tval - 1))
> return -EINVAL;
> if (!vendor)
> - if (!(type & available_error_type))
> + if (!(type & (available_error_type)))
Why are these extra parens being added? Is there a reason for this?
> return -EINVAL;
>
> return 0;
> @@ -769,9 +781,14 @@ static int __init einj_probe(struct platform_device *pdev)
> goto err_put_table;
> }
>
> - 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 (available_error_type & ACPI65_EINJV2_SUPP) {
> + rc = einj_get_available_error_type(&available_error_type_v2, ACPI_EINJV2_GET_ERROR_TYPE);
checkpatch.pl complains about the above line being longer the 100 chars.
This should be broken up.
Thanks,
John
> + if (rc)
> + return rc;
> + }
>
> rc = -ENOMEM;
> einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
> diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
> index a4e709937236..5ffc4a162c70 100644
> --- a/drivers/acpi/apei/einj-cxl.c
> +++ b/drivers/acpi/apei/einj-cxl.c
> @@ -30,7 +30,7 @@ int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
> int cxl_err, rc;
> u32 available_error_type = 0;
>
> - 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;
>
> --
> 2.34.1
>
Powered by blists - more mailing lists