Re: [PATCH v3] cxl/core: Create einj_types when EINJ becomes available

["Cheatham, Benjamin" <benjamin.cheatham@amd.com>]

Hi Yang, thanks for walking through your reasoning in the last revision. It made looking through
the changes here much easier!

Nit-picky comments in line :)

On 10/29/2026 1:56 AM, Yongxin Yang wrote:
> [You don't often get email from yangyongxin@kylinos.cn. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> From: Yang Yongxin <yangyongxin@kylinos.cn>
> 
> The einj_types file under the CXL debugfs directory is never created.
> 

I think this should be "The einj_types file under the CXL debugfs directory
is never created when cxl_core is built-in", but I haven't looked in a while.

I remember you saying something about subsys_initcall() being downgraded to a
module_init() when cxl_core is built-in, and it looks like that's the case
after a quick look at the macro definition. So module dependencies should
cause the file creation to happen post EINJ init when both cxl_core and einj_cxl
are modules, in which case the file will be there.

That's probably why I never noticed this either, I (like most people) almost always
set everything CXL related to 'm' when compiling a kernel...
> cxl_core_init() runs at subsys_initcall level, while EINJ registers itself
> from einj_init() at device_initcall level. When cxl_core_init() evaluates
> einj_cxl_is_initialized() the built-in EINJ driver has not been initialized
> yet, so the value is always false and the debugfs_create_file("einj_types")
> call below it is never reached. Users have no way to discover which CXL
> error types the platform supports from debugfs.
> 
> Create the file from every point that can tell EINJ is up: cxl_core_init()
> covers cxl_core being a module loaded after EINJ has been initialized;
> cxl_acpi_probe() covers the same case for cxl_acpi and also systems where
> the CEDT is parsed but no dport is ever added; and the dport probe path

I would rather this not mention the case where no dport is ever added and
instead talk about the RCH/RCD case. If there's no dport ever added then
error injection isn't possible and this file is useless, so there's not
really a point in bringing it up.
> covers the built-in case, where the dport added from the memdev probe is
> the only hook reached after einj_init().
> 
> The dport probe path can run concurrently for different parent ports, each
> holding only its own device lock, so serialize the check and the create to
> avoid a duplicate debugfs_create_file() attempt that would only fail with
> -EEXIST. Reset the cached dentry in cxl_core_exit() so that reloading
> cxl_core starts from a clean state.
> 
> Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
> Cc: <stable@vger.kernel.org>
> Link: https://lore.kernel.org/all/de14c4ef-e3d8-432f-9ce8-87c468d6d2ad@amd.com/
> Signed-off-by: Yang Yongxin <yangyongxin@kylinos.cn>
> ---

The contents of the patch look good to me, so with the changes to the description
and comment below:

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

> Changes in v3:
> - Take the review from Ben: the helper is now cxl_debugfs_einj_add_types(),
>   non-static, declared in cxl.h and exported to cxl_acpi only, so that the
>   cxl_acpi hook can call it.
> - Add the cxl_acpi_probe() hook, before cxl_bus_rescan(), so the file is
>   created once the CXL topology exists instead of waiting for a dport.
> - Keep the dport path as a retry point. With EINJ built in, cxl_core_init()
>   and cxl_acpi_probe() both run before einj_init(), so dropping it would
>   still leave the file uncreated there.
> - Drop the "RCH is out of scope" paragraph. RCH does add a dport, from
>   add_host_bridge_dport(); the file is still not created there when EINJ is
>   built in, so do not claim the acpi hook fixes RCH.
> - Reset einj_types_dentry and cxl_debugfs in cxl_core_exit(), so that a
>   reloaded cxl_core does not keep a stale pointer and lose the file.
> - Use guard(mutex) instead of mutex_lock()/mutex_unlock().
> 
>  drivers/cxl/acpi.c      |  9 +++++++++
>  drivers/cxl/core/port.c | 41 +++++++++++++++++++++++++++++++++++------
>  drivers/cxl/cxl.h       |  2 ++
>  3 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 3b818adbd38b..0584245b30f2 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -958,6 +958,15 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>         if (rc < 0)
>                 return rc;
> 
> +       /*
> +        * Create einj_types from here as well, so the file also shows up when
> +        * cxl_acpi is a module loaded after EINJ, and on systems where the
> +        * CEDT is parsed but no dport is ever added (no CHBS for the host
> +        * bridge, or a CHBS without a valid component register base), which

Same thing as in the commit description, just mention the RCH/RCD case here instead.

Thanks,
Ben