On Thu, Jun 23, 2022 at 07:45:36PM -0700, Dan Williams wrote:
> Root decoders are responsible for hosting the available host address
> space for endpoints and regions to claim. The tracking of that available
> capacity can be done in iomem_resource directly. As a result, root
> decoders no longer need to host their own resource tree. The
> current ->platform_res attribute was added prematurely.
> 
> Otherwise, ->hpa_range fills the role of conveying the current decode
> range of the decoder.
> 
> Signed-off-by: Dan Williams <[email protected]>
> ---
>  drivers/cxl/acpi.c      |   17 ++++++++++-------
>  drivers/cxl/core/pci.c  |    8 +-------
>  drivers/cxl/core/port.c |   30 +++++++-----------------------
>  drivers/cxl/cxl.h       |    6 +-----
>  4 files changed, 19 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 40286f5df812..951695cdb455 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -108,8 +108,10 @@ static int cxl_parse_cfmws(union acpi_subtable_headers 
> *header, void *arg,
>  
>       cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
>       cxld->target_type = CXL_DECODER_EXPANDER;
> -     cxld->platform_res = (struct resource)DEFINE_RES_MEM(cfmws->base_hpa,
> -                                                          
> cfmws->window_size);
> +     cxld->hpa_range = (struct range) {
> +             .start = cfmws->base_hpa,
> +             .end = cfmws->base_hpa + cfmws->window_size - 1,
> +     };
>       cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
>       cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
>  
> @@ -119,13 +121,14 @@ static int cxl_parse_cfmws(union acpi_subtable_headers 
> *header, void *arg,
>       else
>               rc = cxl_decoder_autoremove(dev, cxld);
>       if (rc) {
> -             dev_err(dev, "Failed to add decoder for %pr\n",
> -                     &cxld->platform_res);
> +             dev_err(dev, "Failed to add decoder for [%#llx - %#llx]\n",
> +                     cxld->hpa_range.start, cxld->hpa_range.end);

Minor nit, should we add range in our debug message?

+               dev_err(dev, "Failed to add decoder for range [%#llx - 
%#llx]\n",

>               return 0;
>       }
> -     dev_dbg(dev, "add: %s node: %d range %pr\n", dev_name(&cxld->dev),
> -             phys_to_target_node(cxld->platform_res.start),
> -             &cxld->platform_res);
> +     dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
> +             dev_name(&cxld->dev),
> +             phys_to_target_node(cxld->hpa_range.start),
> +             cxld->hpa_range.start, cxld->hpa_range.end);
>  
>       return 0;
>  }
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index c4c99ff7b55e..7672789c3225 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -225,7 +225,6 @@ static int dvsec_range_allowed(struct device *dev, void 
> *arg)
>  {
>       struct range *dev_range = arg;
>       struct cxl_decoder *cxld;
> -     struct range root_range;
>  
>       if (!is_root_decoder(dev))
>               return 0;
> @@ -237,12 +236,7 @@ static int dvsec_range_allowed(struct device *dev, void 
> *arg)
>       if (!(cxld->flags & CXL_DECODER_F_RAM))
>               return 0;
>  
> -     root_range = (struct range) {
> -             .start = cxld->platform_res.start,
> -             .end = cxld->platform_res.end,
> -     };
> -
> -     return range_contains(&root_range, dev_range);
> +     return range_contains(&cxld->hpa_range, dev_range);
>  }
>  
>  static void disable_hdm(void *_cxlhdm)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 98bcbbd59a75..b51eb41aa839 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -73,29 +73,17 @@ static ssize_t start_show(struct device *dev, struct 
> device_attribute *attr,
>                         char *buf)
>  {
>       struct cxl_decoder *cxld = to_cxl_decoder(dev);
> -     u64 start;
>  
> -     if (is_root_decoder(dev))
> -             start = cxld->platform_res.start;
> -     else
> -             start = cxld->hpa_range.start;
> -
> -     return sysfs_emit(buf, "%#llx\n", start);
> +     return sysfs_emit(buf, "%#llx\n", cxld->hpa_range.start);
>  }
>  static DEVICE_ATTR_ADMIN_RO(start);
>  
>  static ssize_t size_show(struct device *dev, struct device_attribute *attr,
> -                     char *buf)
> +                      char *buf)
>  {
>       struct cxl_decoder *cxld = to_cxl_decoder(dev);
> -     u64 size;
> -
> -     if (is_root_decoder(dev))
> -             size = resource_size(&cxld->platform_res);
> -     else
> -             size = range_len(&cxld->hpa_range);
>  
> -     return sysfs_emit(buf, "%#llx\n", size);
> +     return sysfs_emit(buf, "%#llx\n", range_len(&cxld->hpa_range));
>  }
>  static DEVICE_ATTR_RO(size);
>  
> @@ -1233,7 +1221,10 @@ static struct cxl_decoder *cxl_decoder_alloc(struct 
> cxl_port *port,
>       cxld->interleave_ways = 1;
>       cxld->interleave_granularity = PAGE_SIZE;
>       cxld->target_type = CXL_DECODER_EXPANDER;
> -     cxld->platform_res = (struct resource)DEFINE_RES_MEM(0, 0);
> +     cxld->hpa_range = (struct range) {
> +             .start = 0,
> +             .end = -1,
> +     };
>  
>       return cxld;
>  err:
> @@ -1347,13 +1338,6 @@ int cxl_decoder_add_locked(struct cxl_decoder *cxld, 
> int *target_map)
>       if (rc)
>               return rc;
>  
> -     /*
> -      * Platform decoder resources should show up with a reasonable name. All
> -      * other resources are just sub ranges within the main decoder resource.
> -      */
> -     if (is_root_decoder(dev))
> -             cxld->platform_res.name = dev_name(dev);
> -
>       return device_add(dev);
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_decoder_add_locked, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 8256728cea8d..35ce17872fc1 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -197,7 +197,6 @@ enum cxl_decoder_type {
>   * struct cxl_decoder - CXL address range decode configuration
>   * @dev: this decoder's device
>   * @id: kernel device name id
> - * @platform_res: address space resources considered by root decoder
>   * @hpa_range: Host physical address range mapped by this decoder
>   * @interleave_ways: number of cxl_dports in this decode
>   * @interleave_granularity: data stride per dport
> @@ -210,10 +209,7 @@ enum cxl_decoder_type {
>  struct cxl_decoder {
>       struct device dev;
>       int id;
> -     union {
> -             struct resource platform_res;
> -             struct range hpa_range;
> -     };
> +     struct range hpa_range;
>       int interleave_ways;
>       int interleave_granularity;
>       enum cxl_decoder_type target_type;
> 
> 


Otherwise, looks good.

Reviewed by: Adam Manzanares <[email protected]>

Reply via email to