On Wed, 4 Dec 2024 15:46:47 -0700
Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
> The current cxl region size only indicates the size of the CXL memory
> region without accounting for the extended linear cache size. Retrieve the
> cache size from HMAT and append that to the cxl region size for the cxl
> region range that matches the SRAT range that has extended linear cache
> enabled.
>
> The SRAT defines the whole memory range that includes the extended linear
> cache and the CXL memory region. The new HMAT ECN/ECR to the Memory Side
> Cache Information Structure defines the size of the extended linear cache
> size and matches to the SRAT Memory Affinity Structure by the memory
> proxmity domain. Add a helper to match the cxl range to the SRAT memory
> range in order to retrieve the cache size.
>
> There are several places that checks the cxl region range against the
> decoder range. Use new helper to check between the two ranges and address
> the new cache size.
>
> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
Hi Dave,
A few minor comments inline given you are going around again for the issue
Li Ming spotted.
Jonathan
> ---
> RFC:
> - Minor refactors (Jonathan)
> - Fix grammar (Jonathan)
> ---
> drivers/acpi/numa/hmat.c | 39 ++++++++++++++++++++++
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/acpi.c | 11 ++++++
> drivers/cxl/core/core.h | 3 ++
> drivers/cxl/core/region.c | 70 ++++++++++++++++++++++++++++++++++++---
> drivers/cxl/cxl.h | 2 ++
> include/linux/acpi.h | 19 +++++++++++
> tools/testing/cxl/Kbuild | 1 +
> 8 files changed, 142 insertions(+), 4 deletions(-)
> create mode 100644 drivers/cxl/core/acpi.c
>
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index 976b3e1a6c2a..1c5b3c37ac29 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -108,6 +108,45 @@ static struct memory_target *find_mem_target(unsigned int mem_pxm)
> return NULL;
> }
>
> +/**
> + * hmat_get_extended_linear_cache_size - Retrieve the extended linear cache size
> + * @backing_res: resource from the backing media
> + * @nid: node id for the memory region
> + * @cache_size: (Output) size of extended linear cache.
> + *
> + * Return: 0 on success. Errno on failure.
> + *
> + */
> +int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid,
> + resource_size_t *cache_size)
> +{
> + unsigned int pxm = node_to_pxm(nid);
> + struct memory_target *target;
> + struct target_cache *tcache;
> + struct resource *res;
> +
> + target = find_mem_target(pxm);
> + if (!target)
> + return -ENOENT;
> +
> + list_for_each_entry(tcache, &target->caches, node) {
> + if (tcache->cache_attrs.address_mode ==
> + NODE_CACHE_ADDR_MODE_EXTENDED_LINEAR)
> + continue;
> +
> + res = &target->memregions;
> + if (!resource_contains(res, backing_res))
> + continue;
> +
> + *cache_size = tcache->cache_attrs.size;
> + return 0;
> + }
> +
> + *cache_size = 0;
> + return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL);
Needs quotes around "CXL" now I think.
> +
> static struct memory_target *acpi_find_genport_target(u32 uid)
> {
> struct memory_target *target;
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 9259bcc6773c..1a0c9c6ca818 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -14,5 +14,6 @@ cxl_core-y += pci.o
> cxl_core-y += hdm.o
> cxl_core-y += pmu.o
> cxl_core-y += cdat.o
> +cxl_core-y += acpi.o
> cxl_core-$(CONFIG_TRACING) += trace.o
> cxl_core-$(CONFIG_CXL_REGION) += region.o
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 70d0a017e99c..72ede9842dab 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -824,6 +824,21 @@ static int match_free_decoder(struct device *dev, void *data)
> return 1;
> }
>
> +static bool region_res_match_cxl_range(struct cxl_region_params *p,
> + struct range *range)
> +{
> + if (!p->res)
> + return false;
> +
> + /*
> + * The CXL range is assumed to be fronted by the DRAM range in
> + * current known implementation. This assumption will be made
> + * until a variant implementation exists.
Start with:
If an extended linear cache region then the CXL range is...
Otherwise the comment sounds more general that it is.
> + */
> + return p->res->start + p->cache_size == range->start &&
> + p->res->end == range->end;
> +}
> +
> static int match_auto_decoder(struct device *dev, void *data)
> {
> struct cxl_region_params *p = data;
> @@ -836,7 +851,7 @@ static int match_auto_decoder(struct device *dev, void *data)
> cxld = to_cxl_decoder(dev);
> r = &cxld->hpa_range;
>
> - if (p->res && p->res->start == r->start && p->res->end == r->end)
> + if (region_res_match_cxl_range(p, r))
> return 1;
>
> return 0;
> @@ -1413,8 +1428,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
> if (cxld->interleave_ways != iw ||
> cxld->interleave_granularity != ig ||
> - cxld->hpa_range.start != p->res->start ||
> - cxld->hpa_range.end != p->res->end ||
> + !region_res_match_cxl_range(p, &cxld->hpa_range) ||
> ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) {
> dev_err(&cxlr->dev,
> "%s:%s %s expected iw: %d ig: %d %pr\n",
> +
> /* Establish an empty region covering the given HPA range */
> static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> struct cxl_endpoint_decoder *cxled)
> @@ -3256,6 +3306,18 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>
> *res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
> dev_name(&cxlr->dev));
> +
> + rc = cxl_extended_linear_cache_resize(cxlr, res);
> + if (rc) {
> + /*
> + * Failing to support extended linear cache region resize does not
> + * prevent the region from functioning. Only cause cxl list showing
Only causes cxl list to show incorrect region size
> + * incorrect region size.
> + */
> + dev_warn(cxlmd->dev.parent,
> + "Failed to support extended linear cache.\n");
> + }
> +
> rc = insert_resource(cxlrd->res, res);
> if (rc) {
> /*
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f6015f24ad38..61df87a31e02 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -492,6 +492,7 @@ enum cxl_config_state {
> * @res: allocated iomem capacity for this region
> * @targets: active ordered targets in current decoder configuration
> * @nr_targets: number of targets
> + * @cache_size: extended linear cache size, if exists
otherwise zero. (as you don't check for existence - or remove "if exists"
and just have "extended linear cache size"
> *
> * State transitions are protected by the cxl_region_rwsem
> */
> @@ -503,6 +504,7 @@ struct cxl_region_params {
> struct resource *res;
> struct cxl_endpoint_decoder *targets[CXL_DECODER_MAX_INTERLEAVE];
> int nr_targets;
> + resource_size_t cache_size;
> };
>
> /*
