linux-kernel - Re: [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint attach

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: <8018c7bf-87c7-4fb1-8398-15436ab0e5fa@intel.com>
Date: Thu, 4 Dec 2025 11:36:14 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
 Smita.KoralahalliChannabasappa@....com, alison.schofield@...el.com,
 terry.bowman@....com, alejandro.lucero-palau@....com,
 linux-pci@...r.kernel.org, Jonathan.Cameron@...wei.com,
 Alejandro Lucero <alucerop@....com>
Subject: Re: [PATCH 3/6] cxl/port: Arrange for always synchronous endpoint
 attach



On 12/3/25 7:21 PM, Dan Williams wrote:
> Make it so that upon return from devm_cxl_add_endpoint() that
> cxl_mem_probe() can assume that the endpoint has had a chance to complete
> cxl_port_probe().  I.e. cxl_port module loading has completed prior to
> device registration.
> 
> Delete the MODULE_SOFTDEP() as it is not sufficient for this purpose, but a
> hard link-time dependency is reliable. Specifically MODULE_SOFTDEP() does
> not guarantee that the module loading has completed prior to the completion
> of the current module's init.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
> Cc: Alejandro Lucero <alucerop@....com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>

Reviewed-by: Dave Jiang <dave.jiang@...el.com>

> ---
>  drivers/cxl/cxl.h  |  2 ++
>  drivers/cxl/mem.c  | 43 -------------------------------------------
>  drivers/cxl/port.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index ba17fa86d249..c796c3db36e0 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -780,6 +780,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>  				   struct cxl_dport *parent_dport);
>  struct cxl_root *devm_cxl_add_root(struct device *host,
>  				   const struct cxl_root_ops *ops);
> +int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> +			  struct cxl_dport *parent_dport);
>  struct cxl_root *find_cxl_root(struct cxl_port *port);
>  
>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 55883797ab2d..d62931526fd4 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -45,44 +45,6 @@ static int cxl_mem_dpa_show(struct seq_file *file, void *data)
>  	return 0;
>  }
>  
> -static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> -				 struct cxl_dport *parent_dport)
> -{
> -	struct cxl_port *parent_port = parent_dport->port;
> -	struct cxl_port *endpoint, *iter, *down;
> -	int rc;
> -
> -	/*
> -	 * Now that the path to the root is established record all the
> -	 * intervening ports in the chain.
> -	 */
> -	for (iter = parent_port, down = NULL; !is_cxl_root(iter);
> -	     down = iter, iter = to_cxl_port(iter->dev.parent)) {
> -		struct cxl_ep *ep;
> -
> -		ep = cxl_ep_load(iter, cxlmd);
> -		ep->next = down;
> -	}
> -
> -	/* Note: endpoint port component registers are derived from @cxlds */
> -	endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
> -				     parent_dport);
> -	if (IS_ERR(endpoint))
> -		return PTR_ERR(endpoint);
> -
> -	rc = cxl_endpoint_autoremove(cxlmd, endpoint);
> -	if (rc)
> -		return rc;
> -
> -	if (!endpoint->dev.driver) {
> -		dev_err(&cxlmd->dev, "%s failed probe\n",
> -			dev_name(&endpoint->dev));
> -		return -ENXIO;
> -	}
> -
> -	return 0;
> -}
> -
>  static int cxl_debugfs_poison_inject(void *data, u64 dpa)
>  {
>  	struct cxl_memdev *cxlmd = data;
> @@ -275,8 +237,3 @@ MODULE_DESCRIPTION("CXL: Memory Expansion");
>  MODULE_LICENSE("GPL v2");
>  MODULE_IMPORT_NS("CXL");
>  MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
> -/*
> - * create_endpoint() wants to validate port driver attach immediately after
> - * endpoint registration.
> - */
> -MODULE_SOFTDEP("pre: cxl_port");
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 51c8f2f84717..7937e7e53797 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -156,10 +156,50 @@ static struct cxl_driver cxl_port_driver = {
>  	.probe = cxl_port_probe,
>  	.id = CXL_DEVICE_PORT,
>  	.drv = {
> +		.probe_type = PROBE_FORCE_SYNCHRONOUS,
>  		.dev_groups = cxl_port_attribute_groups,
>  	},
>  };
>  
> +int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
> +			  struct cxl_dport *parent_dport)
> +{
> +	struct cxl_port *parent_port = parent_dport->port;
> +	struct cxl_port *endpoint, *iter, *down;
> +	int rc;
> +
> +	/*
> +	 * Now that the path to the root is established record all the
> +	 * intervening ports in the chain.
> +	 */
> +	for (iter = parent_port, down = NULL; !is_cxl_root(iter);
> +	     down = iter, iter = to_cxl_port(iter->dev.parent)) {
> +		struct cxl_ep *ep;
> +
> +		ep = cxl_ep_load(iter, cxlmd);
> +		ep->next = down;
> +	}
> +
> +	/* Note: endpoint port component registers are derived from @cxlds */
> +	endpoint = devm_cxl_add_port(host, &cxlmd->dev, CXL_RESOURCE_NONE,
> +				     parent_dport);
> +	if (IS_ERR(endpoint))
> +		return PTR_ERR(endpoint);
> +
> +	rc = cxl_endpoint_autoremove(cxlmd, endpoint);
> +	if (rc)
> +		return rc;
> +
> +	if (!endpoint->dev.driver) {
> +		dev_err(&cxlmd->dev, "%s failed probe\n",
> +			dev_name(&endpoint->dev));
> +		return -ENXIO;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(devm_cxl_add_endpoint, "cxl_mem");
> +
>  static int __init cxl_port_init(void)
>  {
>  	return cxl_driver_register(&cxl_port_driver);