Re: [PATCH 5/7] cxl/region: Ensure endpoint is valid in cxl_dpa_to_region()

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

On 3/10/26 8:57 AM, Li Ming wrote:
> cxl_dpa_to_region() needs to access the endpoint of the given CXL memdev
> to confirm whether the CXL memdev has memory range attached to a CXL
> region. But it is possible to be called before endpoint allocation or
> after the CXL memdev probing failure.  In these two cases,
> cxlmd->endpoint is invalid.
> 
> To solve the problem, cxl_dpa_to_region() requires that callers have to
> hold the given CXL memdev lock, it guarantees the CXL memdev probing has
> completed. And cxl_dpa_to_region() will check the given CXL memdev
> driver binding status, if the memdev has bound to the driver, the
> endpoint is guaranteed to be valid. This checking also requires the CXL
> memdev lock held.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

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


> ---
>  drivers/cxl/core/core.h   |  4 ++--
>  drivers/cxl/core/region.c | 11 +++++++----
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 5b0570df0fd9..fa8402be860f 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -47,7 +47,7 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
>  int cxl_region_init(void);
>  void cxl_region_exit(void);
>  int cxl_get_poison_by_endpoint(struct cxl_port *port);
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
> +struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa);
>  
> @@ -58,7 +58,7 @@ static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
>  	return ULLONG_MAX;
>  }
>  static inline
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> +struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
>  {
>  	return NULL;
>  }
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 806512dfab07..c555e3ae8b62 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2947,16 +2947,19 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
>  	return 1;
>  }
>  
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> +struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
>  {
>  	struct cxl_dpa_to_region_context ctx;
> -	struct cxl_port *port;
> +	struct cxl_port *port = cxlmd->endpoint;
> +
> +	device_lock_assert(&cxlmd->dev);
> +	if (!cxlmd->dev.driver)
> +		return NULL;
>  
>  	ctx = (struct cxl_dpa_to_region_context) {
>  		.dpa = dpa,
>  	};
> -	port = cxlmd->endpoint;
> -	if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
> +	if (cxl_num_decoders_committed(port))
>  		device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
>  
>  	return ctx.cxlr;
>