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 3/5] ACPI: APEI: EINJ: Add debugfs files for EINJv2 support
Date: Tue, 14 May 2024 13:44:06 -0500 [thread overview]
Message-ID: <ZkOw9nPxf4xoAXXd@AUS-L1-JOHALLEN.amd.com> (raw)
In-Reply-To: <20240312212626.29007-4-zaidal@os.amperecomputing.com>
On Tue, Mar 12, 2024 at 02:26:24PM -0700, Zaid Alali wrote:
> EINJv2 enables users to inject errors to multiple components/
> devices at the same time. This commit creates a debugfs blob
Drop "This commit" and write this using imperative mood (as a command).
For example, "Create a debugfs blob file to be used for reading the user
input for the component array".
> file to be used for reading the user input for component array.
>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
> drivers/acpi/apei/einj.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
> index 119f7accd1c9..ceac53aa0d3f 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj.c
> @@ -101,6 +101,10 @@ 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;
> +
> /*
> * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
> * EINJ table through an unpublished extension. Use with caution as
> @@ -810,6 +814,8 @@ static int __init einj_init(void)
>
> einj_param = einj_get_parameter_address();
> if ((param_extension || acpi5) && einj_param) {
> + u32 error_type;
> +
> debugfs_create_x32("flags", S_IRUSR | S_IWUSR, einj_debug_dir,
> &error_flags);
> debugfs_create_x64("param1", S_IRUSR | S_IWUSR, einj_debug_dir,
> @@ -820,6 +826,25 @@ static int __init einj_init(void)
> &error_param3);
> debugfs_create_x64("param4", S_IRUSR | S_IWUSR, einj_debug_dir,
> &error_param4);
> +
> + rc = einj_get_available_error_type(&error_type, ACPI_EINJ_GET_ERROR_TYPE);
> + if (rc)
> + return rc;
> +
> + if (error_type & ACPI65_EINJV2_SUPP) {
> + debugfs_create_x64("einjv2_component_count", S_IRUSR | S_IWUSR,
> + einj_debug_dir, &component_count);
> + user_input = kzalloc(PAGE_SIZE, GFP_KERNEL);
What is the reason for a PAGE_SIZE allocation here?
I would guess that a typical user will only supply a couple entries in
the component array. If this is x86 and PAGE_SIZE is 4k, that's probably
fine, but IIUC, ARM can have up to 64k pages which seems like a lot more
than is needed here. I would think that since this is architecture
independent ACPI code, we want to avoid using something architecture
dependent like page size here anyway.
I also think that while 4k may be fine (and usually overkill) in most
cases, it's much smaller than the maximum possible number of entries.
While uncommon, we might want to allow for a larger allocation while
still keeping the default allocation small. Maybe a module parameter
could be used to allocate a bigger file if needed?
Thanks,
John
> + if (!user_input) {
> + rc = -ENOMEM;
> + goto err_release;
> + }
> + einjv2_component_arr.data = user_input;
> + einjv2_component_arr.size = PAGE_SIZE;
> + debugfs_create_blob("einjv2_component_array", S_IRUSR | S_IWUSR,
> + einj_debug_dir, &einjv2_component_arr);
> + }
> +
> debugfs_create_x32("notrigger", S_IRUSR | S_IWUSR,
> einj_debug_dir, ¬rigger);
> }
> @@ -871,6 +896,7 @@ static void __exit einj_exit(void)
> apei_resources_fini(&einj_resources);
> debugfs_remove_recursive(einj_debug_dir);
> acpi_put_table((struct acpi_table_header *)einj_tab);
> + kfree(user_input);
> }
>
> module_init(einj_init);
> --
> 2.34.1
>
next prev parent reply other threads:[~2024-05-14 18:44 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
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 [this message]
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=ZkOw9nPxf4xoAXXd@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).