[PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Jonathan Cameron]

Introduces a new type of NUMA node for cases where we want to represent
the access characteristics of a non CPU initiator of memory requests,
as these differ from all those for existing nodes containing CPUs and/or
memory.

These Generic Initiators are presented by the node access0 class in
sysfs in the same way as a CPU.   It seems likely that there will be
usecases in which the best 'CPU' is desired and Generic Initiators
should be ignored.  The final few patches in this series introduced
access1 which is a new performance class in the sysfs node description
which presents only CPU to memory relationships.  Test cases for this
are described below.

Thanks to Dan for suggestions on V5.  Most of the changes are
an attempt to implement what was discussed in that thread.

The new patch makes it clear that some of the existing naming is perhaps
more specific than it should be. It may be worth a follow up patch
to rename from *cpu* to *initiator* in a few places where this might
cause confusion.

One outstanding question to highlight in this series is whether
we should assume all ACPI supporting architectures support Generic
Initiator domains, or whether to introduce an
ARCH_HAS_GENERIC_INITIATOR_DOMAINS entry in Kconfig.

Changes since V5:

3 new patches:
* A fix for a subtlety in how ACPI 6.3 changed part of the HMAT table.
* Introduction of access1 class to represent characteristics between CPU
  and memory, ingnoring GIs unlike access0 which includes them.
* Docs to describe the new access0 class.

Note that I ran a number of test cases for the new class which are
described at the end of this email.

Changes since V4:

At Rafael's suggestion:

Rebase on top of Dan William's Specific Purpose Memory series as that
moves srat.c Original patches cherry-picked fine onto mmotm with Dan's
patches applied.

Applies to mmotm-2019-09-25 +
https://lore.kernel.org/linux-acpi/156140036490.2951909.1837804994781523185.stgit@dwillia2-desk3.amr.corp.intel.com/
[PATCH v4 00/10] EFI Specific Purpose Memory Support
(note there are some trivial conflicts to deal with when applying
the SPM series).

Change since V3.
* Rebase.

Changes since RFC V2.
* RFC dropped as now we have x86 support, so the lack of guards in in the
  ACPI code etc should now be fine.
  * Added x86 support.  Note this has only been tested on QEMU as I don't have
    a convenient x86 NUMA machine to play with.  Note that this fitted together
      rather differently from arm64 so I'm particularly interested in feedback
        on the two solutions.

Since RFC V1.
* Fix incorrect interpretation of the ACPI entry noted by Keith Busch
* Use the acpica headers definitions that are now in mmotm.

It's worth noting that, to safely put a given device in a GI node, may
require changes to the existing drivers as it's not unusual to assume
you have local memory or processor core. There may be further constraints
not yet covered by this patch.

Original cover letter...

ACPI 6.3 introduced a new entity that can be part of a NUMA proximity domain.
It may share such a domain with the existing options (memory, CPU etc) but it
may also exist on it's own.

The intent is to allow the description of the NUMA properties (particularly
via HMAT) of accelerators and other initiators of memory activity that are not
the host processor running the operating system.

This patch set introduces 'just enough' to make them work for arm64 and x86.
It should be trivial to support other architectures, I just don't suitable
NUMA systems readily available to test.

There are a few quirks that need to be considered.

1. Fall back nodes
******************

As pre ACPI 6.3 supporting operating systems do not have Generic Initiator
Proximity Domains it is possible to specify, via _PXM in DSDT that another
device is part of such a GI only node.  This currently blows up spectacularly.

Whilst we can obviously 'now' protect against such a situation (see the related
thread on PCI _PXM support and the  threadripper board identified there as
also falling into the  problem of using non existent nodes
https://patchwork.kernel.org/patch/10723311/ ), there is no way to  be sure
we will never have legacy OSes that are not protected  against this.  It would
also be 'non ideal' to fallback to  a default node as there may be a better
(non GI) node to pick  if GI nodes aren't available.

The work around is that we also have a new system wide OSC bit that allows
an operating system to 'announce' that it supports Generic Initiators.  This
allows, the firmware to us DSDT magic to 'move' devices between the nodes
dependent on whether our new nodes are there or not.

2. New ways of assigning a proximity domain for devices
*******************************************************

Until now, the only way firmware could indicate that a particular device
(outside the 'special' set of cpus etc) was to be found in a particular
Proximity Domain by the use of _PXM in DSDT.

That is equally valid with GI domains, but we have new options. The SRAT
affinity structure includes a handle (ACPI or PCI) to identify devices
with the system and specify their proximity domain that way.  If both _PXM
and this are provided, they should give the same answer.

For now this patch set completely ignores that feature as we don't need
it to start the discussion.  It will form a follow up set at some point
(if no one else fancies doing it).

Test cases for the access1 class
********************************

Test cases for Generic Initiator additions to HMAT.

Setup

PXM0 (node 0) - CPU0 CPU1, 2G memory
PXM1 (node 1) - CPU2 CPU3, 2G memory
PXM2 (node 2) - CPU4 CPU5, 2G memory
PXM3 (node 4) - 2G memory (GI in one case below)
PXM4 (node 3) - GI only.

Config 1:  GI in PXM4 nearer to memory in PXM 3 than CPUs, not direct attached

[    2.384064] acpi/hmat: HMAT: Locality: Flags:00 Type:Access Latency Initiator Domains:4 Target Domains:4 Base:256
[    2.384913] acpi/hmat:   Initiator-Target[0-0]:1 nsec
[    2.385190] acpi/hmat:   Initiator-Target[0-1]:9 nsec
[    2.385736] acpi/hmat:   Initiator-Target[0-2]:9 nsec
[    2.385984] acpi/hmat:   Initiator-Target[0-3]:9 nsec
[    2.386447] acpi/hmat:   Initiator-Target[1-0]:9 nsec
[    2.386740] acpi/hmat:   Initiator-Target[1-1]:1 nsec
[    2.386964] acpi/hmat:   Initiator-Target[1-2]:9 nsec
[    2.387174] acpi/hmat:   Initiator-Target[1-3]:9 nsec
[    2.387624] acpi/hmat:   Initiator-Target[2-0]:9 nsec
[    2.387953] acpi/hmat:   Initiator-Target[2-1]:9 nsec
[    2.388155] acpi/hmat:   Initiator-Target[2-2]:1 nsec
[    2.388607] acpi/hmat:   Initiator-Target[2-3]:9 nsec
[    2.388861] acpi/hmat:   Initiator-Target[4-0]:13 nsec
[    2.389126] acpi/hmat:   Initiator-Target[4-1]:13 nsec
[    2.389574] acpi/hmat:   Initiator-Target[4-2]:13 nsec
[    2.389805] acpi/hmat:   Initiator-Target[4-3]:5 nsec

# Sysfs reads the same for nodes 0-2 for access0 and access1 as no GI involved.

/sys/bus/node/devices/...
    node0 #1 and 2 similar.
        access0
            initiators
                node0
                read_bandwidth  0 #not specificed in hmat
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node0
            uevent
        access1
            initiators
                node0
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                read_bandwidth  1   
            power
            targets
                node 0
            uevent
        compact
        cpu0
        cpu1
        ...
    node3 # Note PXM 4, contains GI only
        access0
            initiators
                *empty*
            power
            targets
                node4
            uevent
        compact
        ...
    node4
        access0
            initiators
                node3
                read_bandwidth  0
                read_latency    5
                write_bandwidth 0
                write_latency   5
            power
            targets
                *empty*
            uevent
        access1
            initiators
                node0
                node1
                node2
                read_bandwidth  0
                read_latency    9
                write_bandwidth 0
                write_latency   9
            power
            targets
                *empty*
            uevent
        compact
        ...

Config 2:  GI in PXM4 further to memory in PXM 3 than CPUs, not direct attached

[    4.073493] acpi/hmat: HMAT: Locality: Flags:00 Type:Access Latency Initiator Domains:4 Target Domains:4 Base:256
[    4.074785] acpi/hmat:   Initiator-Target[0-0]:1 nsec
[    4.075150] acpi/hmat:   Initiator-Target[0-1]:9 nsec
[    4.075423] acpi/hmat:   Initiator-Target[0-2]:9 nsec
[    4.076184] acpi/hmat:   Initiator-Target[0-3]:9 nsec
[    4.077116] acpi/hmat:   Initiator-Target[1-0]:9 nsec
[    4.077366] acpi/hmat:   Initiator-Target[1-1]:1 nsec
[    4.077640] acpi/hmat:   Initiator-Target[1-2]:9 nsec
[    4.078156] acpi/hmat:   Initiator-Target[1-3]:9 nsec
[    4.078471] acpi/hmat:   Initiator-Target[2-0]:9 nsec
[    4.078994] acpi/hmat:   Initiator-Target[2-1]:9 nsec
[    4.079277] acpi/hmat:   Initiator-Target[2-2]:1 nsec
[    4.079505] acpi/hmat:   Initiator-Target[2-3]:9 nsec
[    4.080126] acpi/hmat:   Initiator-Target[4-0]:13 nsec
[    4.080995] acpi/hmat:   Initiator-Target[4-1]:13 nsec
[    4.081351] acpi/hmat:   Initiator-Target[4-2]:13 nsec
[    4.082125] acpi/hmat:   Initiator-Target[4-3]:13 nsec

/sys/bus/node/devices/...
    node0 #1 and 2 similar.
        access0
            initiators
                node0
                read_bandwidth  0 #not specificed in hmat
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node0
                node4
            uevent
        access1
            initiators
                node0
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                read_bandwidth  1   
            power
            targets
                node0
                node4
            uevent
        compact
        cpu0
        cpu1
        ...
    node3 # Note PXM 4, contains GI only
        #No accessX directories.
        compact
        ...
    node4
        access0
            initiators
                node0
                node1
                node2
                read_bandwidth  0
                read_latency    9
                write_bandwidth 0
                write_latency   9
            power
            targets
                *empty*
            uevent
        access1
            initiators
                node0
                node1
                node2
                read_bandwidth  0
                read_latency    9
                write_bandwidth 0
                write_latency   9
            power
            targets
                *empty*
            uevent
        compact
        ...


case 3 - as per case 2 but now the memory in node 3 is direct attached to the
GI but nearer the main nodes (not physically sensible :))

/sys/bus/node/devices/...
    node0 #1 and 2 similar.
        access0
            initiators
                node0
                read_bandwidth  0 #not specificed in hmat
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node0
                node4
            uevent
        access1
            initiators
                node0
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                read_bandwidth  1   
            power
            targets
                node0
                node4
            uevent
        compact
        cpu0
        cpu1
        ...
    node3 # Note PXM 4, contains GI only
        access0
            initiators
                *empty*
            power
            targets
                node4
            uevent
        compact
        ...
    node4
        access0
            initiators
                node3
                read_bandwidth  0
                read_latency    13
                write_bandwidth 0
                write_latency   13
            power
            targets
                *empty*
            uevent
        access1
            initiators
                node0
                node1
                node2
                read_bandwidth  0
                read_latency    9
                write_bandwidth 0
                write_latency   9
            power
            targets
                *empty*
            uevent
        compact
        ...

Case 4 - nearer the GI, but direct attached to one of the CPUS.
# Another bonkers one.

/sys/bus/node/devices/...
    node0 #1 similar.
        access0
            initiators
                node0
                read_bandwidth  0 #not specificed in hmat
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node0
                node4
            uevent
        access1
            initiators
                node0
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                read_bandwidth  1   
            power
            targets
                node0
            uevent
        compact
        cpu0
        cpu1
        ...
    node2 # Direct attached to memory in node 3
        access0
            initiators
                node2
                read_bandwidth  0 #not specificed in hmat
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node2
                node4 #direct attached
            uevent
        access1
            initiators
                node2
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                read_bandwidth  1   
            power
            targets
                node2
                node4 #direct attached
            uevent
        compact
        cpu0
        cpu1
        ...

    node3 # Note PXM 4, contains GI only
        #No accessX directories.
        compact
        ...
    node4
        access0
            initiators
                node3
                read_bandwidth  0
                read_latency    13
                write_bandwidth 0
                write_latency   13
            power
            targets
                *empty*
            uevent
        access1
            initiators
                node0
                node1
                node2
                read_bandwidth  0
                read_latency    9
                write_bandwidth 0
                write_latency   9
            power
            targets
                *empty*
            uevent
        compact
        ...

case 5 memory and GI together in node 3 (added an extra GI to node 3)
Note hmat should also reflect this extra initiator domain.

/sys/bus/node/devices/...
    node0 #1 and 2 similar.
        access0
            initiators
                node0
                read_bandwidth  0 #not specificed in hmat
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node0
                node4
            uevent
        access1
            initiators
                node0
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                read_bandwidth  1   
            power
            targets
                node0
            uevent
        compact
        cpu0
        cpu1
        ...
    node3 # Note PXM 3, contains GI only
        #No accessX directories.
        compact
        ...
    node4 # Now memory and GI.
        access0
            initiators
                node4
                read_bandwidth  0
                read_latency    1
                write_bandwidth 0
                write_latency   1
            power
            targets
                node4
            uevent
        access1
            initiators
                node0
                node1
                node2
                read_bandwidth  0
                read_latency    9
                write_bandwidth 0
                write_latency   9
            power
            targets
                *empty* # as expected GI doesn't paticipate in access 1.
            uevent
        compact
        ...

Jonathan Cameron (7):
  ACPI: Support Generic Initiator only domains
  arm64: Support Generic Initiator only domains
  x86: Support Generic Initiator only proximity domains
  ACPI: Let ACPI know we support Generic Initiator Affinity Structures
  ACPI: HMAT: Fix handling of changes from ACPI 6.2 to ACPI 6.3
  node: Add access1 class to represent CPU to memory characteristics
  docs: mm: numaperf.rst Add brief description for access class 1.

 Documentation/admin-guide/mm/numaperf.rst |  8 ++
 arch/arm64/kernel/smp.c                   |  8 ++
 arch/x86/include/asm/numa.h               |  2 +
 arch/x86/kernel/setup.c                   |  1 +
 arch/x86/mm/numa.c                        | 14 ++++
 drivers/acpi/bus.c                        |  1 +
 drivers/acpi/numa/hmat.c                  | 89 ++++++++++++++++++-----
 drivers/acpi/numa/srat.c                  | 62 +++++++++++++++-
 drivers/base/node.c                       |  3 +
 include/asm-generic/topology.h            |  3 +
 include/linux/acpi.h                      |  1 +
 include/linux/nodemask.h                  |  1 +
 include/linux/topology.h                  |  7 ++
 13 files changed, 179 insertions(+), 21 deletions(-)

-- 
2.19.1

[PATCH V6 1/7] ACPI: Support Generic Initiator only domains

[Jonathan Cameron]

Generic Initiators are a new ACPI concept that allows for the
description of proximity domains that contain a device which
performs memory access (such as a network card) but neither
host CPU nor Memory.

This patch has the parsing code and provides the infrastructure
for an architecture to associate these new domains with their
nearest memory processing node.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/acpi/numa/srat.c       | 62 +++++++++++++++++++++++++++++++++-
 drivers/base/node.c            |  3 ++
 include/asm-generic/topology.h |  3 ++
 include/linux/nodemask.h       |  1 +
 include/linux/topology.h       |  7 ++++
 5 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index eadbf90e65d1..a4a2cd52e706 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -170,6 +170,38 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 		}
 		break;
 
+	case ACPI_SRAT_TYPE_GENERIC_AFFINITY:
+	{
+		struct acpi_srat_generic_affinity *p =
+			(struct acpi_srat_generic_affinity *)header;
+		char name[9] = {};
+
+		if (p->device_handle_type == 0) {
+			/*
+			 * For pci devices this may be the only place they
+			 * are assigned a proximity domain
+			 */
+			pr_debug("SRAT Generic Initiator(Seg:%u BDF:%u) in proximity domain %d %s\n",
+				 *(u16 *)(&p->device_handle[0]),
+				 *(u16 *)(&p->device_handle[2]),
+				 p->proximity_domain,
+				 (p->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED) ?
+				 "enabled" : "disabled");
+		} else {
+			/*
+			 * In this case we can rely on the device having a
+			 * proximity domain reference
+			 */
+			memcpy(name, p->device_handle, 8);
+			pr_debug("SRAT Generic Initiator(HID=%.8s UID=%.4s) in proximity domain %d %s\n",
+				 (char *)(&p->device_handle[0]),
+				 (char *)(&p->device_handle[8]),
+				 p->proximity_domain,
+				 (p->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED) ?
+				 "enabled" : "disabled");
+		}
+	}
+	break;
 	default:
 		pr_warn("Found unsupported SRAT entry (type = 0x%x)\n",
 			header->type);
@@ -378,6 +410,32 @@ acpi_parse_gicc_affinity(union acpi_subtable_headers *header,
 	return 0;
 }
 
+static int __init
+acpi_parse_gi_affinity(union acpi_subtable_headers *header,
+		       const unsigned long end)
+{
+	struct acpi_srat_generic_affinity *gi_affinity;
+	int node;
+
+	gi_affinity = (struct acpi_srat_generic_affinity *)header;
+	if (!gi_affinity)
+		return -EINVAL;
+	acpi_table_print_srat_entry(&header->common);
+
+	if (!(gi_affinity->flags & ACPI_SRAT_GENERIC_AFFINITY_ENABLED))
+		return -EINVAL;
+
+	node = acpi_map_pxm_to_node(gi_affinity->proximity_domain);
+	if (node == NUMA_NO_NODE || node >= MAX_NUMNODES) {
+		pr_err("SRAT: Too many proximity domains.\n");
+		return -EINVAL;
+	}
+	node_set(node, numa_nodes_parsed);
+	node_set_state(node, N_GENERIC_INITIATOR);
+
+	return 0;
+}
+
 static int __initdata parsed_numa_memblks;
 
 static int __init
@@ -433,7 +491,7 @@ int __init acpi_numa_init(void)
 
 	/* SRAT: System Resource Affinity Table */
 	if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
-		struct acpi_subtable_proc srat_proc[3];
+		struct acpi_subtable_proc srat_proc[4];
 
 		memset(srat_proc, 0, sizeof(srat_proc));
 		srat_proc[0].id = ACPI_SRAT_TYPE_CPU_AFFINITY;
@@ -442,6 +500,8 @@ int __init acpi_numa_init(void)
 		srat_proc[1].handler = acpi_parse_x2apic_affinity;
 		srat_proc[2].id = ACPI_SRAT_TYPE_GICC_AFFINITY;
 		srat_proc[2].handler = acpi_parse_gicc_affinity;
+		srat_proc[3].id = ACPI_SRAT_TYPE_GENERIC_AFFINITY;
+		srat_proc[3].handler = acpi_parse_gi_affinity;
 
 		acpi_table_parse_entries_array(ACPI_SIG_SRAT,
 					sizeof(struct acpi_table_srat),
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 98a31bafc8a2..f6e80bf22547 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -974,6 +974,8 @@ static struct node_attr node_state_attr[] = {
 #endif
 	[N_MEMORY] = _NODE_ATTR(has_memory, N_MEMORY),
 	[N_CPU] = _NODE_ATTR(has_cpu, N_CPU),
+	[N_GENERIC_INITIATOR] = _NODE_ATTR(has_generic_initiator,
+					   N_GENERIC_INITIATOR),
 };
 
 static struct attribute *node_state_attrs[] = {
@@ -985,6 +987,7 @@ static struct attribute *node_state_attrs[] = {
 #endif
 	&node_state_attr[N_MEMORY].attr.attr,
 	&node_state_attr[N_CPU].attr.attr,
+	&node_state_attr[N_GENERIC_INITIATOR].attr.attr,
 	NULL
 };
 
diff --git a/include/asm-generic/topology.h b/include/asm-generic/topology.h
index 238873739550..54d0b4176a45 100644
--- a/include/asm-generic/topology.h
+++ b/include/asm-generic/topology.h
@@ -71,6 +71,9 @@
 #ifndef set_cpu_numa_mem
 #define set_cpu_numa_mem(cpu, node)
 #endif
+#ifndef set_gi_numa_mem
+#define set_gi_numa_mem(gi, node)
+#endif
 
 #endif	/* !CONFIG_NUMA || !CONFIG_HAVE_MEMORYLESS_NODES */
 
diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
index 27e7fa36f707..1aebf766fb52 100644
--- a/include/linux/nodemask.h
+++ b/include/linux/nodemask.h
@@ -399,6 +399,7 @@ enum node_states {
 #endif
 	N_MEMORY,		/* The node has memory(regular, high, movable) */
 	N_CPU,		/* The node has one or more cpus */
+	N_GENERIC_INITIATOR,	/* The node is a GI only node */
 	NR_NODE_STATES
 };
 
diff --git a/include/linux/topology.h b/include/linux/topology.h
index eb2fe6edd73c..05ccf011e489 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -140,6 +140,13 @@ static inline void set_numa_mem(int node)
 }
 #endif
 
+#ifndef set_gi_numa_mem
+static inline void set_gi_numa_mem(int gi, int node)
+{
+	_node_numa_mem_[gi] = node;
+}
+#endif
+
 #ifndef node_to_mem_node
 static inline int node_to_mem_node(int node)
 {
-- 
2.19.1

[PATCH V6 2/7] arm64: Support Generic Initiator only domains

[Jonathan Cameron]

The one thing that currently needs doing from an architecture
point of view is associating the GI domain with its nearest
memory domain.  This allows all the standard NUMA aware code
to get a 'reasonable' answer.

A clever driver might elect to do load balancing etc
if there are multiple host / memory domains nearby, but
that's a decision for the driver.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 arch/arm64/kernel/smp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index d4ed9a19d8fe..eb5ef84fe7b9 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -716,6 +716,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	int err;
 	unsigned int cpu;
+	unsigned int node;
 	unsigned int this_cpu;
 
 	init_cpu_topology();
@@ -754,6 +755,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 		set_cpu_present(cpu, true);
 		numa_store_cpu_info(cpu);
 	}
+
+	/*
+	 * Walk the numa domains and set the node to numa memory reference
+	 * for any that are Generic Initiator Only.
+	 */
+	for_each_node_state(node, N_GENERIC_INITIATOR)
+		set_gi_numa_mem(node, local_memory_node(node));
 }
 
 void (*__smp_cross_call)(const struct cpumask *, unsigned int);
-- 
2.19.1

[PATCH V6 3/7] x86: Support Generic Initiator only proximity domains

[Jonathan Cameron]

Done in a somewhat different fashion to arm64.
Here the infrastructure for memoryless domains was already
in place.  That infrastruture applies just as well to
domains that also don't have a CPU, hence it works for
Generic Initiator Domains.

In common with memoryless domains we only register GI domains
if the proximity node is not online. If a domain is already
a memory containing domain, or a memoryless domain there is
nothing to do just because it also contains a Generic Initiator.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 arch/x86/include/asm/numa.h |  2 ++
 arch/x86/kernel/setup.c     |  1 +
 arch/x86/mm/numa.c          | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/x86/include/asm/numa.h b/arch/x86/include/asm/numa.h
index bbfde3d2662f..f631467272a3 100644
--- a/arch/x86/include/asm/numa.h
+++ b/arch/x86/include/asm/numa.h
@@ -62,12 +62,14 @@ extern void numa_clear_node(int cpu);
 extern void __init init_cpu_to_node(void);
 extern void numa_add_cpu(int cpu);
 extern void numa_remove_cpu(int cpu);
+extern void init_gi_nodes(void);
 #else	/* CONFIG_NUMA */
 static inline void numa_set_node(int cpu, int node)	{ }
 static inline void numa_clear_node(int cpu)		{ }
 static inline void init_cpu_to_node(void)		{ }
 static inline void numa_add_cpu(int cpu)		{ }
 static inline void numa_remove_cpu(int cpu)		{ }
+static inline void init_gi_nodes(void)			{ }
 #endif	/* CONFIG_NUMA */
 
 #ifdef CONFIG_DEBUG_PER_CPU_MAPS
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index cedfe2077a69..c21fc5b9f729 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1279,6 +1279,7 @@ void __init setup_arch(char **cmdline_p)
 	prefill_possible_map();
 
 	init_cpu_to_node();
+	init_gi_nodes();
 
 	io_apic_init_mappings();
 
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 99f7a68738f0..53ac09f6138c 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -733,6 +733,20 @@ static void __init init_memory_less_node(int nid)
 	 */
 }
 
+/*
+ * Generic Initiator Nodes may have neither CPU nor Memory.
+ * At this stage if either of the others were present we would
+ * already be online.
+ */
+void __init init_gi_nodes(void)
+{
+	int nid;
+
+	for_each_node_state(nid, N_GENERIC_INITIATOR)
+		if (!node_online(nid))
+			init_memory_less_node(nid);
+}
+
 /*
  * Setup early cpu_to_node.
  *
-- 
2.19.1

[PATCH V6 4/7] ACPI: Let ACPI know we support Generic Initiator Affinity Structures

[Jonathan Cameron]

Until we tell ACPI that we support generic initiators, it will have
to operate in fall back domain mode and all _PXM entries should
be on existing non GI domains.

This patch sets the relevant OSC bit to make that happen.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/acpi/bus.c   | 1 +
 include/linux/acpi.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 54002670cb7a..0ac96fb67515 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -302,6 +302,7 @@ static void acpi_bus_osc_support(void)
 
 	capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
 	capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
+	capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT;
 
 #ifdef CONFIG_X86
 	if (boot_cpu_has(X86_FEATURE_HWP)) {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0f37a7d5fa77..7dba8ffdc68a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -505,6 +505,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
 #define OSC_SB_PCLPI_SUPPORT			0x00000080
 #define OSC_SB_OSLPI_SUPPORT			0x00000100
 #define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT		0x00001000
+#define OSC_SB_GENERIC_INITIATOR_SUPPORT	0x00002000
 
 extern bool osc_sb_apei_support_acked;
 extern bool osc_pc_lpi_support_confirmed;
-- 
2.19.1

[PATCH V6 5/7] ACPI: HMAT: Fix handling of changes from ACPI 6.2 to ACPI 6.3

[Jonathan Cameron]

In ACPI 6.3, the Memory Proximity Domain Attributes Structure
changed substantially.  One of those changes was that the flag
for "Memory Proximity Domain field is valid" was deprecated.

This was because the field "Proximity Domain for the Memory"
became a required field and hence having a validity flag makes
no sense.

So the correct logic is to always assume the field is there.
Current code assumes it never is.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/acpi/numa/hmat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 2c32cfb72370..07cfe50136e0 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -424,7 +424,7 @@ static int __init hmat_parse_proximity_domain(union acpi_subtable_headers *heade
 		pr_info("HMAT: Memory Flags:%04x Processor Domain:%u Memory Domain:%u\n",
 			p->flags, p->processor_PD, p->memory_PD);
 
-	if (p->flags & ACPI_HMAT_MEMORY_PD_VALID && hmat_revision == 1) {
+	if ((p->flags & ACPI_HMAT_MEMORY_PD_VALID && hmat_revision == 1) || hmat_revision == 2) {
 		target = find_mem_target(p->memory_PD);
 		if (!target) {
 			pr_debug("HMAT: Memory Domain missing from SRAT\n");
-- 
2.19.1

[PATCH V6 6/7] node: Add access1 class to represent CPU to memory characteristics

[Jonathan Cameron]

New access1 class is nearly the same as access0, but always provides
characteristics for CPUs to memory.   The existing access0 class
provides characteristics to nearest or direct connnect initiator
which may be a Generic Initiator such as a GPU or network adapter.

This new class allows thread placement on CPUs to be performed
so as to give optimal access characteristics to memory, even if that
memory is for example attached to a GPU or similar and only accessible
to the CPU via an appropriate bus.

Suggested-by: Dan Willaims <dan.j.williams@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---

Note that this code could have been shorter by copying the bitmap
and factoring out the generic parts of access0 and access1.
Personally I felt that reduced readability but happy to change that
if people prefer.

 drivers/acpi/numa/hmat.c | 87 +++++++++++++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 07cfe50136e0..00b4cdbefb5e 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -56,7 +56,7 @@ struct memory_target {
 	unsigned int memory_pxm;
 	unsigned int processor_pxm;
 	struct resource memregions;
-	struct node_hmem_attrs hmem_attrs;
+	struct node_hmem_attrs hmem_attrs[2];
 	struct list_head caches;
 	struct node_cache_attrs cache_attrs;
 	bool registered;
@@ -65,6 +65,7 @@ struct memory_target {
 struct memory_initiator {
 	struct list_head node;
 	unsigned int processor_pxm;
+	bool has_cpu;
 };
 
 struct memory_locality {
@@ -108,6 +109,7 @@ static __init void alloc_memory_initiator(unsigned int cpu_pxm)
 		return;
 
 	initiator->processor_pxm = cpu_pxm;
+	initiator->has_cpu = node_state(pxm_to_node(cpu_pxm), N_CPU);
 	list_add_tail(&initiator->node, &initiators);
 }
 
@@ -215,28 +217,28 @@ static u32 hmat_normalize(u16 entry, u64 base, u8 type)
 }
 
 static void hmat_update_target_access(struct memory_target *target,
-					     u8 type, u32 value)
+				      u8 type, u32 value, int access)
 {
 	switch (type) {
 	case ACPI_HMAT_ACCESS_LATENCY:
-		target->hmem_attrs.read_latency = value;
-		target->hmem_attrs.write_latency = value;
+		target->hmem_attrs[access].read_latency = value;
+		target->hmem_attrs[access].write_latency = value;
 		break;
 	case ACPI_HMAT_READ_LATENCY:
-		target->hmem_attrs.read_latency = value;
+		target->hmem_attrs[access].read_latency = value;
 		break;
 	case ACPI_HMAT_WRITE_LATENCY:
-		target->hmem_attrs.write_latency = value;
+		target->hmem_attrs[access].write_latency = value;
 		break;
 	case ACPI_HMAT_ACCESS_BANDWIDTH:
-		target->hmem_attrs.read_bandwidth = value;
-		target->hmem_attrs.write_bandwidth = value;
+		target->hmem_attrs[access].read_bandwidth = value;
+		target->hmem_attrs[access].write_bandwidth = value;
 		break;
 	case ACPI_HMAT_READ_BANDWIDTH:
-		target->hmem_attrs.read_bandwidth = value;
+		target->hmem_attrs[access].read_bandwidth = value;
 		break;
 	case ACPI_HMAT_WRITE_BANDWIDTH:
-		target->hmem_attrs.write_bandwidth = value;
+		target->hmem_attrs[access].write_bandwidth = value;
 		break;
 	default:
 		break;
@@ -329,8 +331,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);
+				if (target && target->processor_pxm == inits[init]) {
+					hmat_update_target_access(target, type, value, 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);
+				}
 			}
 		}
 	}
@@ -566,6 +572,7 @@ static void hmat_register_target_initiators(struct memory_target *target)
 	unsigned int mem_nid, cpu_nid;
 	struct memory_locality *loc = NULL;
 	u32 best = 0;
+	bool access0done = false;
 	int i;
 
 	mem_nid = pxm_to_node(target->memory_pxm);
@@ -577,7 +584,11 @@ 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);
-		return;
+		access0done = true;
+		if (node_state(cpu_nid, N_CPU)) {
+			register_memory_node_under_compute_node(mem_nid, cpu_nid, 1);
+			return;
+		}
 	}
 
 	if (list_empty(&localities))
@@ -591,6 +602,40 @@ static void hmat_register_target_initiators(struct memory_target *target)
 	 */
 	bitmap_zero(p_nodes, MAX_NUMNODES);
 	list_sort(p_nodes, &initiators, initiator_cmp);
+	if (!access0done) {
+		for (i = WRITE_LATENCY; i <= READ_BANDWIDTH; i++) {
+			loc = localities_types[i];
+			if (!loc)
+				continue;
+
+			best = 0;
+			list_for_each_entry(initiator, &initiators, node) {
+				u32 value;
+
+				if (!test_bit(initiator->processor_pxm, p_nodes))
+					continue;
+
+				value = hmat_initiator_perf(target, initiator,
+							    loc->hmat_loc);
+				if (hmat_update_best(loc->hmat_loc->data_type, value, &best))
+					bitmap_clear(p_nodes, 0, initiator->processor_pxm);
+				if (value != best)
+					clear_bit(initiator->processor_pxm, p_nodes);
+			}
+			if (best)
+				hmat_update_target_access(target, loc->hmat_loc->data_type, best, 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);
+		}
+	}
+
+	/* Access 1 ignores Generic Initiators */
+	bitmap_zero(p_nodes, MAX_NUMNODES);
+	list_sort(p_nodes, &initiators, initiator_cmp);
+	best = 0;
 	for (i = WRITE_LATENCY; i <= READ_BANDWIDTH; i++) {
 		loc = localities_types[i];
 		if (!loc)
@@ -600,6 +645,10 @@ static void hmat_register_target_initiators(struct memory_target *target)
 		list_for_each_entry(initiator, &initiators, node) {
 			u32 value;
 
+			if (!initiator->has_cpu) {
+				clear_bit(initiator->processor_pxm, p_nodes);
+				continue;
+			}
 			if (!test_bit(initiator->processor_pxm, p_nodes))
 				continue;
 
@@ -610,12 +659,11 @@ 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);
+			hmat_update_target_access(target, loc->hmat_loc->data_type, best, 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, 0);
+		register_memory_node_under_compute_node(mem_nid, cpu_nid, 1);
 	}
 }
 
@@ -628,10 +676,10 @@ static void hmat_register_target_cache(struct memory_target *target)
 		node_add_cache(mem_nid, &tcache->cache_attrs);
 }
 
-static void hmat_register_target_perf(struct memory_target *target)
+static void hmat_register_target_perf(struct memory_target *target, int access)
 {
 	unsigned mem_nid = pxm_to_node(target->memory_pxm);
-	node_set_perf_attrs(mem_nid, &target->hmem_attrs, 0);
+	node_set_perf_attrs(mem_nid, &target->hmem_attrs[access], access);
 }
 
 static void hmat_register_target_device(struct memory_target *target,
@@ -733,7 +781,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);
+		hmat_register_target_perf(target, 0);
+		hmat_register_target_perf(target, 1);
 		target->registered = true;
 	}
 	mutex_unlock(&target_lock);
-- 
2.19.1

[PATCH V6 7/7] docs: mm: numaperf.rst Add brief description for access class 1.

[Jonathan Cameron]

Try to make minimal changes to the document which already describes
access class 0 in a generic fashion (including IO initiatiors that
are not CPUs).

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 Documentation/admin-guide/mm/numaperf.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/admin-guide/mm/numaperf.rst b/Documentation/admin-guide/mm/numaperf.rst
index a80c3c37226e..327c0d72692d 100644
--- a/Documentation/admin-guide/mm/numaperf.rst
+++ b/Documentation/admin-guide/mm/numaperf.rst
@@ -56,6 +56,11 @@ nodes' access characteristics share the same performance relative to other
 linked initiator nodes. Each target within an initiator's access class,
 though, do not necessarily perform the same as each other.
 
+The access class "1" is used to allow differentiation between initiators
+that are CPUs and hence suitable for generic task scheduling, and
+IO initiators such as GPUs and CPUs.  Unlike access class 0, only
+nodes containing CPUs are considered.
+
 ================
 NUMA Performance
 ================
@@ -88,6 +93,9 @@ The latency attributes are provided in nanoseconds.
 The values reported here correspond to the rated latency and bandwidth
 for the platform.
 
+Access class 0, takes the same form, but only includes values for CPU to
+memory activity.
+
 ==========
 NUMA Cache
 ==========
-- 
2.19.1

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Brice Goglin]

Le 16/12/2019 à 16:38, Jonathan Cameron a écrit :
> Introduces a new type of NUMA node for cases where we want to represent
> the access characteristics of a non CPU initiator of memory requests,
> as these differ from all those for existing nodes containing CPUs and/or
> memory.
>
> These Generic Initiators are presented by the node access0 class in
> sysfs in the same way as a CPU.   It seems likely that there will be
> usecases in which the best 'CPU' is desired and Generic Initiators
> should be ignored.  The final few patches in this series introduced
> access1 which is a new performance class in the sysfs node description
> which presents only CPU to memory relationships.  Test cases for this
> are described below.


Hello Jonathan

If I want to test this with a fake GI, what are the minimal set of
changes I should put in my ACPI tables? Can I just specify a dummy GI in
SRAT? What handle should I use there?

Thanks

Brice

Re: [PATCH V6 7/7] docs: mm: numaperf.rst Add brief description for access class 1.

[Brice Goglin]

Le 16/12/2019 à 16:38, Jonathan Cameron a écrit :
> Try to make minimal changes to the document which already describes
> access class 0 in a generic fashion (including IO initiatiors that
> are not CPUs).
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  Documentation/admin-guide/mm/numaperf.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/admin-guide/mm/numaperf.rst b/Documentation/admin-guide/mm/numaperf.rst
> index a80c3c37226e..327c0d72692d 100644
> --- a/Documentation/admin-guide/mm/numaperf.rst
> +++ b/Documentation/admin-guide/mm/numaperf.rst
> @@ -56,6 +56,11 @@ nodes' access characteristics share the same performance relative to other
>  linked initiator nodes. Each target within an initiator's access class,
>  though, do not necessarily perform the same as each other.
>  
> +The access class "1" is used to allow differentiation between initiators
> +that are CPUs and hence suitable for generic task scheduling, and
> +IO initiators such as GPUs and CPUs.  Unlike access class 0, only
> +nodes containing CPUs are considered.
> +
>  ================
>  NUMA Performance
>  ================
> @@ -88,6 +93,9 @@ The latency attributes are provided in nanoseconds.
>  The values reported here correspond to the rated latency and bandwidth
>  for the platform.
>  
> +Access class 0, takes the same form, but only includes values for CPU to
> +memory activity.


Shouldn't this be "class 1" here?

Both hunks look contradictory to me.

Brice

Re: [PATCH V6 7/7] docs: mm: numaperf.rst Add brief description for access class 1.

[Jonathan Cameron]

On Wed, 18 Dec 2019 12:34:34 +0100
Brice Goglin <brice.goglin@gmail.com> wrote:

> Le 16/12/2019 à 16:38, Jonathan Cameron a écrit :
> > Try to make minimal changes to the document which already describes
> > access class 0 in a generic fashion (including IO initiatiors that
> > are not CPUs).
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  Documentation/admin-guide/mm/numaperf.rst | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/mm/numaperf.rst b/Documentation/admin-guide/mm/numaperf.rst
> > index a80c3c37226e..327c0d72692d 100644
> > --- a/Documentation/admin-guide/mm/numaperf.rst
> > +++ b/Documentation/admin-guide/mm/numaperf.rst
> > @@ -56,6 +56,11 @@ nodes' access characteristics share the same performance relative to other
> >  linked initiator nodes. Each target within an initiator's access class,
> >  though, do not necessarily perform the same as each other.
> >  
> > +The access class "1" is used to allow differentiation between initiators
> > +that are CPUs and hence suitable for generic task scheduling, and
> > +IO initiators such as GPUs and CPUs.  Unlike access class 0, only
> > +nodes containing CPUs are considered.
> > +
> >  ================
> >  NUMA Performance
> >  ================
> > @@ -88,6 +93,9 @@ The latency attributes are provided in nanoseconds.
> >  The values reported here correspond to the rated latency and bandwidth
> >  for the platform.
> >  
> > +Access class 0, takes the same form, but only includes values for CPU to
> > +memory activity.  
> 
> 
> Shouldn't this be "class 1" here?
> 
Good point.

Jonathan

> Both hunks look contradictory to me.
> 
> Brice
> 
> 

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Jonathan Cameron]

On Wed, 18 Dec 2019 12:32:06 +0100
Brice Goglin <brice.goglin@gmail.com> wrote:

> Le 16/12/2019 à 16:38, Jonathan Cameron a écrit :
> > Introduces a new type of NUMA node for cases where we want to represent
> > the access characteristics of a non CPU initiator of memory requests,
> > as these differ from all those for existing nodes containing CPUs and/or
> > memory.
> >
> > These Generic Initiators are presented by the node access0 class in
> > sysfs in the same way as a CPU.   It seems likely that there will be
> > usecases in which the best 'CPU' is desired and Generic Initiators
> > should be ignored.  The final few patches in this series introduced
> > access1 which is a new performance class in the sysfs node description
> > which presents only CPU to memory relationships.  Test cases for this
> > are described below.  
> 
> 
> Hello Jonathan
> 
> If I want to test this with a fake GI, what are the minimal set of
> changes I should put in my ACPI tables? Can I just specify a dummy GI in
> SRAT? What handle should I use there?

Exactly that for a dummy GI.  Also extend HMAT and SLIT for the extra
proximity domain / initiator.

For the handle, anything is fine.  This patch set doesn't currently use it.
That handle was a bit controversial when this spec feature was being
discussed because it can 'disagree' with information from _PXM.

The ACPI spec ended up effectively relying on them agreeing.  So any handle
must identify a device that either doesn't have a _PXM entry or that
has one that refers to the same proximity domain.

Also note there is a fiddly corner case which is covered by an _OSC.
If you have a device that you want to use _PXM to put in a GI only
domain then older kernels will not know about the GI domain. Hence
ACPI goes through a dance to ensure that a kernel that hasn't
announced it is GI aware, doesn't get told anything is in a GI only domain.
For testing this series though you can just ignore that.

The logic to actually pass that handle based specification through to the
devices is complex, so this set relies on _PXM in DSDT to actually associate
any device with the Generic Initiator domain.  If doing this for a PCI
device, note that you need the fix mentioned in the cover letter to actually
have _PXM apply to PCI EPs.  Note that the _PXM case needs to work anyway
as you might have a GI node with multiple GIs and there is no obligation
for them all to be specified in SRAT.

Once this initial set is in place we can work out how to use the SRAT
handle to associate it with a device.  To be honest, I haven't really
thought about how we'd do that yet.

Thanks,

Jonathan


> 
> Thanks
> 
> Brice
> 
> 

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Brice Goglin]

Le 18/12/2019 à 15:50, Jonathan Cameron a écrit :
> On Wed, 18 Dec 2019 12:32:06 +0100
> Brice Goglin <brice.goglin@gmail.com> wrote:
>
>> Le 16/12/2019 à 16:38, Jonathan Cameron a écrit :
>>> Introduces a new type of NUMA node for cases where we want to represent
>>> the access characteristics of a non CPU initiator of memory requests,
>>> as these differ from all those for existing nodes containing CPUs and/or
>>> memory.
>>>
>>> These Generic Initiators are presented by the node access0 class in
>>> sysfs in the same way as a CPU.   It seems likely that there will be
>>> usecases in which the best 'CPU' is desired and Generic Initiators
>>> should be ignored.  The final few patches in this series introduced
>>> access1 which is a new performance class in the sysfs node description
>>> which presents only CPU to memory relationships.  Test cases for this
>>> are described below.  
>>
>> Hello Jonathan
>>
>> If I want to test this with a fake GI, what are the minimal set of
>> changes I should put in my ACPI tables? Can I just specify a dummy GI in
>> SRAT? What handle should I use there?
> Exactly that for a dummy GI.  Also extend HMAT and SLIT for the extra
> proximity domain / initiator.


I couldn't get this to work (your patches on top of 5.5-rc2). I added
the GI in SRAT, and extended HMAT and SLIT accordingly.

I don't know if that's expected but I get an additional node in sysfs,
with 0kB memory.

However the HMAT table gets ignored because find_mem_target() fails in
hmat_parse_proximity_domain(). The target should have been allocated in
alloc_memory_target() which is called in srat_parse_mem_affinity(), but
it seems to me that this function isn't called for GI nodes. Or should
SRAT also contain a normal Memory node with same PM as the GI?

Brice

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Jonathan Cameron]

On Fri, 20 Dec 2019 22:40:18 +0100
Brice Goglin <brice.goglin@gmail.com> wrote:

> Le 18/12/2019 à 15:50, Jonathan Cameron a écrit :
> > On Wed, 18 Dec 2019 12:32:06 +0100
> > Brice Goglin <brice.goglin@gmail.com> wrote:
> >  
> >> Le 16/12/2019 à 16:38, Jonathan Cameron a écrit :  
> >>> Introduces a new type of NUMA node for cases where we want to represent
> >>> the access characteristics of a non CPU initiator of memory requests,
> >>> as these differ from all those for existing nodes containing CPUs and/or
> >>> memory.
> >>>
> >>> These Generic Initiators are presented by the node access0 class in
> >>> sysfs in the same way as a CPU.   It seems likely that there will be
> >>> usecases in which the best 'CPU' is desired and Generic Initiators
> >>> should be ignored.  The final few patches in this series introduced
> >>> access1 which is a new performance class in the sysfs node description
> >>> which presents only CPU to memory relationships.  Test cases for this
> >>> are described below.    
> >>
> >> Hello Jonathan
> >>
> >> If I want to test this with a fake GI, what are the minimal set of
> >> changes I should put in my ACPI tables? Can I just specify a dummy GI in
> >> SRAT? What handle should I use there?  
> > Exactly that for a dummy GI.  Also extend HMAT and SLIT for the extra
> > proximity domain / initiator.  
> 
> 
> I couldn't get this to work (your patches on top of 5.5-rc2). I added
> the GI in SRAT, and extended HMAT and SLIT accordingly.
> 
> I don't know if that's expected but I get an additional node in sysfs,
> with 0kB memory.
> 
> However the HMAT table gets ignored because find_mem_target() fails in
> hmat_parse_proximity_domain(). The target should have been allocated in
> alloc_memory_target() which is called in srat_parse_mem_affinity(), but
> it seems to me that this function isn't called for GI nodes. Or should
> SRAT also contain a normal Memory node with same PM as the GI?
> 
Hi Brice,

Yes you should see a node with 0kB memory.  Same as you get for a processor
only node I believe.

srat_parse_mem_affinity shouldn't call alloc_memory_target for the GI nodes
as they don't have any memory.   The hmat table should only refer to
GI domains as initiators.  Just to check, do you have them listed as
a target node?  Or perhaps in some hmat proximity entry as memory_PD?

To answer your question, SRAT should not contain a normal memory node
with the same PXM as that would defeat the whole purpose as we would have
been able to have such a domain without Generic Initiators.

Also, just to check, x86 or arm64?

Thanks for testing this.

Jonathan


> Brice
> 
> 

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Brice Goglin]

Le 02/01/2020 à 16:27, Jonathan Cameron a écrit :
>
>> However the HMAT table gets ignored because find_mem_target() fails in
>> hmat_parse_proximity_domain(). The target should have been allocated in
>> alloc_memory_target() which is called in srat_parse_mem_affinity(), but
>> it seems to me that this function isn't called for GI nodes. Or should
>> SRAT also contain a normal Memory node with same PM as the GI?
>>
> Hi Brice,
>
> Yes you should see a node with 0kB memory.  Same as you get for a processor
> only node I believe.
>
> srat_parse_mem_affinity shouldn't call alloc_memory_target for the GI nodes
> as they don't have any memory.   The hmat table should only refer to
> GI domains as initiators.  Just to check, do you have them listed as
> a target node?  Or perhaps in some hmat proximity entry as memory_PD?
>

Thanks, I finally got things to work. I am on x86. It's a dual-socket
machine with SubNUMA clusters (2 nodes per socket) and NVDIMMs (one
dax-kmem node per socket). Before adding a GI, initiators look like this:

node0 -> node0 and node4

node1 -> node1 and node5

node2 -> node2 and node4

node3 -> node3 and node5

I added a GI with faster access to node0, node2, node4 (first socket).

The GI node becomes an access0 initiator for node4, and node0 and node2
remain access1 initiators.

The GI node doesn't become access0 initiator for node0 and node2, likely
because of this test :

        /*
         * If the Address Range Structure provides a local processor pxm, link
         * only that one. Otherwise, find the best performance attributes and
         * register all initiators that match.
         */
        if (target->processor_pxm != PXM_INVAL) {

I guess I should split node0-3 into separate CPU nodes and memory nodes
in SRAT?

Brice

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Jonathan Cameron]

On Thu, 2 Jan 2020 22:37:04 +0100
Brice Goglin <brice.goglin@gmail.com> wrote:

> Le 02/01/2020 à 16:27, Jonathan Cameron a écrit :
> >  
> >> However the HMAT table gets ignored because find_mem_target() fails in
> >> hmat_parse_proximity_domain(). The target should have been allocated in
> >> alloc_memory_target() which is called in srat_parse_mem_affinity(), but
> >> it seems to me that this function isn't called for GI nodes. Or should
> >> SRAT also contain a normal Memory node with same PM as the GI?
> >>  
> > Hi Brice,
> >
> > Yes you should see a node with 0kB memory.  Same as you get for a processor
> > only node I believe.
> >
> > srat_parse_mem_affinity shouldn't call alloc_memory_target for the GI nodes
> > as they don't have any memory.   The hmat table should only refer to
> > GI domains as initiators.  Just to check, do you have them listed as
> > a target node?  Or perhaps in some hmat proximity entry as memory_PD?
> >  
> 
> Thanks, I finally got things to work. I am on x86. It's a dual-socket
> machine with SubNUMA clusters (2 nodes per socket) and NVDIMMs (one
> dax-kmem node per socket). Before adding a GI, initiators look like this:
> 
> node0 -> node0 and node4
> 
> node1 -> node1 and node5
> 
> node2 -> node2 and node4
> 
> node3 -> node3 and node5
> 
> I added a GI with faster access to node0, node2, node4 (first socket).
> 
> The GI node becomes an access0 initiator for node4, and node0 and node2
> remain access1 initiators.
> 
> The GI node doesn't become access0 initiator for node0 and node2, likely
> because of this test :
> 
>         /*
>          * If the Address Range Structure provides a local processor pxm, link
>          * only that one. Otherwise, find the best performance attributes and
>          * register all initiators that match.
>          */
>         if (target->processor_pxm != PXM_INVAL) {
> 
> I guess I should split node0-3 into separate CPU nodes and memory nodes
> in SRAT?

It sounds like it's working as expected.  There are a few assumptions made about
'sensible' hmat configurations.

1) If the memory and processor are in the same domain, that should mean the
access characteristics within that domain are the best in the system.
It is possible to have a setup with very low latency access
from a particular processor but also low bandwidth.  Another domain may have
high bandwidth but long latency.   Such systems may occur, but they are probably
going to not be for 'normal memory the OS can just use'.

2) If we have a relevant "Memory Proximity Domain Attributes Structure"
Note this was renamed in acpi 6.3 from "Address Range Structure" as
it no longer has any address ranges.
(which are entirely optional btw) that indicates that the memory controller
for a given memory lies in the proximity domain of the Initiator specified.
If that happens we ignore cases where hmat says somewhere else is nearer
via bandwidth and latency.

For case 1) I'm not sure we actually enforce it.
I think you've hit case 2).  

Removing the address range structures should work, or as you say you can
move that memory into separate memory nodes.  It will be a bit of a strange
setup though.  Assuming node4 is an NVDIMM then that would be closer to a
potential real system.  With a suitable coherent bus (CCIX is most familiar
to me and can do this) You might have

 _______       ________    _______
|       |     |        |   |       |
| Node0 |     | Node4  |---| Node6 |
| CPU   |-----| Mem +  |---| GI    |
| Mem   |     | CCHome |---|       |
|_______|     |________|   |_______|
   |                          |
   |__________________________|

CCHome Cache Coherency directory location to avoid the need for more
esoteric cache coherency short cut methods etc.

Idea being the GI node is some big fat DB accelerator or similar doing
offloaded queries.  It has a fat pipe to the NVDIMMs.  

Lets ignore that, to actually justify the use of a GI only node,
you need some more elements as this situation could be represented
by fusing node4 and node6 and having asymmetric HMAT between Node0
and the fused Node4.

So in conclusion, with your setup, only the NVDIMM nodes look like the
sort of memory that might be in a node nearer to a GI than the host.
> 
> Brice

Thanks again for looking at this!

Jonathan
> 
> 
> 
> 

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Brice Goglin]

Le 03/01/2020 à 11:09, Jonathan Cameron a écrit :
>
> 1) If the memory and processor are in the same domain, that should mean the
> access characteristics within that domain are the best in the system.
> It is possible to have a setup with very low latency access
> from a particular processor but also low bandwidth.  Another domain may have
> high bandwidth but long latency.   Such systems may occur, but they are probably
> going to not be for 'normal memory the OS can just use'.
>
> 2) If we have a relevant "Memory Proximity Domain Attributes Structure"
> Note this was renamed in acpi 6.3 from "Address Range Structure" as
> it no longer has any address ranges.
> (which are entirely optional btw) that indicates that the memory controller
> for a given memory lies in the proximity domain of the Initiator specified.
> If that happens we ignore cases where hmat says somewhere else is nearer
> via bandwidth and latency.
>
> For case 1) I'm not sure we actually enforce it.
> I think you've hit case 2).  
>
> Removing the address range structures should work, or as you say you can
> move that memory into separate memory nodes.


I removed the "processor proximity domain valid" flag from the address
range structure of node2, and the GI is now its access0 initiator
instead of node2 itself. Looks like it confirms I was in case 2)

Thanks

Brice

Re: [PATCH V6 0/7] ACPI: Support Generic Initiator proximity domains

[Jonathan Cameron]

On Fri, 3 Jan 2020 13:18:59 +0100
Brice Goglin <brice.goglin@gmail.com> wrote:

> Le 03/01/2020 à 11:09, Jonathan Cameron a écrit :
> >
> > 1) If the memory and processor are in the same domain, that should mean the
> > access characteristics within that domain are the best in the system.
> > It is possible to have a setup with very low latency access
> > from a particular processor but also low bandwidth.  Another domain may have
> > high bandwidth but long latency.   Such systems may occur, but they are probably
> > going to not be for 'normal memory the OS can just use'.
> >
> > 2) If we have a relevant "Memory Proximity Domain Attributes Structure"
> > Note this was renamed in acpi 6.3 from "Address Range Structure" as
> > it no longer has any address ranges.
> > (which are entirely optional btw) that indicates that the memory controller
> > for a given memory lies in the proximity domain of the Initiator specified.
> > If that happens we ignore cases where hmat says somewhere else is nearer
> > via bandwidth and latency.
> >
> > For case 1) I'm not sure we actually enforce it.
> > I think you've hit case 2).  
> >
> > Removing the address range structures should work, or as you say you can
> > move that memory into separate memory nodes.  
> 
> 
> I removed the "processor proximity domain valid" flag from the address
> range structure of node2, and the GI is now its access0 initiator
> instead of node2 itself. Looks like it confirms I was in case 2)
> 
> Thanks
> 
> Brice

Cool. I was wondering if that change would work fine.
It is a somewhat crazy setup so I didn't have an equivalent in my test set.

Sounds like all is working as expected.

Thanks,

Jonathan