Re: [PATCH 5/8] cxl/region: Add helper function to determine memory is online

[Alison Schofield <alison.schofield@intel.com>]

On Thu, Feb 27, 2025 at 10:38:12PM +0000, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> Add helper function to determine a CXL memory is online.
> 
> Use case: certain memory operations are permitted when the memory
> is offline only, for eg. some memory repair operations.
> 

Hi Shiju,

This helper is basically wrapping the existing helper
cxl_num_decoders_committed(port) so that you can start from a cxlmd.
Other callers of cxl_num_decoders_committed() do similar work, ie 
find the port, confirm an endpoint, then call it. I don't think
it's done enough to warrant a helper for all callers. (but others
may disagree and ask for that)

This patchset does this work a few times, so a helper makes sense
here. The part I don't get is why it is placed in the region driver,
requiring it to be stubbed out when region driver is not present.

cxl_num_decoders_committed() is part of the cxl/core and is 
defined in port.c.

Long way of asking, would it work to just add a helper to 
core/memfeatures.c and use it within?

The name could use an update. cxl_are_decoders_committed(cxlmd) is
a stretch because a cxlmd does not have decoders. That naming made
sense for 'cxl_num_decoders_committed(port) because a port has
decoders, a memdev does not.

-- Alison


> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
>  drivers/cxl/core/core.h   |  9 +++++++++
>  drivers/cxl/core/region.c | 10 ++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 6c83f6f18122..386e36b18c19 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -32,8 +32,17 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa);
> +bool cxl_are_decoders_committed(const struct cxl_memdev *cxlmd);
>  
>  #else
> +static inline bool cxl_are_decoders_committed(const struct cxl_memdev *cxlmd)
> +{
> +	/*
> +	 * If no driver, in absence of a way to check, assume decoders are committed.
> +	 */
> +	return true;
> +}
> +
>  static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
>  				 const struct cxl_memdev *cxlmd, u64 dpa)
>  {
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 9e7b716296d7..3fd14cb0c470 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2864,6 +2864,16 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
>  	return ctx.cxlr;
>  }
>  
> +bool cxl_are_decoders_committed(const struct cxl_memdev *cxlmd)
> +{
> +	struct cxl_port *port = cxlmd->endpoint;
> +
> +	if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool cxl_is_hpa_in_chunk(u64 hpa, struct cxl_region *cxlr, int pos)
>  {
>  	struct cxl_region_params *p = &cxlr->params;
> -- 
> 2.43.0
> 
>