2019-10-10 13:34:06

by John Garry

[permalink] [raw]
Subject: [RFC PATCH 0/3] ACPI, arm64: Backport for ACPI PPTT 6.3 thread flag for stable 4.19.x

This series is a backport of the ACPI PPTT 6.3 thread flag feature for
supporting arm64 systems.

The background is that some arm64 implementations are broken, in that they
incorrectly advertise that a CPU is mutli-threaded, when it is not - the
HiSilicon Taishanv110 rev 2, aka tsv110, being an example.

This leads to the system topology being incorrect. The reason being that
arm64 topology code uses a combination of ACPI PPTT (Processor Properties
Topology Table) and the system MPIDR (Multiprocessor Affinity Register) MT
bit to determine the topology.

Until ACPI 6.3, the PPTT did not have any method to determine whether
a CPU was multi-threaded, so only the MT bit is used - hence the
broken topology for some systems.

In ACPI 6.3, a PPTT thread flag was introduced, which - when supported -
would be used by the kernel to determine really if a CPU is multi-threaded
or not, so that we don't get incorrect topology.

Note: I'm sending this as an RFC before sending to stable proper. I also
have a 5.2 and 5.3 backport which are almost the same, and only
significant change being that the ACPICA patch is not required.

Erik Schmauss (1):
ACPICA: ACPI 6.3: PPTT add additional fields in Processor Structure
Flags

Jeremy Linton (2):
ACPI/PPTT: Add support for ACPI 6.3 thread flag
arm64: topology: Use PPTT to determine if PE is a thread

arch/arm64/kernel/topology.c | 19 ++++++++++---
drivers/acpi/pptt.c | 52 ++++++++++++++++++++++++++++++++++++
include/acpi/actbl2.h | 7 +++--
include/linux/acpi.h | 5 ++++
4 files changed, 77 insertions(+), 6 deletions(-)

--
2.17.1


2019-10-10 13:34:19

by John Garry

[permalink] [raw]
Subject: [RFC PATCH 3/3] arm64: topology: Use PPTT to determine if PE is a thread

From: Jeremy Linton <[email protected]>

Commit 98dc19902a0b2e5348e43d6a2c39a0a7d0fc639e upstream.

ACPI 6.3 adds a thread flag to represent if a CPU/PE is
actually a thread. Given that the MPIDR_MT bit may not
represent this information consistently on homogeneous machines
we should prefer the PPTT flag if its available.

Signed-off-by: Jeremy Linton <[email protected]>
Reviewed-by: Sudeep Holla <[email protected]>
Reviewed-by: Robert Richter <[email protected]>
[will: made acpi_cpu_is_threaded() return 'bool']
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: John Garry <[email protected]>
---
arch/arm64/kernel/topology.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 0825c4a856e3..6106c49f84bc 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -340,17 +340,28 @@ void remove_cpu_topology(unsigned int cpu)
}

#ifdef CONFIG_ACPI
+static bool __init acpi_cpu_is_threaded(int cpu)
+{
+ int is_threaded = acpi_pptt_cpu_is_thread(cpu);
+
+ /*
+ * if the PPTT doesn't have thread information, assume a homogeneous
+ * machine and return the current CPU's thread state.
+ */
+ if (is_threaded < 0)
+ is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
+
+ return !!is_threaded;
+}
+
/*
* Propagate the topology information of the processor_topology_node tree to the
* cpu_topology array.
*/
static int __init parse_acpi_topology(void)
{
- bool is_threaded;
int cpu, topology_id;

- is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
-
for_each_possible_cpu(cpu) {
int i, cache_id;

@@ -358,7 +369,7 @@ static int __init parse_acpi_topology(void)
if (topology_id < 0)
return topology_id;

- if (is_threaded) {
+ if (acpi_cpu_is_threaded(cpu)) {
cpu_topology[cpu].thread_id = topology_id;
topology_id = find_acpi_cpu_topology(cpu, 1);
cpu_topology[cpu].core_id = topology_id;
--
2.17.1

2019-10-10 13:35:01

by John Garry

[permalink] [raw]
Subject: [RFC PATCH 2/3] ACPI/PPTT: Add support for ACPI 6.3 thread flag

From: Jeremy Linton <[email protected]>

Commit bbd1b70639f785a970d998f35155c713f975e3ac upstream.

ACPI 6.3 adds a flag to the CPU node to indicate whether
the given PE is a thread. Add a function to return that
information for a given linux logical CPU.

Signed-off-by: Jeremy Linton <[email protected]>
Reviewed-by: Sudeep Holla <[email protected]>
Reviewed-by: Robert Richter <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
[jpg: backport for 4.19, replace acpi_pptt_warn_missing()]
Signed-off-by: John Garry <[email protected]>
---
drivers/acpi/pptt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 5 +++++
2 files changed, 57 insertions(+)

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index da031b1df6f5..9dbf86a0c827 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -509,6 +509,44 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
return retval;
}

+/**
+ * check_acpi_cpu_flag() - Determine if CPU node has a flag set
+ * @cpu: Kernel logical CPU number
+ * @rev: The minimum PPTT revision defining the flag
+ * @flag: The flag itself
+ *
+ * Check the node representing a CPU for a given flag.
+ *
+ * Return: -ENOENT if the PPTT doesn't exist, the CPU cannot be found or
+ * the table revision isn't new enough.
+ * 1, any passed flag set
+ * 0, flag unset
+ */
+static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
+{
+ struct acpi_table_header *table;
+ acpi_status status;
+ u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
+ struct acpi_pptt_processor *cpu_node = NULL;
+ int ret = -ENOENT;
+
+ status = acpi_get_table(ACPI_SIG_PPTT, 0, &table);
+ if (ACPI_FAILURE(status)) {
+ pr_warn_once("No PPTT table found, cpu topology may be inaccurate\n");
+ return ret;
+ }
+
+ if (table->revision >= rev)
+ cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
+
+ if (cpu_node)
+ ret = (cpu_node->flags & flag) != 0;
+
+ acpi_put_table(table);
+
+ return ret;
+}
+
/**
* acpi_find_last_cache_level() - Determines the number of cache levels for a PE
* @cpu: Kernel logical cpu number
@@ -573,6 +611,20 @@ int cache_setup_acpi(unsigned int cpu)
return status;
}

+/**
+ * acpi_pptt_cpu_is_thread() - Determine if CPU is a thread
+ * @cpu: Kernel logical CPU number
+ *
+ * Return: 1, a thread
+ * 0, not a thread
+ * -ENOENT ,if the PPTT doesn't exist, the CPU cannot be found or
+ * the table revision isn't new enough.
+ */
+int acpi_pptt_cpu_is_thread(unsigned int cpu)
+{
+ return check_acpi_cpu_flag(cpu, 2, ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD);
+}
+
/**
* find_acpi_cpu_topology() - Determine a unique topology value for a given cpu
* @cpu: Kernel logical cpu number
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index b4d23b3a2ef2..59a416dfcaaa 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1291,10 +1291,15 @@ static inline int lpit_read_residency_count_address(u64 *address)
#endif

#ifdef CONFIG_ACPI_PPTT
+int acpi_pptt_cpu_is_thread(unsigned int cpu);
int find_acpi_cpu_topology(unsigned int cpu, int level);
int find_acpi_cpu_topology_package(unsigned int cpu);
int find_acpi_cpu_cache_topology(unsigned int cpu, int level);
#else
+static inline int acpi_pptt_cpu_is_thread(unsigned int cpu)
+{
+ return -EINVAL;
+}
static inline int find_acpi_cpu_topology(unsigned int cpu, int level)
{
return -EINVAL;
--
2.17.1

2019-10-10 13:37:16

by John Garry

[permalink] [raw]
Subject: [RFC PATCH 1/3] ACPICA: ACPI 6.3: PPTT add additional fields in Processor Structure Flags

From: Erik Schmauss <[email protected]>

Commit b5eab512e7cffb2bb37c4b342b5594e9e75fd486 upstream.

ACPICA commit c736ea34add19a3a07e0e398711847cd6b95affd

Link: https://github.com/acpica/acpica/commit/c736ea34
Signed-off-by: Erik Schmauss <[email protected]>
Signed-off-by: Bob Moore <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: John Garry <[email protected]>
---
include/acpi/actbl2.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index c50ef7e6b942..1d4ef0621174 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -1472,8 +1472,11 @@ struct acpi_pptt_processor {

/* Flags */

-#define ACPI_PPTT_PHYSICAL_PACKAGE (1) /* Physical package */
-#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID (2) /* ACPI Processor ID valid */
+#define ACPI_PPTT_PHYSICAL_PACKAGE (1)
+#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID (1<<1)
+#define ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD (1<<2) /* ACPI 6.3 */
+#define ACPI_PPTT_ACPI_LEAF_NODE (1<<3) /* ACPI 6.3 */
+#define ACPI_PPTT_ACPI_IDENTICAL (1<<4) /* ACPI 6.3 */

/* 1: Cache Type Structure */

--
2.17.1

2019-10-10 14:25:54

by Moore, Robert

[permalink] [raw]
Subject: RE: [RFC PATCH 1/3] ACPICA: ACPI 6.3: PPTT add additional fields in Processor Structure Flags

John,
These #defines are all already in actbl2.h. Perhaps they didn't make it into Linux.
Bob


-----Original Message-----
From: John Garry <[email protected]>
Sent: Thursday, October 10, 2019 6:30 AM
To: [email protected]; [email protected]; [email protected]; [email protected]; Moore, Robert <[email protected]>; Schmauss, Erik <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Wysocki, Rafael J <[email protected]>; John Garry <[email protected]>
Subject: [RFC PATCH 1/3] ACPICA: ACPI 6.3: PPTT add additional fields in Processor Structure Flags

From: Erik Schmauss <[email protected]>

Commit b5eab512e7cffb2bb37c4b342b5594e9e75fd486 upstream.

ACPICA commit c736ea34add19a3a07e0e398711847cd6b95affd

Link: https://github.com/acpica/acpica/commit/c736ea34
Signed-off-by: Erik Schmauss <[email protected]>
Signed-off-by: Bob Moore <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: John Garry <[email protected]>
---
include/acpi/actbl2.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index c50ef7e6b942..1d4ef0621174 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -1472,8 +1472,11 @@ struct acpi_pptt_processor {

/* Flags */

-#define ACPI_PPTT_PHYSICAL_PACKAGE (1) /* Physical package */
-#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID (2) /* ACPI Processor ID valid */
+#define ACPI_PPTT_PHYSICAL_PACKAGE (1)
+#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID (1<<1)
+#define ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD (1<<2) /* ACPI 6.3 */
+#define ACPI_PPTT_ACPI_LEAF_NODE (1<<3) /* ACPI 6.3 */
+#define ACPI_PPTT_ACPI_IDENTICAL (1<<4) /* ACPI 6.3 */

/* 1: Cache Type Structure */

--
2.17.1

2019-10-10 14:31:32

by John Garry

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] ACPICA: ACPI 6.3: PPTT add additional fields in Processor Structure Flags

On 10/10/2019 15:22, Moore, Robert wrote:
> John,
> These #defines are all already in actbl2.h. Perhaps they didn't make it into Linux.
> Bob

Hi Bob,

Yes, they are in the latest linux mainline release.

But this patch is just a preview to backport them to an earlier kernel
version.

Thanks,
John

>
>
> -----Original Message-----
> From: John Garry <[email protected]>
> Sent: Thursday, October 10, 2019 6:30 AM
> To: [email protected]; [email protected]; [email protected]; [email protected]; Moore, Robert <[email protected]>; Schmauss, Erik <[email protected]>; [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Wysocki, Rafael J <[email protected]>; John Garry <[email protected]>
> Subject: [RFC PATCH 1/3] ACPICA: ACPI 6.3: PPTT add additional fields in Processor Structure Flags
>
> From: Erik Schmauss <[email protected]>
>
> Commit b5eab512e7cffb2bb37c4b342b5594e9e75fd486 upstream.
>
> ACPICA commit c736ea34add19a3a07e0e398711847cd6b95affd
>
> Link: https://github.com/acpica/acpica/commit/c736ea34
> Signed-off-by: Erik Schmauss <[email protected]>
> Signed-off-by: Bob Moore <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> Signed-off-by: John Garry <[email protected]>
> ---
> include/acpi/actbl2.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index c50ef7e6b942..1d4ef0621174 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -1472,8 +1472,11 @@ struct acpi_pptt_processor {
>
> /* Flags */
>
> -#define ACPI_PPTT_PHYSICAL_PACKAGE (1) /* Physical package */
> -#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID (2) /* ACPI Processor ID valid */
> +#define ACPI_PPTT_PHYSICAL_PACKAGE (1)
> +#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID (1<<1)
> +#define ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD (1<<2) /* ACPI 6.3 */
> +#define ACPI_PPTT_ACPI_LEAF_NODE (1<<3) /* ACPI 6.3 */
> +#define ACPI_PPTT_ACPI_IDENTICAL (1<<4) /* ACPI 6.3 */
>
> /* 1: Cache Type Structure */
>
> --
> 2.17.1
>
>
> .
>


2019-10-11 00:54:46

by Hanjun Guo

[permalink] [raw]
Subject: Re: [RFC PATCH 0/3] ACPI, arm64: Backport for ACPI PPTT 6.3 thread flag for stable 4.19.x

Hi John,

On 2019/10/10 21:29, John Garry wrote:
> This series is a backport of the ACPI PPTT 6.3 thread flag feature for
> supporting arm64 systems.
>
> The background is that some arm64 implementations are broken, in that they
> incorrectly advertise that a CPU is mutli-threaded, when it is not - the
> HiSilicon Taishanv110 rev 2, aka tsv110, being an example.
>
> This leads to the system topology being incorrect. The reason being that
> arm64 topology code uses a combination of ACPI PPTT (Processor Properties
> Topology Table) and the system MPIDR (Multiprocessor Affinity Register) MT
> bit to determine the topology.
>
> Until ACPI 6.3, the PPTT did not have any method to determine whether
> a CPU was multi-threaded, so only the MT bit is used - hence the
> broken topology for some systems.
>
> In ACPI 6.3, a PPTT thread flag was introduced, which - when supported -
> would be used by the kernel to determine really if a CPU is multi-threaded
> or not, so that we don't get incorrect topology.

Thanks for doing this, and this patch set fix my CPU topology issue
in 4.19 kernel with updated firmware.

>
> Note: I'm sending this as an RFC before sending to stable proper. I also
> have a 5.2 and 5.3 backport which are almost the same, and only
> significant change being that the ACPICA patch is not required.

5.2 stable was end of life, so only 4.19 and 5.3 are needed I think.

Thanks
Hanjun