Re: [PATCHv8 07/10] acpi/hmat: Register processor domain to its memory
On Fri, Mar 29, 2019 at 02:15:03PM -0700, Dan Williams wrote:
> On Mon, Mar 11, 2019 at 1:55 PM Keith Busch wrote:
> > +static __init struct memory_target *find_mem_target(unsigned int mem_pxm)
> > +{
> > + struct memory_target *target;
> > +
> > + list_for_each_entry(target, , node)
> > + if (target->memory_pxm == mem_pxm)
> > + return target;
> > + return NULL;
>
> The above implementation assumes that every SRAT entry has a unique
> @mem_pxm. I don't think that's valid if the memory map is sparse,
> right?
Oh, we don't really care if multiple entries report the same PXM. We do
assume there may be multiple entires with the same PXM and have tested
this, but we're just allocating one memory target per unique memory
PXM and consider multiple entires comprise the same memory target. That
is okay if since we only need to identify unique PXMs and have no use
for the adderss ranges that make up that target, which is the case
for this series. I see you have a future use that has address ranges
considerations, so separate targets for sparse ranges can definitely
be added.
Re: [PATCHv8 07/10] acpi/hmat: Register processor domain to its memory
On Mon, Mar 11, 2019 at 1:55 PM Keith Busch wrote:
>
> If the HMAT Subsystem Address Range provides a valid processor proximity
> domain for a memory domain, or a processor domain matches the performance
> access of the valid processor proximity domain, register the memory
> target with that initiator so this relationship will be visible under
> the node's sysfs directory.
>
> Since HMAT requires valid address ranges have an equivalent SRAT entry,
> verify each memory target satisfies this requirement.
>
> Reviewed-by: Jonathan Cameron
> Signed-off-by: Keith Busch
> ---
> drivers/acpi/hmat/Kconfig | 3 +-
> drivers/acpi/hmat/hmat.c | 392
> +-
> 2 files changed, 393 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> index 2f7111b7af62..13cddd612a52 100644
> --- a/drivers/acpi/hmat/Kconfig
> +++ b/drivers/acpi/hmat/Kconfig
> @@ -4,4 +4,5 @@ config ACPI_HMAT
> depends on ACPI_NUMA
> help
> If set, this option has the kernel parse and report the
> -platform's ACPI HMAT (Heterogeneous Memory Attributes Table).
> +platform's ACPI HMAT (Heterogeneous Memory Attributes Table),
> +and register memory initiators with their targets.
> diff --git a/drivers/acpi/hmat/hmat.c b/drivers/acpi/hmat/hmat.c
> index 4758beb3b2c1..01a6eddac6f7 100644
> --- a/drivers/acpi/hmat/hmat.c
> +++ b/drivers/acpi/hmat/hmat.c
> @@ -13,11 +13,105 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> static __initdata u8 hmat_revision;
>
> +static __initdata LIST_HEAD(targets);
> +static __initdata LIST_HEAD(initiators);
> +static __initdata LIST_HEAD(localities);
> +
> +/*
> + * The defined enum order is used to prioritize attributes to break ties when
> + * selecting the best performing node.
> + */
> +enum locality_types {
> + WRITE_LATENCY,
> + READ_LATENCY,
> + WRITE_BANDWIDTH,
> + READ_BANDWIDTH,
> +};
> +
> +static struct memory_locality *localities_types[4];
> +
> +struct memory_target {
> + struct list_head node;
> + unsigned int memory_pxm;
> + unsigned int processor_pxm;
> + struct node_hmem_attrs hmem_attrs;
> +};
> +
> +struct memory_initiator {
> + struct list_head node;
> + unsigned int processor_pxm;
> +};
> +
> +struct memory_locality {
> + struct list_head node;
> + struct acpi_hmat_locality *hmat_loc;
> +};
> +
> +static __init struct memory_initiator *find_mem_initiator(unsigned int
> cpu_pxm)
> +{
> + struct memory_initiator *initiator;
> +
> + list_for_each_entry(initiator, , node)
> + if (initiator->processor_pxm == cpu_pxm)
> + return initiator;
> + return NULL;
> +}
> +
> +static __init struct memory_target *find_mem_target(unsigned int mem_pxm)
> +{
> + struct memory_target *target;
> +
> + list_for_each_entry(target, , node)
> + if (target->memory_pxm == mem_pxm)
> + return target;
> + return NULL;
The above implementation assumes that every SRAT entry has a unique
@mem_pxm. I don't think that's valid if the memory map is sparse,
right?
Re: [PATCHv8 07/10] acpi/hmat: Register processor domain to its memory
On Mon, Mar 11, 2019 at 9:55 PM Keith Busch wrote:
>
> If the HMAT Subsystem Address Range provides a valid processor proximity
> domain for a memory domain, or a processor domain matches the performance
> access of the valid processor proximity domain, register the memory
> target with that initiator so this relationship will be visible under
> the node's sysfs directory.
>
> Since HMAT requires valid address ranges have an equivalent SRAT entry,
> verify each memory target satisfies this requirement.
>
> Reviewed-by: Jonathan Cameron
> Signed-off-by: Keith Busch
Acked-by: Rafael J. Wysocki
> ---
> drivers/acpi/hmat/Kconfig | 3 +-
> drivers/acpi/hmat/hmat.c | 392
> +-
> 2 files changed, 393 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> index 2f7111b7af62..13cddd612a52 100644
> --- a/drivers/acpi/hmat/Kconfig
> +++ b/drivers/acpi/hmat/Kconfig
> @@ -4,4 +4,5 @@ config ACPI_HMAT
> depends on ACPI_NUMA
> help
> If set, this option has the kernel parse and report the
> -platform's ACPI HMAT (Heterogeneous Memory Attributes Table).
> +platform's ACPI HMAT (Heterogeneous Memory Attributes Table),
> +and register memory initiators with their targets.
> diff --git a/drivers/acpi/hmat/hmat.c b/drivers/acpi/hmat/hmat.c
> index 4758beb3b2c1..01a6eddac6f7 100644
> --- a/drivers/acpi/hmat/hmat.c
> +++ b/drivers/acpi/hmat/hmat.c
> @@ -13,11 +13,105 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> static __initdata u8 hmat_revision;
>
> +static __initdata LIST_HEAD(targets);
> +static __initdata LIST_HEAD(initiators);
> +static __initdata LIST_HEAD(localities);
> +
> +/*
> + * The defined enum order is used to prioritize attributes to break ties when
> + * selecting the best performing node.
> + */
> +enum locality_types {
> + WRITE_LATENCY,
> + READ_LATENCY,
> + WRITE_BANDWIDTH,
> + READ_BANDWIDTH,
> +};
> +
> +static struct memory_locality *localities_types[4];
> +
> +struct memory_target {
> + struct list_head node;
> + unsigned int memory_pxm;
> + unsigned int processor_pxm;
> + struct node_hmem_attrs hmem_attrs;
> +};
> +
> +struct memory_initiator {
> + struct list_head node;
> + unsigned int processor_pxm;
> +};
> +
> +struct memory_locality {
> + struct list_head node;
> + struct acpi_hmat_locality *hmat_loc;
> +};
> +
> +static __init struct memory_initiator *find_mem_initiator(unsigned int
> cpu_pxm)
> +{
> + struct memory_initiator *initiator;
> +
> + list_for_each_entry(initiator, , node)
> + if (initiator->processor_pxm == cpu_pxm)
> + return initiator;
> + return NULL;
> +}
> +
> +static __init struct memory_target *find_mem_target(unsigned int mem_pxm)
> +{
> + struct memory_target *target;
> +
> + list_for_each_entry(target, , node)
> + if (target->memory_pxm == mem_pxm)
> + return target;
> + return NULL;
> +}
> +
> +static __init void alloc_memory_initiator(unsigned int cpu_pxm)
> +{
> + struct memory_initiator *initiator;
> +
> + if (pxm_to_node(cpu_pxm) == NUMA_NO_NODE)
> + return;
> +
> + initiator = find_mem_initiator(cpu_pxm);
> + if (initiator)
> + return;
> +
> + initiator = kzalloc(sizeof(*initiator), GFP_KERNEL);
> + if (!initiator)
> + return;
> +
> + initiator->processor_pxm = cpu_pxm;
> + list_add_tail(>node, );
> +}
> +
> +static __init void alloc_memory_target(unsigned int mem_pxm)
> +{
> + struct memory_target *target;
> +
> + if (pxm_to_node(mem_pxm) == NUMA_NO_NODE)
> + return;
> +
> + target = find_mem_target(mem_pxm);
> + if (target)
> + return;
> +
> + target = kzalloc(sizeof(*target), GFP_KERNEL);
> + if (!target)
> + return;
> +
> + target->memory_pxm = mem_pxm;
> + target->processor_pxm = PXM_INVAL;
> + list_add_tail(>node, );
> +}
> +
> static __init const char *hmat_data_type(u8 type)
> {
> switch (type) {
> @@ -89,14 +183,83 @@ static __init u32 hmat_normalize(u16 entry, u64 base, u8
> type)
> return value;
> }
>
> +static __init void hmat_update_target_access(struct memory_target *target,
> +u8 type, u32 value)
> +{
> + switch (type) {
> + case ACPI_HMAT_ACCESS_LATENCY:
> + target->hmem_attrs.read_latency = value;
> + target->hmem_attrs.write_latency = value;
> + break;
> + case ACPI_HMAT_READ_LATENCY:
> + target->hmem_attrs.read_latency = value;
> + break;
> + case ACPI_HMAT_WRITE_LATENCY:
> + target->hmem_attrs.write_latency = value;
> +
[PATCHv8 07/10] acpi/hmat: Register processor domain to its memory
If the HMAT Subsystem Address Range provides a valid processor proximity
domain for a memory domain, or a processor domain matches the performance
access of the valid processor proximity domain, register the memory
target with that initiator so this relationship will be visible under
the node's sysfs directory.
Since HMAT requires valid address ranges have an equivalent SRAT entry,
verify each memory target satisfies this requirement.
Reviewed-by: Jonathan Cameron
Signed-off-by: Keith Busch
---
drivers/acpi/hmat/Kconfig | 3 +-
drivers/acpi/hmat/hmat.c | 392 +-
2 files changed, 393 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
index 2f7111b7af62..13cddd612a52 100644
--- a/drivers/acpi/hmat/Kconfig
+++ b/drivers/acpi/hmat/Kconfig
@@ -4,4 +4,5 @@ config ACPI_HMAT
depends on ACPI_NUMA
help
If set, this option has the kernel parse and report the
-platform's ACPI HMAT (Heterogeneous Memory Attributes Table).
+platform's ACPI HMAT (Heterogeneous Memory Attributes Table),
+and register memory initiators with their targets.
diff --git a/drivers/acpi/hmat/hmat.c b/drivers/acpi/hmat/hmat.c
index 4758beb3b2c1..01a6eddac6f7 100644
--- a/drivers/acpi/hmat/hmat.c
+++ b/drivers/acpi/hmat/hmat.c
@@ -13,11 +13,105 @@
#include
#include
#include
+#include
#include
#include
static __initdata u8 hmat_revision;
+static __initdata LIST_HEAD(targets);
+static __initdata LIST_HEAD(initiators);
+static __initdata LIST_HEAD(localities);
+
+/*
+ * The defined enum order is used to prioritize attributes to break ties when
+ * selecting the best performing node.
+ */
+enum locality_types {
+ WRITE_LATENCY,
+ READ_LATENCY,
+ WRITE_BANDWIDTH,
+ READ_BANDWIDTH,
+};
+
+static struct memory_locality *localities_types[4];
+
+struct memory_target {
+ struct list_head node;
+ unsigned int memory_pxm;
+ unsigned int processor_pxm;
+ struct node_hmem_attrs hmem_attrs;
+};
+
+struct memory_initiator {
+ struct list_head node;
+ unsigned int processor_pxm;
+};
+
+struct memory_locality {
+ struct list_head node;
+ struct acpi_hmat_locality *hmat_loc;
+};
+
+static __init struct memory_initiator *find_mem_initiator(unsigned int cpu_pxm)
+{
+ struct memory_initiator *initiator;
+
+ list_for_each_entry(initiator, , node)
+ if (initiator->processor_pxm == cpu_pxm)
+ return initiator;
+ return NULL;
+}
+
+static __init struct memory_target *find_mem_target(unsigned int mem_pxm)
+{
+ struct memory_target *target;
+
+ list_for_each_entry(target, , node)
+ if (target->memory_pxm == mem_pxm)
+ return target;
+ return NULL;
+}
+
+static __init void alloc_memory_initiator(unsigned int cpu_pxm)
+{
+ struct memory_initiator *initiator;
+
+ if (pxm_to_node(cpu_pxm) == NUMA_NO_NODE)
+ return;
+
+ initiator = find_mem_initiator(cpu_pxm);
+ if (initiator)
+ return;
+
+ initiator = kzalloc(sizeof(*initiator), GFP_KERNEL);
+ if (!initiator)
+ return;
+
+ initiator->processor_pxm = cpu_pxm;
+ list_add_tail(>node, );
+}
+
+static __init void alloc_memory_target(unsigned int mem_pxm)
+{
+ struct memory_target *target;
+
+ if (pxm_to_node(mem_pxm) == NUMA_NO_NODE)
+ return;
+
+ target = find_mem_target(mem_pxm);
+ if (target)
+ return;
+
+ target = kzalloc(sizeof(*target), GFP_KERNEL);
+ if (!target)
+ return;
+
+ target->memory_pxm = mem_pxm;
+ target->processor_pxm = PXM_INVAL;
+ list_add_tail(>node, );
+}
+
static __init const char *hmat_data_type(u8 type)
{
switch (type) {
@@ -89,14 +183,83 @@ static __init u32 hmat_normalize(u16 entry, u64 base, u8
type)
return value;
}
+static __init void hmat_update_target_access(struct memory_target *target,
+u8 type, u32 value)
+{
+ switch (type) {
+ case ACPI_HMAT_ACCESS_LATENCY:
+ target->hmem_attrs.read_latency = value;
+ target->hmem_attrs.write_latency = value;
+ break;
+ case ACPI_HMAT_READ_LATENCY:
+ target->hmem_attrs.read_latency = value;
+ break;
+ case ACPI_HMAT_WRITE_LATENCY:
+ target->hmem_attrs.write_latency = value;
+ break;
+ case ACPI_HMAT_ACCESS_BANDWIDTH:
+ target->hmem_attrs.read_bandwidth = value;
+ target->hmem_attrs.write_bandwidth = value;
+ break;
+ case ACPI_HMAT_READ_BANDWIDTH:
+ target->hmem_attrs.read_bandwidth = value;
+ break;
+ case ACPI_HMAT_WRITE_BANDWIDTH:
+
