Re: [PATCH 11/13] cxl/core: Convert decoder range to resource

[Dan Williams <dan.j.williams@intel.com>]

On Thu, Sep 2, 2021 at 12:50 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> Regions will use the resource API in order to help manage allocated
> space. As regions are children of the decoder, it makes sense that the
> parent host the main resource to be suballocated by the region.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> ---
>  drivers/cxl/acpi.c     | 12 ++++--------
>  drivers/cxl/core/bus.c |  4 ++--
>  drivers/cxl/cxl.h      |  4 ++--
>  3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index fd14094bdb3f..26691313d716 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -125,10 +125,9 @@ static void cxl_add_cfmws_decoders(struct device *dev,
>
>                 cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
>                 cxld->target_type = CXL_DECODER_EXPANDER;
> -               cxld->range = (struct range) {
> -                       .start = cfmws->base_hpa,
> -                       .end = cfmws->base_hpa + cfmws->window_size - 1,
> -               };
> +               cxld->res = (struct resource)DEFINE_RES_MEM_NAMED(cfmws->base_hpa,
> +                                                                 cfmws->window_size,
> +                                                                 "cfmws");

I like this direction, but it's unfortunate to carry the bloat of
'struct resource' in all decoders when only the top-level needs it (as
far as I can see). I think it will be handy to have a global resource
tree available for address translation service to the rest of the OS
where resource providers / holders can be looked up by name in a
cxl-specific memory resource tree. I.e. how about something like:

diff --git a/drivers/cxl/core/bus.c b/drivers/cxl/core/bus.c
index be787685b13e..e64939f1b07d 100644
--- a/drivers/cxl/core/bus.c
+++ b/drivers/cxl/core/bus.c
@@ -26,6 +26,8 @@

 static DEFINE_IDA(cxl_port_ida);

+static struct resource cxlmem_resource = DEFINE_RES_MEM_NAMED(0, -1,
"CXL mem");
+
 static ssize_t devtype_show(struct device *dev, struct device_attribute *attr,
                            char *buf)
 {
@@ -180,6 +182,9 @@ static void cxl_decoder_release(struct device *dev)
        struct cxl_decoder *cxld = to_cxl_decoder(dev);
        struct cxl_port *port = to_cxl_port(dev->parent);

+       if (cxld->res)
+               remove_resource(cxld->res);
+       kfree(cxld->res);
        ida_free(&port->decoder_ida, cxld->id);
        kfree(cxld);
 }
@@ -545,6 +550,24 @@ int cxl_decoder_add(struct device *host, struct
cxl_decoder *cxld,
        if (rc)
                return rc;

+       if (dev->type == &cxl_decoder_root_type) {
+               struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
+
+               if (!res)
+                       return -ENOMEM;
+               *res = (struct resource) DEFINE_RES_MEM_NAMED(
+                       cxld->range.start,
+                       cxld->range.end,
+                       dev_name(dev)
+               );
+
+               rc = insert_resource(&cxlmem_resource, res);
+               if (rc) {
+                       kfree(res);
+                       return rc;
+               }
+       }
+
        return device_add(dev);
 }
 EXPORT_SYMBOL_GPL(cxl_decoder_add);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 6c7a7e9af0d4..7d0d218d9883 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -190,7 +190,8 @@ enum cxl_decoder_type {
  * struct cxl_decoder - CXL address range decode configuration
  * @dev: this decoder's device
  * @id: kernel device name id
- * @range: address range considered by this decoder
+ * @range: current address range considered by this decoder
+ * @res: top level CXL mem resource (root decoder only)
  * @interleave_ways: number of cxl_dports in this decode
  * @interleave_granularity: data stride per dport
  * @target_type: accelerator vs expander (type2 vs type3) selector
@@ -202,6 +203,7 @@ struct cxl_decoder {
        struct device dev;
        int id;
        struct range range;
+       struct resource *res;
        int interleave_ways;
        int interleave_granularity;
        enum cxl_decoder_type target_type;