On Thu, Feb 27, 2025 at 10:38:12PM +0000, shiju.jose@xxxxxxxxxx wrote:
> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
> ---
> 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
>
>
