linux-kernel - Re: [PATCH v3 7/9] ACPI: APEI: EINJ: Add debugfs files for EINJv2 support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250212165440.0000728b@huawei.com>
Date: Wed, 12 Feb 2025 16:54:40 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.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>, <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 v3 7/9] ACPI: APEI: EINJ: Add debugfs files for EINJv2
 support

On Mon, 10 Feb 2025 10:37:03 -0800
Zaid Alali <zaidal@...amperecomputing.com> wrote:

> Create a debugfs blob file to be used for reading the user
> input for the component array. EINJv2 enables users to inject
> errors to multiple components/devices at the same time using
> component array.
> 
> Signed-off-by: Zaid Alali <zaidal@...amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index c604aa875644..40ebdbc4961f 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 COMP_ARR_SIZE		1024
>  #define ACPI65_EINJV2_SUPP	BIT(30)
>  #define ACPI5_VENDOR_BIT	BIT(31)
>  #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
> @@ -107,6 +108,9 @@ static struct debugfs_blob_wrapper vendor_blob;
>  static struct debugfs_blob_wrapper vendor_errors;
>  static char vendor_dev[64];
>  
> +static struct debugfs_blob_wrapper einjv2_component_arr;
> +static u64 component_count;
> +static void *user_input;
>  static u32 available_error_type;
>  static u32 available_error_type_v2;
>  
> @@ -892,6 +896,19 @@ static int __init einj_probe(struct platform_device *pdev)
>  				   &error_param4);
>  		debugfs_create_x32("notrigger", S_IRUSR | S_IWUSR,
>  				   einj_debug_dir, &notrigger);
> +		if (available_error_type & ACPI65_EINJV2_SUPP) {
> +			debugfs_create_x64("einjv2_component_count", S_IRUSR | S_IWUSR,
> +					einj_debug_dir,	&component_count);
> +			user_input = kzalloc(COMP_ARR_SIZE, GFP_KERNEL);
> +			if (!user_input) {
> +				rc = -ENOMEM;
> +				goto err_release;
I think you need a new label as need to undo a few more things.
At least call apei_exec_post_unmap_gars() probably given it is called
in the remove function.

> +			}
> +			einjv2_component_arr.data = user_input;
> +			einjv2_component_arr.size = COMP_ARR_SIZE;
> +			debugfs_create_blob("einjv2_component_array", S_IRUSR | S_IWUSR,
> +					einj_debug_dir, &einjv2_component_arr);
> +		}
>  	}
>  
>  	if (vendor_dev[0]) {
> @@ -941,6 +958,7 @@ static void __exit einj_remove(struct platform_device *pdev)
>  	apei_resources_fini(&einj_resources);
>  	debugfs_remove_recursive(einj_debug_dir);
>  	acpi_put_table((struct acpi_table_header *)einj_tab);
> +	kfree(user_input);
Should be place din equivalent location of where you allocate it in probe.
So probably before einj_exec_ctx_init()

Whilst it may make no actual difference a clean reverse order
is much easier to review as any exception can be easily spotted
and typically should have a comment on why the order is different.

>  }
>  
>  static struct platform_device *einj_dev;