[PATCH v3 0/6] acpi: numa: add target support for generic port to HMAT parsing

[PATCH v3 0/6] acpi: numa: add target support for generic port to HMAT parsing

[Dave Jiang]

v3:
- Break out ACPI_SRAT_DEVICE_HANDLE_SIZE define. Merged into ACPICA repo.
- Break out code in hmat_parse_locality() to improve readability. (Jonathan)
- Drop continue after setting gen target access data. (Jonathan)
v2:
- Add check for memory type for setting GENPORT access data. (Jonathan)
- NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK
- Change strncmp() to memcmp() for device handle compare

Hi Rafael,
Please consider acking this series. Dan will take it through his CXL tree.
This is the second set of ACPI changes that's part of the "cxl: Add support for
QTG ID retrieval for CXL subsystem" [1]. It has dependency on the patch [2]
that was sent to you and GregKH and Greg has ack'd. But it's independent of the
first set of ACPI changes [3]. The series adds support to store the device handle
from the SRAT Generic Port Affinity Structure in correlation with a proximity
domain. And later on the HMAT is parsed and the performance data is stored
associated with the proximity domain. A helper function is added to allow the CXL
driver to retrieve the performance data that matches with the device handle later
on. The full series of CXL code is at this git repo [4] for usage reference.


[1]: https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t 
[2]: https://lore.kernel.org/lkml/168332248685.2190392.1983307884583782116.stgit@djiang5-mobl3/
[3]: https://lore.kernel.org/linux-acpi/168330787964.2042604.17648905811002211147.stgit@djiang5-mobl3/T/#t
[4]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg

---

Dave Jiang (6):
      acpi: numa: Create enum for memory_target access coordinates indexing
      ACPICA: Add a define for size of acpi_srat_generic_affinity DeviceHandle
      acpi: numa: Add genport target allocation to the HMAT parsing
      acpi: Break out nesting for hmat_parse_locality()
      acpi: numa: Add setting of generic port system locality attributes
      acpi: numa: Add helper function to retrieve the performance attributes


 drivers/acpi/numa/hmat.c | 151 +++++++++++++++++++++++++++++++++------
 include/acpi/actbl3.h    |   4 +-
 include/linux/acpi.h     |  12 ++++
 3 files changed, 144 insertions(+), 23 deletions(-)

--

[PATCH v3 1/6] acpi: numa: Create enum for memory_target access coordinates indexing

[Dave Jiang]

Create enums to provide named indexing for the access coordinate array.
This is in preparation for adding generic port support which will add a
third index in the array to keep the generic port attributes separate from
the memory attributes.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/numa/hmat.c |   35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index f9ff992038fa..abed728bf09d 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -57,12 +57,18 @@ struct target_cache {
 	struct node_cache_attrs cache_attrs;
 };
 
+enum {
+	NODE_ACCESS_CLASS_0 = 0,
+	NODE_ACCESS_CLASS_1,
+	NODE_ACCESS_CLASS_MAX,
+};
+
 struct memory_target {
 	struct list_head node;
 	unsigned int memory_pxm;
 	unsigned int processor_pxm;
 	struct resource memregions;
-	struct access_coordinate coord[2];
+	struct access_coordinate coord[NODE_ACCESS_CLASS_MAX];
 	struct list_head caches;
 	struct node_cache_attrs cache_attrs;
 	bool registered;
@@ -338,10 +344,12 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
 			if (mem_hier == ACPI_HMAT_MEMORY) {
 				target = find_mem_target(targs[targ]);
 				if (target && target->processor_pxm == inits[init]) {
-					hmat_update_target_access(target, type, value, 0);
+					hmat_update_target_access(target, type, value,
+								  NODE_ACCESS_CLASS_0);
 					/* If the node has a CPU, update access 1 */
 					if (node_state(pxm_to_node(inits[init]), N_CPU))
-						hmat_update_target_access(target, type, value, 1);
+						hmat_update_target_access(target, type, value,
+									  NODE_ACCESS_CLASS_1);
 				}
 			}
 		}
@@ -600,10 +608,12 @@ static void hmat_register_target_initiators(struct memory_target *target)
 	 */
 	if (target->processor_pxm != PXM_INVAL) {
 		cpu_nid = pxm_to_node(target->processor_pxm);
-		register_memory_node_under_compute_node(mem_nid, cpu_nid, 0);
+		register_memory_node_under_compute_node(mem_nid, cpu_nid,
+							NODE_ACCESS_CLASS_0);
 		access0done = true;
 		if (node_state(cpu_nid, N_CPU)) {
-			register_memory_node_under_compute_node(mem_nid, cpu_nid, 1);
+			register_memory_node_under_compute_node(mem_nid, cpu_nid,
+								NODE_ACCESS_CLASS_1);
 			return;
 		}
 	}
@@ -644,12 +654,13 @@ static void hmat_register_target_initiators(struct memory_target *target)
 			}
 			if (best)
 				hmat_update_target_access(target, loc->hmat_loc->data_type,
-							  best, 0);
+							  best, NODE_ACCESS_CLASS_0);
 		}
 
 		for_each_set_bit(i, p_nodes, MAX_NUMNODES) {
 			cpu_nid = pxm_to_node(i);
-			register_memory_node_under_compute_node(mem_nid, cpu_nid, 0);
+			register_memory_node_under_compute_node(mem_nid, cpu_nid,
+								NODE_ACCESS_CLASS_0);
 		}
 	}
 
@@ -681,11 +692,13 @@ static void hmat_register_target_initiators(struct memory_target *target)
 				clear_bit(initiator->processor_pxm, p_nodes);
 		}
 		if (best)
-			hmat_update_target_access(target, loc->hmat_loc->data_type, best, 1);
+			hmat_update_target_access(target, loc->hmat_loc->data_type, best,
+						  NODE_ACCESS_CLASS_1);
 	}
 	for_each_set_bit(i, p_nodes, MAX_NUMNODES) {
 		cpu_nid = pxm_to_node(i);
-		register_memory_node_under_compute_node(mem_nid, cpu_nid, 1);
+		register_memory_node_under_compute_node(mem_nid, cpu_nid,
+							NODE_ACCESS_CLASS_1);
 	}
 }
 
@@ -746,8 +759,8 @@ static void hmat_register_target(struct memory_target *target)
 	if (!target->registered) {
 		hmat_register_target_initiators(target);
 		hmat_register_target_cache(target);
-		hmat_register_target_perf(target, 0);
-		hmat_register_target_perf(target, 1);
+		hmat_register_target_perf(target, NODE_ACCESS_CLASS_0);
+		hmat_register_target_perf(target, NODE_ACCESS_CLASS_1);
 		target->registered = true;
 	}
 	mutex_unlock(&target_lock);

[PATCH v3 2/6] ACPICA: Add a define for size of acpi_srat_generic_affinity DeviceHandle

[Dave Jiang]

ACPICA commit be56820b03d8aeabfa6709c4d99bf1711afe7ef1

Replace magic number with a define. Linux kernel code will utilize this
define.

Link: https://github.com/acpica/acpica/commit/be56820b
Link: https://github.com/acpica/acpica/pull/876
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 include/acpi/actbl3.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index f51c46f4e3e4..1838e5cb313a 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -279,12 +279,14 @@ struct acpi_srat_gic_its_affinity {
  * 6: ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY
  */
 
+#define ACPI_SRAT_DEVICE_HANDLE_SIZE	16
+
 struct acpi_srat_generic_affinity {
 	struct acpi_subtable_header header;
 	u8 reserved;
 	u8 device_handle_type;
 	u32 proximity_domain;
-	u8 device_handle[16];
+	u8 device_handle[ACPI_SRAT_DEVICE_HANDLE_SIZE];
 	u32 flags;
 	u32 reserved1;
 };

[PATCH v3 3/6] acpi: numa: Add genport target allocation to the HMAT parsing

[Dave Jiang]

Add SRAT parsing for the HMAT init in order to collect the device handle
from the Generic Port Affinity Structure. The device handle will serve as
the key to search for target data.

Consoliate the common code with alloc_memory_target() in a helper function
alloc_target().

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v3:
- Move ACPI_SRAT_DEVICE_HANDLE_SIZE to separate patch for ACPICA
---
 drivers/acpi/numa/hmat.c |   53 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index abed728bf09d..e2ab1cce0add 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -71,6 +71,7 @@ struct memory_target {
 	struct access_coordinate coord[NODE_ACCESS_CLASS_MAX];
 	struct list_head caches;
 	struct node_cache_attrs cache_attrs;
+	u8 device_handle[ACPI_SRAT_DEVICE_HANDLE_SIZE];
 	bool registered;
 };
 
@@ -125,8 +126,7 @@ static __init void alloc_memory_initiator(unsigned int cpu_pxm)
 	list_add_tail(&initiator->node, &initiators);
 }
 
-static __init void alloc_memory_target(unsigned int mem_pxm,
-		resource_size_t start, resource_size_t len)
+static __init struct memory_target *alloc_target(unsigned int mem_pxm)
 {
 	struct memory_target *target;
 
@@ -134,7 +134,7 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
 	if (!target) {
 		target = kzalloc(sizeof(*target), GFP_KERNEL);
 		if (!target)
-			return;
+			return NULL;
 		target->memory_pxm = mem_pxm;
 		target->processor_pxm = PXM_INVAL;
 		target->memregions = (struct resource) {
@@ -147,6 +147,19 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
 		INIT_LIST_HEAD(&target->caches);
 	}
 
+	return target;
+}
+
+static __init void alloc_memory_target(unsigned int mem_pxm,
+				       resource_size_t start,
+				       resource_size_t len)
+{
+	struct memory_target *target;
+
+	target = alloc_target(mem_pxm);
+	if (!target)
+		return;
+
 	/*
 	 * There are potentially multiple ranges per PXM, so record each
 	 * in the per-target memregions resource tree.
@@ -157,6 +170,17 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
 				start, start + len, mem_pxm);
 }
 
+static __init void alloc_genport_target(unsigned int mem_pxm, u8 *handle)
+{
+	struct memory_target *target;
+
+	target = alloc_target(mem_pxm);
+	if (!target)
+		return;
+
+	memcpy(target->device_handle, handle, ACPI_SRAT_DEVICE_HANDLE_SIZE);
+}
+
 static __init const char *hmat_data_type(u8 type)
 {
 	switch (type) {
@@ -498,6 +522,22 @@ static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
 	return 0;
 }
 
+static __init int srat_parse_genport_affinity(union acpi_subtable_headers *header,
+					      const unsigned long end)
+{
+	struct acpi_srat_generic_affinity *ga = (void *)header;
+
+	if (!ga)
+		return -EINVAL;
+
+	if (!(ga->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED))
+		return 0;
+
+	alloc_genport_target(ga->proximity_domain, (u8 *)ga->device_handle);
+
+	return 0;
+}
+
 static u32 hmat_initiator_perf(struct memory_target *target,
 			       struct memory_initiator *initiator,
 			       struct acpi_hmat_locality *hmat_loc)
@@ -848,6 +888,13 @@ static __init int hmat_init(void)
 				ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 				srat_parse_mem_affinity, 0) < 0)
 		goto out_put;
+
+	if (acpi_table_parse_entries(ACPI_SIG_SRAT,
+				     sizeof(struct acpi_table_srat),
+				     ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY,
+				     srat_parse_genport_affinity, 0) < 0)
+		goto out_put;
+
 	acpi_put_table(tbl);
 
 	status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);

[PATCH v3 4/6] acpi: Break out nesting for hmat_parse_locality()

[Dave Jiang]

Refactor hmat_parse_locality() to break up the deep nesting of the
function.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/numa/hmat.c |   32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index e2ab1cce0add..cb240f5233fe 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -320,11 +320,28 @@ static __init void hmat_add_locality(struct acpi_hmat_locality *hmat_loc)
 	}
 }
 
+static __init void hmat_update_target(unsigned int tgt_pxm, unsigned int init_pxm,
+				      u8 mem_hier, u8 type, u32 value)
+{
+	struct memory_target *target = find_mem_target(tgt_pxm);
+
+	if (mem_hier != ACPI_HMAT_MEMORY)
+		return;
+
+	if (target && target->processor_pxm == init_pxm) {
+		hmat_update_target_access(target, type, value,
+					  NODE_ACCESS_CLASS_0);
+		/* If the node has a CPU, update access 1 */
+		if (node_state(pxm_to_node(init_pxm), N_CPU))
+			hmat_update_target_access(target, type, value,
+						  NODE_ACCESS_CLASS_1);
+	}
+}
+
 static __init int hmat_parse_locality(union acpi_subtable_headers *header,
 				      const unsigned long end)
 {
 	struct acpi_hmat_locality *hmat_loc = (void *)header;
-	struct memory_target *target;
 	unsigned int init, targ, total_size, ipds, tpds;
 	u32 *inits, *targs, value;
 	u16 *entries;
@@ -365,17 +382,8 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
 				inits[init], targs[targ], value,
 				hmat_data_type_suffix(type));
 
-			if (mem_hier == ACPI_HMAT_MEMORY) {
-				target = find_mem_target(targs[targ]);
-				if (target && target->processor_pxm == inits[init]) {
-					hmat_update_target_access(target, type, value,
-								  NODE_ACCESS_CLASS_0);
-					/* If the node has a CPU, update access 1 */
-					if (node_state(pxm_to_node(inits[init]), N_CPU))
-						hmat_update_target_access(target, type, value,
-									  NODE_ACCESS_CLASS_1);
-				}
-			}
+			hmat_update_target(targs[targ], inits[init],
+					   mem_hier, type, value);
 		}
 	}
 

[PATCH v3 5/6] acpi: numa: Add setting of generic port system locality attributes

[Dave Jiang]

Add generic port support for the parsing of HMAT system locality sub-table.
The attributes will be added to the third array member of the access
coordinates in order to not mix with the existing memory attributes. It only
provides the system locality attributes from initator to the generic port
targets and is missing the rest of the data to the actual memory device.

The complete attributes will be updated when a memory device is
attached and the system locality information is calculated end to end.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v3:
- Drop continue after setting gen target access data. (Jonathan)
v2:
- Fix commit log runon sentence. (Jonathan)
- Add a check for memory type for skipping other access levels. (Jonathan)
- NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK. (Jonathan)
---
 drivers/acpi/numa/hmat.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index cb240f5233fe..32b951cd5ee4 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -60,6 +60,7 @@ struct target_cache {
 enum {
 	NODE_ACCESS_CLASS_0 = 0,
 	NODE_ACCESS_CLASS_1,
+	NODE_ACCESS_CLASS_GENPORT_SINK,
 	NODE_ACCESS_CLASS_MAX,
 };
 
@@ -329,6 +330,9 @@ static __init void hmat_update_target(unsigned int tgt_pxm, unsigned int init_px
 		return;
 
 	if (target && target->processor_pxm == init_pxm) {
+		if (*target->device_handle)
+			hmat_update_target_access(target, type, value,
+						  NODE_ACCESS_CLASS_GENPORT_SINK);
 		hmat_update_target_access(target, type, value,
 					  NODE_ACCESS_CLASS_0);
 		/* If the node has a CPU, update access 1 */

[PATCH v3 6/6] acpi: numa: Add helper function to retrieve the performance attributes

[Dave Jiang]

Add helper to retrieve the performance attributes based on the device
handle.  The helper function is exported so the CXL driver can use that
to acquire the performance data between the CPU and the CXL host bridge.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Change strncmp to memcmp. (Jonathan)
---
 drivers/acpi/numa/hmat.c |   35 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h     |   12 ++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 32b951cd5ee4..b356c149ddb1 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -107,6 +107,41 @@ static struct memory_target *find_mem_target(unsigned int mem_pxm)
 	return NULL;
 }
 
+static struct memory_target *acpi_find_genport_target(u8 *device_handle)
+{
+	struct memory_target *target;
+
+	list_for_each_entry(target, &targets, node) {
+		if (!memcmp(target->device_handle, device_handle,
+			    ACPI_SRAT_DEVICE_HANDLE_SIZE))
+			return target;
+	}
+
+	return NULL;
+}
+
+/**
+ * acpi_get_genport_coordinates - Retrieve the access coordinates for a generic port
+ * @device_handle: Device handle string (ACPI or PCI) to match up to the gen port
+ * @coord: The access coordinates written back out for the generic port
+ *
+ * Return: 0 on success. Errno on failure.
+ */
+int acpi_get_genport_coordinates(u8 *device_handle,
+				 struct access_coordinate *coord)
+{
+	struct memory_target *target;
+
+	target = acpi_find_genport_target(device_handle);
+	if (!target)
+		return -ENOENT;
+
+	*coord = target->coord[NODE_ACCESS_CLASS_GENPORT_SINK];
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(acpi_get_genport_coordinates, CXL);
+
 static __init void alloc_memory_initiator(unsigned int cpu_pxm)
 {
 	struct memory_initiator *initiator;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index bf3433747495..9f4ac0a39ab6 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -16,6 +16,7 @@
 #include <linux/property.h>
 #include <linux/uuid.h>
 #include <linux/fw_table.h>
+#include <linux/node.h>
 
 struct irq_domain;
 struct irq_domain_ops;
@@ -430,6 +431,17 @@ extern int acpi_blacklisted(void);
 extern void acpi_osi_setup(char *str);
 extern bool acpi_osi_is_win8(void);
 
+#ifdef CONFIG_ACPI_HMAT
+int acpi_get_genport_coordinates(u8 *device_handle,
+				 struct access_coordinate *coord);
+#else
+static inline int acpi_get_genport_coordinates(u8 *device_handle,
+					       struct access_coordinate *coord)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 #ifdef CONFIG_ACPI_NUMA
 int acpi_map_pxm_to_node(int pxm);
 int acpi_get_node(acpi_handle handle);

Re: [PATCH v3 0/6] acpi: numa: add target support for generic port to HMAT parsing

[Rafael J. Wysocki]

On Thu, Jun 15, 2023 at 11:23 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> v3:
> - Break out ACPI_SRAT_DEVICE_HANDLE_SIZE define. Merged into ACPICA repo.
> - Break out code in hmat_parse_locality() to improve readability. (Jonathan)
> - Drop continue after setting gen target access data. (Jonathan)
> v2:
> - Add check for memory type for setting GENPORT access data. (Jonathan)
> - NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK
> - Change strncmp() to memcmp() for device handle compare
>
> Hi Rafael,
> Please consider acking this series.

Sure.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> Dan will take it through his CXL tree.

Works for me!

> This is the second set of ACPI changes that's part of the "cxl: Add support for
> QTG ID retrieval for CXL subsystem" [1]. It has dependency on the patch [2]
> that was sent to you and GregKH and Greg has ack'd. But it's independent of the
> first set of ACPI changes [3]. The series adds support to store the device handle
> from the SRAT Generic Port Affinity Structure in correlation with a proximity
> domain. And later on the HMAT is parsed and the performance data is stored
> associated with the proximity domain. A helper function is added to allow the CXL
> driver to retrieve the performance data that matches with the device handle later
> on. The full series of CXL code is at this git repo [4] for usage reference.
>
>
> [1]: https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t
> [2]: https://lore.kernel.org/lkml/168332248685.2190392.1983307884583782116.stgit@djiang5-mobl3/
> [3]: https://lore.kernel.org/linux-acpi/168330787964.2042604.17648905811002211147.stgit@djiang5-mobl3/T/#t
> [4]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
>
> ---
>
> Dave Jiang (6):
>       acpi: numa: Create enum for memory_target access coordinates indexing
>       ACPICA: Add a define for size of acpi_srat_generic_affinity DeviceHandle
>       acpi: numa: Add genport target allocation to the HMAT parsing
>       acpi: Break out nesting for hmat_parse_locality()
>       acpi: numa: Add setting of generic port system locality attributes
>       acpi: numa: Add helper function to retrieve the performance attributes
>
>
>  drivers/acpi/numa/hmat.c | 151 +++++++++++++++++++++++++++++++++------
>  include/acpi/actbl3.h    |   4 +-
>  include/linux/acpi.h     |  12 ++++
>  3 files changed, 144 insertions(+), 23 deletions(-)
>
> --
>

Re: [PATCH v3 4/6] acpi: Break out nesting for hmat_parse_locality()

[Jonathan Cameron]

On Thu, 15 Jun 2023 14:24:07 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Refactor hmat_parse_locality() to break up the deep nesting of the
> function.
> 
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/acpi/numa/hmat.c |   32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index e2ab1cce0add..cb240f5233fe 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -320,11 +320,28 @@ static __init void hmat_add_locality(struct acpi_hmat_locality *hmat_loc)
>  	}
>  }
>  
> +static __init void hmat_update_target(unsigned int tgt_pxm, unsigned int init_pxm,
> +				      u8 mem_hier, u8 type, u32 value)
> +{
> +	struct memory_target *target = find_mem_target(tgt_pxm);
> +
> +	if (mem_hier != ACPI_HMAT_MEMORY)
> +		return;
> +
> +	if (target && target->processor_pxm == init_pxm) {
> +		hmat_update_target_access(target, type, value,
> +					  NODE_ACCESS_CLASS_0);
> +		/* If the node has a CPU, update access 1 */
> +		if (node_state(pxm_to_node(init_pxm), N_CPU))
> +			hmat_update_target_access(target, type, value,
> +						  NODE_ACCESS_CLASS_1);
> +	}
> +}
> +
>  static __init int hmat_parse_locality(union acpi_subtable_headers *header,
>  				      const unsigned long end)
>  {
>  	struct acpi_hmat_locality *hmat_loc = (void *)header;
> -	struct memory_target *target;
>  	unsigned int init, targ, total_size, ipds, tpds;
>  	u32 *inits, *targs, value;
>  	u16 *entries;
> @@ -365,17 +382,8 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
>  				inits[init], targs[targ], value,
>  				hmat_data_type_suffix(type));
>  
> -			if (mem_hier == ACPI_HMAT_MEMORY) {
> -				target = find_mem_target(targs[targ]);
> -				if (target && target->processor_pxm == inits[init]) {
> -					hmat_update_target_access(target, type, value,
> -								  NODE_ACCESS_CLASS_0);
> -					/* If the node has a CPU, update access 1 */
> -					if (node_state(pxm_to_node(inits[init]), N_CPU))
> -						hmat_update_target_access(target, type, value,
> -									  NODE_ACCESS_CLASS_1);
> -				}
> -			}
> +			hmat_update_target(targs[targ], inits[init],
> +					   mem_hier, type, value);
>  		}
>  	}
>  
> 
> 
> 

Re: [PATCH v3 3/6] acpi: numa: Add genport target allocation to the HMAT parsing

[Jonathan Cameron]

On Thu, 15 Jun 2023 14:24:01 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add SRAT parsing for the HMAT init in order to collect the device handle
> from the Generic Port Affinity Structure. The device handle will serve as
> the key to search for target data.
> 
> Consoliate the common code with alloc_memory_target() in a helper function
> alloc_target().
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 

On second thoughts - device_handle is rather cryptic as a name
and there are no handy comments on what it is.
Can we call it gen_port_device_handle or something like that?


> ---
> v3:
> - Move ACPI_SRAT_DEVICE_HANDLE_SIZE to separate patch for ACPICA
> ---
>  drivers/acpi/numa/hmat.c |   53 +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index abed728bf09d..e2ab1cce0add 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -71,6 +71,7 @@ struct memory_target {
>  	struct access_coordinate coord[NODE_ACCESS_CLASS_MAX];
>  	struct list_head caches;
>  	struct node_cache_attrs cache_attrs;
> +	u8 device_handle[ACPI_SRAT_DEVICE_HANDLE_SIZE];
>  	bool registered;
>  };
>  
> @@ -125,8 +126,7 @@ static __init void alloc_memory_initiator(unsigned int cpu_pxm)
>  	list_add_tail(&initiator->node, &initiators);
>  }
>  
> -static __init void alloc_memory_target(unsigned int mem_pxm,
> -		resource_size_t start, resource_size_t len)
> +static __init struct memory_target *alloc_target(unsigned int mem_pxm)
>  {
>  	struct memory_target *target;
>  
> @@ -134,7 +134,7 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
>  	if (!target) {
>  		target = kzalloc(sizeof(*target), GFP_KERNEL);
>  		if (!target)
> -			return;
> +			return NULL;
>  		target->memory_pxm = mem_pxm;
>  		target->processor_pxm = PXM_INVAL;
>  		target->memregions = (struct resource) {
> @@ -147,6 +147,19 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
>  		INIT_LIST_HEAD(&target->caches);
>  	}
>  
> +	return target;
> +}
> +
> +static __init void alloc_memory_target(unsigned int mem_pxm,
> +				       resource_size_t start,
> +				       resource_size_t len)
> +{
> +	struct memory_target *target;
> +
> +	target = alloc_target(mem_pxm);
> +	if (!target)
> +		return;
> +
>  	/*
>  	 * There are potentially multiple ranges per PXM, so record each
>  	 * in the per-target memregions resource tree.
> @@ -157,6 +170,17 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
>  				start, start + len, mem_pxm);
>  }
>  
> +static __init void alloc_genport_target(unsigned int mem_pxm, u8 *handle)
> +{
> +	struct memory_target *target;
> +
> +	target = alloc_target(mem_pxm);
> +	if (!target)
> +		return;
> +
> +	memcpy(target->device_handle, handle, ACPI_SRAT_DEVICE_HANDLE_SIZE);
> +}
> +
>  static __init const char *hmat_data_type(u8 type)
>  {
>  	switch (type) {
> @@ -498,6 +522,22 @@ static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
>  	return 0;
>  }
>  
> +static __init int srat_parse_genport_affinity(union acpi_subtable_headers *header,
> +					      const unsigned long end)
> +{
> +	struct acpi_srat_generic_affinity *ga = (void *)header;
> +
> +	if (!ga)
> +		return -EINVAL;
> +
> +	if (!(ga->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED))
> +		return 0;
> +
> +	alloc_genport_target(ga->proximity_domain, (u8 *)ga->device_handle);
> +
> +	return 0;
> +}
> +
>  static u32 hmat_initiator_perf(struct memory_target *target,
>  			       struct memory_initiator *initiator,
>  			       struct acpi_hmat_locality *hmat_loc)
> @@ -848,6 +888,13 @@ static __init int hmat_init(void)
>  				ACPI_SRAT_TYPE_MEMORY_AFFINITY,
>  				srat_parse_mem_affinity, 0) < 0)
>  		goto out_put;
> +
> +	if (acpi_table_parse_entries(ACPI_SIG_SRAT,
> +				     sizeof(struct acpi_table_srat),
> +				     ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY,
> +				     srat_parse_genport_affinity, 0) < 0)
> +		goto out_put;
> +
>  	acpi_put_table(tbl);
>  
>  	status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
> 
> 
> 

Re: [PATCH v3 5/6] acpi: numa: Add setting of generic port system locality attributes

[Jonathan Cameron]

On Thu, 15 Jun 2023 14:24:13 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add generic port support for the parsing of HMAT system locality sub-table.
> The attributes will be added to the third array member of the access
> coordinates in order to not mix with the existing memory attributes. It only
> provides the system locality attributes from initator to the generic port
> targets and is missing the rest of the data to the actual memory device.
> 
> The complete attributes will be updated when a memory device is
> attached and the system locality information is calculated end to end.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
With rename of device_handle, or a comment perhaps, in earlier patch.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> v3:
> - Drop continue after setting gen target access data. (Jonathan)
> v2:
> - Fix commit log runon sentence. (Jonathan)
> - Add a check for memory type for skipping other access levels. (Jonathan)
> - NODE_ACCESS_CLASS_GENPORT to NODE_ACCESS_CLASS_GENPORT_SINK. (Jonathan)
> ---
>  drivers/acpi/numa/hmat.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index cb240f5233fe..32b951cd5ee4 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -60,6 +60,7 @@ struct target_cache {
>  enum {
>  	NODE_ACCESS_CLASS_0 = 0,
>  	NODE_ACCESS_CLASS_1,
> +	NODE_ACCESS_CLASS_GENPORT_SINK,
>  	NODE_ACCESS_CLASS_MAX,
>  };
>  
> @@ -329,6 +330,9 @@ static __init void hmat_update_target(unsigned int tgt_pxm, unsigned int init_px
>  		return;
>  
>  	if (target && target->processor_pxm == init_pxm) {
> +		if (*target->device_handle)

Just for reference - this is where I suddenly wondered what a generic thing
like a device_handle had to do with this path ;)

> +			hmat_update_target_access(target, type, value,
> +						  NODE_ACCESS_CLASS_GENPORT_SINK);
Real nitpick but could we put them in order of the enum... 
That is put this at the end.

>  		hmat_update_target_access(target, type, value,
>  					  NODE_ACCESS_CLASS_0);
>  		/* If the node has a CPU, update access 1 */
> 
> 

Re: [PATCH v3 3/6] acpi: numa: Add genport target allocation to the HMAT parsing

[Dave Jiang]

On 6/22/23 06:45, Jonathan Cameron wrote:
> On Thu, 15 Jun 2023 14:24:01 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> Add SRAT parsing for the HMAT init in order to collect the device handle
>> from the Generic Port Affinity Structure. The device handle will serve as
>> the key to search for target data.
>>
>> Consoliate the common code with alloc_memory_target() in a helper function
>> alloc_target().
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
> 
> On second thoughts - device_handle is rather cryptic as a name
> and there are no handy comments on what it is.
> Can we call it gen_port_device_handle or something like that?

Sure. I can update and send out a new series.

> 
> 
>> ---
>> v3:
>> - Move ACPI_SRAT_DEVICE_HANDLE_SIZE to separate patch for ACPICA
>> ---
>>   drivers/acpi/numa/hmat.c |   53 +++++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 50 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
>> index abed728bf09d..e2ab1cce0add 100644
>> --- a/drivers/acpi/numa/hmat.c
>> +++ b/drivers/acpi/numa/hmat.c
>> @@ -71,6 +71,7 @@ struct memory_target {
>>   	struct access_coordinate coord[NODE_ACCESS_CLASS_MAX];
>>   	struct list_head caches;
>>   	struct node_cache_attrs cache_attrs;
>> +	u8 device_handle[ACPI_SRAT_DEVICE_HANDLE_SIZE];
>>   	bool registered;
>>   };
>>   
>> @@ -125,8 +126,7 @@ static __init void alloc_memory_initiator(unsigned int cpu_pxm)
>>   	list_add_tail(&initiator->node, &initiators);
>>   }
>>   
>> -static __init void alloc_memory_target(unsigned int mem_pxm,
>> -		resource_size_t start, resource_size_t len)
>> +static __init struct memory_target *alloc_target(unsigned int mem_pxm)
>>   {
>>   	struct memory_target *target;
>>   
>> @@ -134,7 +134,7 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
>>   	if (!target) {
>>   		target = kzalloc(sizeof(*target), GFP_KERNEL);
>>   		if (!target)
>> -			return;
>> +			return NULL;
>>   		target->memory_pxm = mem_pxm;
>>   		target->processor_pxm = PXM_INVAL;
>>   		target->memregions = (struct resource) {
>> @@ -147,6 +147,19 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
>>   		INIT_LIST_HEAD(&target->caches);
>>   	}
>>   
>> +	return target;
>> +}
>> +
>> +static __init void alloc_memory_target(unsigned int mem_pxm,
>> +				       resource_size_t start,
>> +				       resource_size_t len)
>> +{
>> +	struct memory_target *target;
>> +
>> +	target = alloc_target(mem_pxm);
>> +	if (!target)
>> +		return;
>> +
>>   	/*
>>   	 * There are potentially multiple ranges per PXM, so record each
>>   	 * in the per-target memregions resource tree.
>> @@ -157,6 +170,17 @@ static __init void alloc_memory_target(unsigned int mem_pxm,
>>   				start, start + len, mem_pxm);
>>   }
>>   
>> +static __init void alloc_genport_target(unsigned int mem_pxm, u8 *handle)
>> +{
>> +	struct memory_target *target;
>> +
>> +	target = alloc_target(mem_pxm);
>> +	if (!target)
>> +		return;
>> +
>> +	memcpy(target->device_handle, handle, ACPI_SRAT_DEVICE_HANDLE_SIZE);
>> +}
>> +
>>   static __init const char *hmat_data_type(u8 type)
>>   {
>>   	switch (type) {
>> @@ -498,6 +522,22 @@ static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
>>   	return 0;
>>   }
>>   
>> +static __init int srat_parse_genport_affinity(union acpi_subtable_headers *header,
>> +					      const unsigned long end)
>> +{
>> +	struct acpi_srat_generic_affinity *ga = (void *)header;
>> +
>> +	if (!ga)
>> +		return -EINVAL;
>> +
>> +	if (!(ga->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED))
>> +		return 0;
>> +
>> +	alloc_genport_target(ga->proximity_domain, (u8 *)ga->device_handle);
>> +
>> +	return 0;
>> +}
>> +
>>   static u32 hmat_initiator_perf(struct memory_target *target,
>>   			       struct memory_initiator *initiator,
>>   			       struct acpi_hmat_locality *hmat_loc)
>> @@ -848,6 +888,13 @@ static __init int hmat_init(void)
>>   				ACPI_SRAT_TYPE_MEMORY_AFFINITY,
>>   				srat_parse_mem_affinity, 0) < 0)
>>   		goto out_put;
>> +
>> +	if (acpi_table_parse_entries(ACPI_SIG_SRAT,
>> +				     sizeof(struct acpi_table_srat),
>> +				     ACPI_SRAT_TYPE_GENERIC_PORT_AFFINITY,
>> +				     srat_parse_genport_affinity, 0) < 0)
>> +		goto out_put;
>> +
>>   	acpi_put_table(tbl);
>>   
>>   	status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
>>
>>
>>
>