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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/core: einj_types still missing on built-in RCH/RCD and redundant ACPI hook
--

commit 85011aad4fb602610b914a77c4e46997107304a4
Author: Yang Yongxin <yangyongxin@kylinos.cn>

cxl/core: Create einj_types when EINJ becomes available

This commit defers the creation of the CXL debugfs einj_types file to a
new helper, cxl_debugfs_einj_add_types(), called from cxl_core_init(),
cxl_acpi_probe(), and cxl_debugfs_create_dport_dir() so the file can be
created once EINJ is initialized.

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/

> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index b7ce6b59c680b..62dcb2b308430 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -968,6 +968,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();

[Severity: Medium]
Does this hook in cxl_acpi_probe() ever actually create the einj_types
file, and does it still leave RCH/RCD hierarchies without einj_types (and
without the dport einj_inject file) when cxl_acpi is built in?

Earlier in cxl_acpi_probe(), host bridge dports are registered via
bus_for_each_dev(..., add_host_bridge_dport):

drivers/cxl/acpi.c:add_host_bridge_dport() {
	...
	if (ctx.cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11) {
		dev_dbg(match, "RCRB found for UID %lld: %pa\n", ctx.uid,
			&ctx.base);
		dport = devm_cxl_add_rch_dport(root_port, bridge, ctx.uid,
					       ctx.base);
	} else {
		dport = devm_cxl_add_dport(root_port, bridge, ctx.uid,
					   CXL_RESOURCE_NONE);
	}
	...
}

Both devm_cxl_add_rch_dport() and devm_cxl_add_dport() call
__devm_cxl_add_dport() -> cxl_debugfs_create_dport_dir() ->
cxl_debugfs_einj_add_types().

When cxl_acpi is loaded as a module after einj_init() has completed,
add_host_bridge_dport() has already called cxl_debugfs_einj_add_types()
before execution reaches the direct cxl_debugfs_einj_add_types() call at
the end of cxl_acpi_probe().

Conversely, when both cxl_core and cxl_acpi are built in, cxl_acpi_init()
runs at subsys_initcall_sync level, before einj_init() runs at
device_initcall level. At that point, einj_cxl_is_initialized() returns
false during both add_host_bridge_dport() and the direct call in
cxl_acpi_probe().

Later, when an RCD endpoint probes after einj_init(),
devm_cxl_enumerate_ports() returns early without adding any dports:

drivers/cxl/core/port.c:devm_cxl_enumerate_ports() {
	...
	/*
	 * Skip intermediate port enumeration in the RCH case, there
	 * are no ports in between a host bridge and an endpoint.
	 */
	if (cxlmd->cxlds->rcd)
		return 0;
	...
}

And cxl_endpoint_port_probe() sets up RCH RAS without setting up EINJ
debugfs:

drivers/cxl/port.c:cxl_endpoint_port_probe() {
	...
	if (dport->rch)
		devm_cxl_dport_rch_ras_setup(dport);
	...
}

As a result, on built-in RCH/RCD configurations, wouldn't
cxl_debugfs_create_dport_dir() and cxl_debugfs_einj_add_types() never be
called after einj_init(), leaving both einj_types and the RCH dport's
einj_inject debugfs files missing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030032050.787682-1-yangyongxin@kylinos.cn?part=1