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

[Dave Jiang <dave.jiang@intel.com>]

On 10/29/26 8:20 PM, Yongxin Yang wrote:
> From: Yang Yongxin <yangyongxin@kylinos.cn>
> 
> The einj_types file under the CXL debugfs directory is never created when
> cxl_core is built in.
> 
> cxl_core_init() runs at subsys_initcall level, while EINJ registers itself
> from einj_init() at device_initcall level. With cxl_core built in, the
> einj_cxl_is_initialized() check in cxl_core_init() therefore always sees
> false and the debugfs_create_file("einj_types") call below it is never
> reached, so users have no way to discover which CXL error types the
> platform supports from debugfs. When cxl_core is a module, module
> dependencies make sure it is loaded after EINJ is up, and the file is
> created.
> 
> 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. The
> cxl_acpi_probe() hook covers cxl_core being built in while cxl_acpi is a
> module, and is the only hook that can work on RCH/RCD hierarchies, where
> the single dport is added by the ACPI probe itself, too early to help. The
> dport probe path covers the built-in virtual host 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/
> Link: https://lore.kernel.org/all/7a22cc6a-deba-40e6-8033-97d1234420ba@amd.com/
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Yang Yongxin <yangyongxin@kylinos.cn>

Couple more things. Can you send a new email per revision instead of reply-to the previous post? Otherwise it's a mess of email threads.

Also, please update email addresses in your submission. Many of them have changed and are now bouncing. Check the latest MAINTAINERS please.

DJ

> ---
> Changes in v4:
> - Scope the problem statement to the built-in case: the file is only never
>   created when cxl_core is built in, the module dependencies take care of the
>   ordering otherwise.
> - Replace the "no dport is ever added" rationale with the RCH/RCD case, in
>   the changelog and in the cxl_acpi_probe() comment: a system without any
>   dport cannot do error injection at all, so the file is useless there. On
>   RCH/RCD hierarchies the only dport comes from the ACPI probe, so this hook
>   is the one that matters.
> 
>  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..ca7741203740 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: with cxl_core built in,
> +	 * cxl_core_init() runs before EINJ is up, so when cxl_acpi is a module
> +	 * loaded afterwards this is the only hook that can create the file,
> +	 * including on RCH/RCD hierarchies, where the single dport is added by
> +	 * this probe itself and cannot help.
> +	 */
> +	cxl_debugfs_einj_add_types();
> +
>  	/* In case PCI is scanned before ACPI re-trigger memdev attach */
>  	cxl_bus_rescan();
>  	return 0;
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 625e4aa427db..76ac612b514c 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -802,6 +802,35 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
>  
>  DEFINE_SHOW_ATTRIBUTE(einj_cxl_available_error_type);
>  
> +static struct dentry *cxl_debugfs;
> +static struct dentry *einj_types_dentry;
> +
> +/*
> + * einj_types can only be created once EINJ is up, and no single hook is
> + * reached after that point in every configuration:
> + *
> + *  - cxl_core_init() runs at subsys_initcall level and cxl_acpi_probe() at
> + *    subsys_initcall_sync level, so both create the file when cxl_core and
> + *    cxl_acpi are modules loaded after EINJ has been initialized;
> + *  - when EINJ is built in, einj_init() runs at device_initcall level, so
> + *    both of them are too early and the only hook left is the dport added
> + *    from the memdev probe.
> + *
> + * The dport probe path can run concurrently for different parent ports, each
> + * holding only its own device lock, so the check and the create need to be
> + * serialized to avoid a duplicate debugfs_create_file() attempt.
> + */
> +static DEFINE_MUTEX(einj_types_lock);
> +void cxl_debugfs_einj_add_types(void)
> +{
> +	guard(mutex)(&einj_types_lock);
> +	if (cxl_debugfs && !einj_types_dentry && einj_cxl_is_initialized())
> +		einj_types_dentry = debugfs_create_file("einj_types", 0400,
> +							cxl_debugfs, NULL,
> +							&einj_cxl_available_error_type_fops);
> +}
> +EXPORT_SYMBOL_FOR_MODULES(cxl_debugfs_einj_add_types, "cxl_acpi");
> +
>  static int cxl_einj_inject(void *data, u64 type)
>  {
>  	struct cxl_dport *dport = data;
> @@ -822,6 +851,8 @@ static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>  	if (!einj_cxl_is_initialized())
>  		return;
>  
> +	cxl_debugfs_einj_add_types();
> +
>  	/*
>  	 * Protocol error injection is only available for CXL 2.0+ root ports
>  	 * and CXL 1.1 downstream ports
> @@ -2501,8 +2532,6 @@ const struct bus_type cxl_bus_type = {
>  };
>  EXPORT_SYMBOL_NS_GPL(cxl_bus_type, "CXL");
>  
> -static struct dentry *cxl_debugfs;
> -
>  struct dentry *cxl_debugfs_create_dir(const char *dir)
>  {
>  	return debugfs_create_dir(dir, cxl_debugfs);
> @@ -2515,10 +2544,6 @@ static __init int cxl_core_init(void)
>  
>  	cxl_debugfs = debugfs_create_dir("cxl", NULL);
>  
> -	if (einj_cxl_is_initialized())
> -		debugfs_create_file("einj_types", 0400, cxl_debugfs, NULL,
> -				    &einj_cxl_available_error_type_fops);
> -
>  	cxl_mbox_init();
>  
>  	rc = cxl_memdev_init();
> @@ -2541,6 +2566,8 @@ static __init int cxl_core_init(void)
>  
>  	cxl_ras_init();
>  
> +	cxl_debugfs_einj_add_types();
> +
>  	return 0;
>  
>  err_region:
> @@ -2560,6 +2587,8 @@ static void cxl_core_exit(void)
>  	destroy_workqueue(cxl_bus_wq);
>  	cxl_memdev_exit();
>  	debugfs_remove_recursive(cxl_debugfs);
> +	cxl_debugfs = NULL;
> +	einj_types_dentry = NULL;
>  }
>  
>  subsys_initcall(cxl_core_init);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index cab8ce39f465..b9245261ee39 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -926,4 +926,6 @@ struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
>  
>  u16 cxl_gpf_get_dvsec(struct device *dev);
>  
> +void cxl_debugfs_einj_add_types(void);
> +
>  #endif /* __CXL_H__ */