| Message ID | 20241112221335.432583-3-dave.jiang@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | acpi/hmat / cxl: Add exclusive caching enumeration and RAS support | expand |
On Tue, 12 Nov 2024 15:12:34 -0700 Dave Jiang <dave.jiang@intel.com> 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@intel.com> Hi Dave, A few minor comments inline. Thanks, Jonathan > --- > drivers/acpi/numa/hmat.c | 44 ++++++++++++++++++++++++ > 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, 147 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 39524f36be5b..92b818b72ecc 100644 > --- a/drivers/acpi/numa/hmat.c > +++ b/drivers/acpi/numa/hmat.c > @@ -108,6 +108,50 @@ 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; > + bool cache_found = false; > + struct resource *res; > + > + target = find_mem_target(pxm); > + if (!target) > + return -ENOENT; > + > + list_for_each_entry(tcache, &target->caches, node) { > + if (tcache->cache_attrs.mode == NODE_CACHE_MODE_EXTENDED_LINEAR) { I'd flip this for slightly better readability. if (tcache->cache_attrs.mode != NODE_CACHE_MODE_EXTENDED_LINEAR) continue; res = ... > + res = &target->memregions; > + if (!resource_contains(res, backing_res)) > + continue; > + > + cache_found = true; > + break; > + } > + } > + > + if (!cache_found) { > + *cache_size = 0; > + return 0; > + } > + > + *cache_size = tcache->cache_attrs.size; Why not set this and return in the loop? That way no need to have a local variable. > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); > diff --git a/drivers/cxl/core/acpi.c b/drivers/cxl/core/acpi.c > new file mode 100644 > index 000000000000..f13b4dae6ac5 > --- /dev/null > +++ b/drivers/cxl/core/acpi.c > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */ > +#include <linux/acpi.h> > +#include "cxl.h" > +#include "core.h" Why do you need the cxl headers? Maybe a forwards def of struct resource, but I'm not seeing anything else being needed. > + > +int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res, > + int nid, resource_size_t *size) > +{ > + return hmat_get_extended_linear_cache_size(backing_res, nid, size); > +} > @@ -3215,6 +3229,42 @@ static int match_region_by_range(struct device *dev, void *data) > return rc; > } > > +static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr, > + struct resource *res) > +{ > + struct cxl_region_params *p = &cxlr->params; > + int nid = phys_to_target_node(res->start); > + resource_size_t size, cache_size; > + int rc; > + > + size = resource_size(res); > + if (!size) > + return -EINVAL; > + > + rc = cxl_acpi_get_extended_linear_cache_size(res, nid, &cache_size); > + if (rc) > + return rc; > + > + if (!cache_size) > + return 0; > + > + if (size != cache_size) { > + dev_warn(&cxlr->dev, "Extended Linear Cache is not 1:1, unsupported!"); > + return -EOPNOTSUPP; > + } > + > + /* > + * Move the start of the range to where the cache range starts. The > + * implementation assumes that the cache range is in front of the > + * CXL range. This is not dictated by the HMAT spec but is how the > + * currently known implementation configured. is configured > + */ > + res->start -= cache_size; > + p->cache_size = cache_size; > + > + return 0; > +}
On 11/26/24 9:23 AM, Jonathan Cameron wrote: > On Tue, 12 Nov 2024 15:12:34 -0700 > Dave Jiang <dave.jiang@intel.com> 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@intel.com> > Hi Dave, > > A few minor comments inline. > > Thanks, > > Jonathan > >> --- >> drivers/acpi/numa/hmat.c | 44 ++++++++++++++++++++++++ >> 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, 147 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 39524f36be5b..92b818b72ecc 100644 >> --- a/drivers/acpi/numa/hmat.c >> +++ b/drivers/acpi/numa/hmat.c >> @@ -108,6 +108,50 @@ 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; >> + bool cache_found = false; >> + struct resource *res; >> + >> + target = find_mem_target(pxm); >> + if (!target) >> + return -ENOENT; >> + >> + list_for_each_entry(tcache, &target->caches, node) { >> + if (tcache->cache_attrs.mode == NODE_CACHE_MODE_EXTENDED_LINEAR) { > > I'd flip this for slightly better readability. ok > if (tcache->cache_attrs.mode != NODE_CACHE_MODE_EXTENDED_LINEAR) > continue; > > res = ... > > >> + res = &target->memregions; >> + if (!resource_contains(res, backing_res)) >> + continue; >> + >> + cache_found = true; >> + break; >> + } >> + } >> + >> + if (!cache_found) { >> + *cache_size = 0; >> + return 0; >> + } >> + >> + *cache_size = tcache->cache_attrs.size; > > Why not set this and return in the loop? > That way no need to have a local variable. ok > >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); > >> diff --git a/drivers/cxl/core/acpi.c b/drivers/cxl/core/acpi.c >> new file mode 100644 >> index 000000000000..f13b4dae6ac5 >> --- /dev/null >> +++ b/drivers/cxl/core/acpi.c >> @@ -0,0 +1,11 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */ >> +#include <linux/acpi.h> >> +#include "cxl.h" >> +#include "core.h" > > Why do you need the cxl headers? Maybe a forwards def of > struct resource, but I'm not seeing anything else being needed. The prototype is declared in core.h, and it seems core.h needs cxl.h. I wonder if core.h should just include cxl.h. > > >> + >> +int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res, >> + int nid, resource_size_t *size) >> +{ >> + return hmat_get_extended_linear_cache_size(backing_res, nid, size); >> +} > > >> @@ -3215,6 +3229,42 @@ static int match_region_by_range(struct device *dev, void *data) >> return rc; >> } >> >> +static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr, >> + struct resource *res) >> +{ >> + struct cxl_region_params *p = &cxlr->params; >> + int nid = phys_to_target_node(res->start); >> + resource_size_t size, cache_size; >> + int rc; >> + >> + size = resource_size(res); >> + if (!size) >> + return -EINVAL; >> + >> + rc = cxl_acpi_get_extended_linear_cache_size(res, nid, &cache_size); >> + if (rc) >> + return rc; >> + >> + if (!cache_size) >> + return 0; >> + >> + if (size != cache_size) { >> + dev_warn(&cxlr->dev, "Extended Linear Cache is not 1:1, unsupported!"); >> + return -EOPNOTSUPP; >> + } >> + >> + /* >> + * Move the start of the range to where the cache range starts. The >> + * implementation assumes that the cache range is in front of the >> + * CXL range. This is not dictated by the HMAT spec but is how the >> + * currently known implementation configured. > > is configured will fix > >> + */ >> + res->start -= cache_size; >> + p->cache_size = cache_size; >> + >> + return 0; >> +} > >
diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c index 39524f36be5b..92b818b72ecc 100644 --- a/drivers/acpi/numa/hmat.c +++ b/drivers/acpi/numa/hmat.c @@ -108,6 +108,50 @@ 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; + bool cache_found = false; + struct resource *res; + + target = find_mem_target(pxm); + if (!target) + return -ENOENT; + + list_for_each_entry(tcache, &target->caches, node) { + if (tcache->cache_attrs.mode == NODE_CACHE_MODE_EXTENDED_LINEAR) { + res = &target->memregions; + if (!resource_contains(res, backing_res)) + continue; + + cache_found = true; + break; + } + } + + if (!cache_found) { + *cache_size = 0; + return 0; + } + + *cache_size = tcache->cache_attrs.size; + + return 0; +} +EXPORT_SYMBOL_NS_GPL(hmat_get_extended_linear_cache_size, CXL); + 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/acpi.c b/drivers/cxl/core/acpi.c new file mode 100644 index 000000000000..f13b4dae6ac5 --- /dev/null +++ b/drivers/cxl/core/acpi.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */ +#include <linux/acpi.h> +#include "cxl.h" +#include "core.h" + +int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res, + int nid, resource_size_t *size) +{ + return hmat_get_extended_linear_cache_size(backing_res, nid, size); +} diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h index 0c62b4069ba0..c4dc9aefe25f 100644 --- a/drivers/cxl/core/core.h +++ b/drivers/cxl/core/core.h @@ -110,4 +110,7 @@ bool cxl_need_node_perf_attrs_update(int nid); int cxl_port_get_switch_dport_bandwidth(struct cxl_port *port, struct access_coordinate *c); +int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res, + int nid, resource_size_t *size); + #endif /* __CXL_CORE_H__ */ diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index e701e4b04032..a37923c030a3 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -816,6 +816,21 @@ static int match_free_decoder(struct device *dev, void *data) return 0; } +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. + */ + 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; @@ -828,7 +843,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; @@ -1406,8 +1421,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", @@ -1931,7 +1945,7 @@ static int cxl_region_attach(struct cxl_region *cxlr, return -ENXIO; } - if (resource_size(cxled->dpa_res) * p->interleave_ways != + if (resource_size(cxled->dpa_res) * p->interleave_ways + p->cache_size != resource_size(p->res)) { dev_dbg(&cxlr->dev, "%s:%s: decoder-size-%#llx * ways-%d != region-size-%#llx\n", @@ -3215,6 +3229,42 @@ static int match_region_by_range(struct device *dev, void *data) return rc; } +static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr, + struct resource *res) +{ + struct cxl_region_params *p = &cxlr->params; + int nid = phys_to_target_node(res->start); + resource_size_t size, cache_size; + int rc; + + size = resource_size(res); + if (!size) + return -EINVAL; + + rc = cxl_acpi_get_extended_linear_cache_size(res, nid, &cache_size); + if (rc) + return rc; + + if (!cache_size) + return 0; + + if (size != cache_size) { + dev_warn(&cxlr->dev, "Extended Linear Cache is not 1:1, unsupported!"); + return -EOPNOTSUPP; + } + + /* + * Move the start of the range to where the cache range starts. The + * implementation assumes that the cache range is in front of the + * CXL range. This is not dictated by the HMAT spec but is how the + * currently known implementation configured. + */ + res->start -= cache_size; + p->cache_size = cache_size; + + return 0; +} + /* 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) @@ -3261,6 +3311,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 + * 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 0d8b810a51f0..26466807fa7a 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -483,6 +483,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 * * State transitions are protected by the cxl_region_rwsem */ @@ -494,6 +495,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; }; /* diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 4d5ee84c468b..10ffba7cb9ad 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -436,12 +436,20 @@ int thermal_acpi_critical_trip_temp(struct acpi_device *adev, int *ret_temp); #ifdef CONFIG_ACPI_HMAT int acpi_get_genport_coordinates(u32 uid, struct access_coordinate *coord); +int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, + resource_size_t *size); #else static inline int acpi_get_genport_coordinates(u32 uid, struct access_coordinate *coord) { return -EOPNOTSUPP; } + +static inline int hmat_get_extended_linear_cache_size(struct resource *backing_res, + int nid, resource_size_t *size) +{ + return -EOPNOTSUPP; +} #endif #ifdef CONFIG_ACPI_NUMA @@ -1090,6 +1098,17 @@ static inline acpi_handle acpi_get_processor_handle(int cpu) #endif /* !CONFIG_ACPI */ +#ifdef CONFIG_ACPI_HMAT +int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid, + resource_size_t *size); +#else +static inline int hmat_get_extended_linear_cache_size(struct resource *backing_res, + int nid, resource_size_t *size) +{ + return -EOPNOTSUPP; +} +#endif + extern void arch_post_acpi_subsys_init(void); #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild index b1256fee3567..1ae13987a8a2 100644 --- a/tools/testing/cxl/Kbuild +++ b/tools/testing/cxl/Kbuild @@ -61,6 +61,7 @@ cxl_core-y += $(CXL_CORE_SRC)/pci.o cxl_core-y += $(CXL_CORE_SRC)/hdm.o cxl_core-y += $(CXL_CORE_SRC)/pmu.o cxl_core-y += $(CXL_CORE_SRC)/cdat.o +cxl_core-y += $(CXL_CORE_SRC)/acpi.o cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o cxl_core-y += config_check.o
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@intel.com> --- v2: - Fix spelling errors (Jonathan) - Move matching of res range to the match loop. (Jonathan) - Rename region_res_match_range() to region_res_match_cxl_range() and add comments. (Jonathan) - Refactor region_res_match_cxl_range() to simplify code. (Jonathan) - Remove unintended blank line. (Jonathan) - Add warning emission when cache is not 1:1 to cxl region. (Jonathan) --- drivers/acpi/numa/hmat.c | 44 ++++++++++++++++++++++++ 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, 147 insertions(+), 4 deletions(-) create mode 100644 drivers/cxl/core/acpi.c