[PATCH v3 0/1] Add support for parse_acpi_topology() on RISC-V

[PATCH v3 0/1] Add support for parse_acpi_topology() on RISC-V

[Yunhui Cui]

v1->v2:
Place the struct cpu_smt_info{} in drivers/base/arch_topology.c instead of arch_topology.h

v2->v3:
Modify the return values of acpi_pptt_cpu_is_thread()

Yunhui Cui (1):
  arch_topology: move parse_acpi_topology() to common code

 arch/arm64/kernel/topology.c  | 87 +---------------------------------
 drivers/base/arch_topology.c  | 89 ++++++++++++++++++++++++++++++++++-
 include/linux/arch_topology.h |  1 +
 3 files changed, 90 insertions(+), 87 deletions(-)

-- 
2.39.5

[PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[Yunhui Cui]

Currently, RISC-V lacks arch-specific registers for CPU topology
properties and must get them from ACPI. Thus, parse_acpi_topology()
is moved from arm64/ to drivers/ for RISC-V reuse.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 arch/arm64/kernel/topology.c  | 87 +---------------------------------
 drivers/base/arch_topology.c  | 89 ++++++++++++++++++++++++++++++++++-
 include/linux/arch_topology.h |  1 +
 3 files changed, 90 insertions(+), 87 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 5d07ee85bdae4..55650db53b526 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -26,7 +26,7 @@
 #include <asm/topology.h>
 
 #ifdef CONFIG_ACPI
-static bool __init acpi_cpu_is_threaded(int cpu)
+bool __init acpi_cpu_is_threaded(int cpu)
 {
 	int is_threaded = acpi_pptt_cpu_is_thread(cpu);
 
@@ -39,91 +39,6 @@ static bool __init acpi_cpu_is_threaded(int cpu)
 
 	return !!is_threaded;
 }
-
-struct cpu_smt_info {
-	unsigned int thread_num;
-	int core_id;
-};
-
-/*
- * Propagate the topology information of the processor_topology_node tree to the
- * cpu_topology array.
- */
-int __init parse_acpi_topology(void)
-{
-	unsigned int max_smt_thread_num = 1;
-	struct cpu_smt_info *entry;
-	struct xarray hetero_cpu;
-	unsigned long hetero_id;
-	int cpu, topology_id;
-
-	if (acpi_disabled)
-		return 0;
-
-	xa_init(&hetero_cpu);
-
-	for_each_possible_cpu(cpu) {
-		topology_id = find_acpi_cpu_topology(cpu, 0);
-		if (topology_id < 0)
-			return topology_id;
-
-		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;
-
-			/*
-			 * In the PPTT, CPUs below a node with the 'identical
-			 * implementation' flag have the same number of threads.
-			 * Count the number of threads for only one CPU (i.e.
-			 * one core_id) among those with the same hetero_id.
-			 * See the comment of find_acpi_cpu_topology_hetero_id()
-			 * for more details.
-			 *
-			 * One entry is created for each node having:
-			 * - the 'identical implementation' flag
-			 * - its parent not having the flag
-			 */
-			hetero_id = find_acpi_cpu_topology_hetero_id(cpu);
-			entry = xa_load(&hetero_cpu, hetero_id);
-			if (!entry) {
-				entry = kzalloc(sizeof(*entry), GFP_KERNEL);
-				WARN_ON_ONCE(!entry);
-
-				if (entry) {
-					entry->core_id = topology_id;
-					entry->thread_num = 1;
-					xa_store(&hetero_cpu, hetero_id,
-						 entry, GFP_KERNEL);
-				}
-			} else if (entry->core_id == topology_id) {
-				entry->thread_num++;
-			}
-		} else {
-			cpu_topology[cpu].thread_id  = -1;
-			cpu_topology[cpu].core_id    = topology_id;
-		}
-		topology_id = find_acpi_cpu_topology_cluster(cpu);
-		cpu_topology[cpu].cluster_id = topology_id;
-		topology_id = find_acpi_cpu_topology_package(cpu);
-		cpu_topology[cpu].package_id = topology_id;
-	}
-
-	/*
-	 * This is a short loop since the number of XArray elements is the
-	 * number of heterogeneous CPU clusters. On a homogeneous system
-	 * there's only one entry in the XArray.
-	 */
-	xa_for_each(&hetero_cpu, hetero_id, entry) {
-		max_smt_thread_num = max(max_smt_thread_num, entry->thread_num);
-		xa_erase(&hetero_cpu, hetero_id);
-		kfree(entry);
-	}
-
-	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
-	xa_destroy(&hetero_cpu);
-	return 0;
-}
 #endif
 
 #ifdef CONFIG_ARM64_AMU_EXTN
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 1037169abb459..09f77fd549490 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -823,12 +823,99 @@ void remove_cpu_topology(unsigned int cpu)
 	clear_cpu_topology(cpu);
 }
 
+__weak bool __init acpi_cpu_is_threaded(int cpu)
+{
+	int is_threaded = acpi_pptt_cpu_is_thread(cpu);
+
+	return is_threaded == 1;
+}
+
+#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
+struct cpu_smt_info {
+	unsigned int thread_num;
+	int core_id;
+};
+
+/*
+ * Propagate the topology information of the processor_topology_node tree to the
+ * cpu_topology array.
+ */
 __weak int __init parse_acpi_topology(void)
 {
+	unsigned int max_smt_thread_num = 1;
+	struct cpu_smt_info *entry;
+	struct xarray hetero_cpu;
+	unsigned long hetero_id;
+	int cpu, topology_id;
+
+	if (acpi_disabled)
+		return 0;
+
+	xa_init(&hetero_cpu);
+
+	for_each_possible_cpu(cpu) {
+		topology_id = find_acpi_cpu_topology(cpu, 0);
+		if (topology_id < 0)
+			return topology_id;
+
+		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;
+
+			/*
+			 * In the PPTT, CPUs below a node with the 'identical
+			 * implementation' flag have the same number of threads.
+			 * Count the number of threads for only one CPU (i.e.
+			 * one core_id) among those with the same hetero_id.
+			 * See the comment of find_acpi_cpu_topology_hetero_id()
+			 * for more details.
+			 *
+			 * One entry is created for each node having:
+			 * - the 'identical implementation' flag
+			 * - its parent not having the flag
+			 */
+			hetero_id = find_acpi_cpu_topology_hetero_id(cpu);
+			entry = xa_load(&hetero_cpu, hetero_id);
+			if (!entry) {
+				entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+				WARN_ON_ONCE(!entry);
+
+				if (entry) {
+					entry->core_id = topology_id;
+					entry->thread_num = 1;
+					xa_store(&hetero_cpu, hetero_id,
+						 entry, GFP_KERNEL);
+				}
+			} else if (entry->core_id == topology_id) {
+				entry->thread_num++;
+			}
+		} else {
+			cpu_topology[cpu].thread_id  = -1;
+			cpu_topology[cpu].core_id    = topology_id;
+		}
+		topology_id = find_acpi_cpu_topology_cluster(cpu);
+		cpu_topology[cpu].cluster_id = topology_id;
+		topology_id = find_acpi_cpu_topology_package(cpu);
+		cpu_topology[cpu].package_id = topology_id;
+	}
+
+	/*
+	 * This is a short loop since the number of XArray elements is the
+	 * number of heterogeneous CPU clusters. On a homogeneous system
+	 * there's only one entry in the XArray.
+	 */
+	xa_for_each(&hetero_cpu, hetero_id, entry) {
+		max_smt_thread_num = max(max_smt_thread_num, entry->thread_num);
+		xa_erase(&hetero_cpu, hetero_id);
+		kfree(entry);
+	}
+
+	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
+	xa_destroy(&hetero_cpu);
 	return 0;
 }
 
-#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
 void __init init_cpu_topology(void)
 {
 	int cpu, ret;
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index d72d6e5aa2002..8cd8a9604f33f 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -88,6 +88,7 @@ void update_siblings_masks(unsigned int cpu);
 void remove_cpu_topology(unsigned int cpuid);
 void reset_cpu_topology(void);
 int parse_acpi_topology(void);
+bool acpi_cpu_is_threaded(int cpu);
 void freq_inv_set_max_ratio(int cpu, u64 max_rate);
 #endif
 
-- 
2.39.5

Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[Will Deacon]

On Fri, Sep 19, 2025 at 04:59:18PM +0800, Yunhui Cui wrote:
> Currently, RISC-V lacks arch-specific registers for CPU topology
> properties and must get them from ACPI. Thus, parse_acpi_topology()
> is moved from arm64/ to drivers/ for RISC-V reuse.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  arch/arm64/kernel/topology.c  | 87 +---------------------------------
>  drivers/base/arch_topology.c  | 89 ++++++++++++++++++++++++++++++++++-
>  include/linux/arch_topology.h |  1 +
>  3 files changed, 90 insertions(+), 87 deletions(-)

I'm fine with shedding code from arch/arm64/ but this needs an Ack
from Sudeep.

Will

Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[Sudeep Holla]

On Fri, Sep 19, 2025 at 04:59:18PM +0800, Yunhui Cui wrote:
> Currently, RISC-V lacks arch-specific registers for CPU topology
> properties and must get them from ACPI. Thus, parse_acpi_topology()
> is moved from arm64/ to drivers/ for RISC-V reuse.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  arch/arm64/kernel/topology.c  | 87 +---------------------------------
>  drivers/base/arch_topology.c  | 89 ++++++++++++++++++++++++++++++++++-
>  include/linux/arch_topology.h |  1 +
>  3 files changed, 90 insertions(+), 87 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 5d07ee85bdae4..55650db53b526 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -26,7 +26,7 @@
>  #include <asm/topology.h>
>  
>  #ifdef CONFIG_ACPI
> -static bool __init acpi_cpu_is_threaded(int cpu)
> +bool __init acpi_cpu_is_threaded(int cpu)
>  {
>  	int is_threaded = acpi_pptt_cpu_is_thread(cpu);
>  
> @@ -39,91 +39,6 @@ static bool __init acpi_cpu_is_threaded(int cpu)
>  
>  	return !!is_threaded;
>  }
> -
> -struct cpu_smt_info {
> -	unsigned int thread_num;
> -	int core_id;
> -};
> -
> -/*
> - * Propagate the topology information of the processor_topology_node tree to the
> - * cpu_topology array.
> - */
> -int __init parse_acpi_topology(void)
> -{
> -	unsigned int max_smt_thread_num = 1;
> -	struct cpu_smt_info *entry;
> -	struct xarray hetero_cpu;
> -	unsigned long hetero_id;
> -	int cpu, topology_id;
> -
> -	if (acpi_disabled)
> -		return 0;
> -
> -	xa_init(&hetero_cpu);
> -
> -	for_each_possible_cpu(cpu) {
> -		topology_id = find_acpi_cpu_topology(cpu, 0);
> -		if (topology_id < 0)
> -			return topology_id;
> -
> -		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;
> -
> -			/*
> -			 * In the PPTT, CPUs below a node with the 'identical
> -			 * implementation' flag have the same number of threads.
> -			 * Count the number of threads for only one CPU (i.e.
> -			 * one core_id) among those with the same hetero_id.
> -			 * See the comment of find_acpi_cpu_topology_hetero_id()
> -			 * for more details.
> -			 *
> -			 * One entry is created for each node having:
> -			 * - the 'identical implementation' flag
> -			 * - its parent not having the flag
> -			 */
> -			hetero_id = find_acpi_cpu_topology_hetero_id(cpu);
> -			entry = xa_load(&hetero_cpu, hetero_id);
> -			if (!entry) {
> -				entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> -				WARN_ON_ONCE(!entry);
> -
> -				if (entry) {
> -					entry->core_id = topology_id;
> -					entry->thread_num = 1;
> -					xa_store(&hetero_cpu, hetero_id,
> -						 entry, GFP_KERNEL);
> -				}
> -			} else if (entry->core_id == topology_id) {
> -				entry->thread_num++;
> -			}
> -		} else {
> -			cpu_topology[cpu].thread_id  = -1;
> -			cpu_topology[cpu].core_id    = topology_id;
> -		}
> -		topology_id = find_acpi_cpu_topology_cluster(cpu);
> -		cpu_topology[cpu].cluster_id = topology_id;
> -		topology_id = find_acpi_cpu_topology_package(cpu);
> -		cpu_topology[cpu].package_id = topology_id;
> -	}
> -
> -	/*
> -	 * This is a short loop since the number of XArray elements is the
> -	 * number of heterogeneous CPU clusters. On a homogeneous system
> -	 * there's only one entry in the XArray.
> -	 */
> -	xa_for_each(&hetero_cpu, hetero_id, entry) {
> -		max_smt_thread_num = max(max_smt_thread_num, entry->thread_num);
> -		xa_erase(&hetero_cpu, hetero_id);
> -		kfree(entry);
> -	}
> -
> -	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
> -	xa_destroy(&hetero_cpu);
> -	return 0;
> -}
>  #endif
>  
>  #ifdef CONFIG_ARM64_AMU_EXTN
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 1037169abb459..09f77fd549490 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -823,12 +823,99 @@ void remove_cpu_topology(unsigned int cpu)
>  	clear_cpu_topology(cpu);
>  }
>  
> +__weak bool __init acpi_cpu_is_threaded(int cpu)
> +{
> +	int is_threaded = acpi_pptt_cpu_is_thread(cpu);
> +

Just thinking if it makes sense keep acpi_cpu_is_threaded generic without
the need for weak definition.

Additional note: not sure why you haven't moved this under CONFIG_ARM64/RISCV as
done with other code.

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, check for architecture
	 * specific fallback if available
	 */
        if (is_threaded < 0)
                is_threaded = arch_cpu_is_threaded();

        return !!is_threaded;
}

Then you can just have the define in

#define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)

in arch/arm64/include/asm/topology.h

and

+#ifndef arch_cpu_is_threaded
+#define arch_cpu_is_threaded           (0)
+#endif

in include/linux/arch_topology.h

Thoughts ?

-- 
Regards,
Sudeep

Re: [External] Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[yunhui cui]

Hi Sudeep,

On Fri, Sep 19, 2025 at 10:05 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Fri, Sep 19, 2025 at 04:59:18PM +0800, Yunhui Cui wrote:
> > Currently, RISC-V lacks arch-specific registers for CPU topology
> > properties and must get them from ACPI. Thus, parse_acpi_topology()
> > is moved from arm64/ to drivers/ for RISC-V reuse.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >  arch/arm64/kernel/topology.c  | 87 +---------------------------------
> >  drivers/base/arch_topology.c  | 89 ++++++++++++++++++++++++++++++++++-
> >  include/linux/arch_topology.h |  1 +
> >  3 files changed, 90 insertions(+), 87 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> > index 5d07ee85bdae4..55650db53b526 100644
> > --- a/arch/arm64/kernel/topology.c
> > +++ b/arch/arm64/kernel/topology.c
> > @@ -26,7 +26,7 @@
> >  #include <asm/topology.h>
> >
> >  #ifdef CONFIG_ACPI
> > -static bool __init acpi_cpu_is_threaded(int cpu)
> > +bool __init acpi_cpu_is_threaded(int cpu)
> >  {
> >       int is_threaded = acpi_pptt_cpu_is_thread(cpu);
> >
> > @@ -39,91 +39,6 @@ static bool __init acpi_cpu_is_threaded(int cpu)
> >
> >       return !!is_threaded;
> >  }
> > -
> > -struct cpu_smt_info {
> > -     unsigned int thread_num;
> > -     int core_id;
> > -};
> > -
> > -/*
> > - * Propagate the topology information of the processor_topology_node tree to the
> > - * cpu_topology array.
> > - */
> > -int __init parse_acpi_topology(void)
> > -{
> > -     unsigned int max_smt_thread_num = 1;
> > -     struct cpu_smt_info *entry;
> > -     struct xarray hetero_cpu;
> > -     unsigned long hetero_id;
> > -     int cpu, topology_id;
> > -
> > -     if (acpi_disabled)
> > -             return 0;
> > -
> > -     xa_init(&hetero_cpu);
> > -
> > -     for_each_possible_cpu(cpu) {
> > -             topology_id = find_acpi_cpu_topology(cpu, 0);
> > -             if (topology_id < 0)
> > -                     return topology_id;
> > -
> > -             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;
> > -
> > -                     /*
> > -                      * In the PPTT, CPUs below a node with the 'identical
> > -                      * implementation' flag have the same number of threads.
> > -                      * Count the number of threads for only one CPU (i.e.
> > -                      * one core_id) among those with the same hetero_id.
> > -                      * See the comment of find_acpi_cpu_topology_hetero_id()
> > -                      * for more details.
> > -                      *
> > -                      * One entry is created for each node having:
> > -                      * - the 'identical implementation' flag
> > -                      * - its parent not having the flag
> > -                      */
> > -                     hetero_id = find_acpi_cpu_topology_hetero_id(cpu);
> > -                     entry = xa_load(&hetero_cpu, hetero_id);
> > -                     if (!entry) {
> > -                             entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> > -                             WARN_ON_ONCE(!entry);
> > -
> > -                             if (entry) {
> > -                                     entry->core_id = topology_id;
> > -                                     entry->thread_num = 1;
> > -                                     xa_store(&hetero_cpu, hetero_id,
> > -                                              entry, GFP_KERNEL);
> > -                             }
> > -                     } else if (entry->core_id == topology_id) {
> > -                             entry->thread_num++;
> > -                     }
> > -             } else {
> > -                     cpu_topology[cpu].thread_id  = -1;
> > -                     cpu_topology[cpu].core_id    = topology_id;
> > -             }
> > -             topology_id = find_acpi_cpu_topology_cluster(cpu);
> > -             cpu_topology[cpu].cluster_id = topology_id;
> > -             topology_id = find_acpi_cpu_topology_package(cpu);
> > -             cpu_topology[cpu].package_id = topology_id;
> > -     }
> > -
> > -     /*
> > -      * This is a short loop since the number of XArray elements is the
> > -      * number of heterogeneous CPU clusters. On a homogeneous system
> > -      * there's only one entry in the XArray.
> > -      */
> > -     xa_for_each(&hetero_cpu, hetero_id, entry) {
> > -             max_smt_thread_num = max(max_smt_thread_num, entry->thread_num);
> > -             xa_erase(&hetero_cpu, hetero_id);
> > -             kfree(entry);
> > -     }
> > -
> > -     cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
> > -     xa_destroy(&hetero_cpu);
> > -     return 0;
> > -}
> >  #endif
> >
> >  #ifdef CONFIG_ARM64_AMU_EXTN
> > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > index 1037169abb459..09f77fd549490 100644
> > --- a/drivers/base/arch_topology.c
> > +++ b/drivers/base/arch_topology.c
> > @@ -823,12 +823,99 @@ void remove_cpu_topology(unsigned int cpu)
> >       clear_cpu_topology(cpu);
> >  }
> >
> > +__weak bool __init acpi_cpu_is_threaded(int cpu)
> > +{
> > +     int is_threaded = acpi_pptt_cpu_is_thread(cpu);
> > +
>
> Just thinking if it makes sense keep acpi_cpu_is_threaded generic without
> the need for weak definition.
>
> Additional note: not sure why you haven't moved this under CONFIG_ARM64/RISCV as
> done with other code.
>
> 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, check for architecture
>          * specific fallback if available
>          */
>         if (is_threaded < 0)
>                 is_threaded = arch_cpu_is_threaded();
>
>         return !!is_threaded;
> }
>
> Then you can just have the define in
>
> #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
>
> in arch/arm64/include/asm/topology.h
>
> and
>
> +#ifndef arch_cpu_is_threaded
> +#define arch_cpu_is_threaded           (0)
> +#endif
>
> in include/linux/arch_topology.h
>
> Thoughts ?

If placed in include/linux/arch_topology.h, there is a possibility
that "arch_cpu_is_threaded" will be redefined.

include/linux/topology.h
    #include <linux/arch_topology.h>
        #ifndef arch_cpu_is_threaded
        #define arch_cpu_is_threaded()  (0)
        #endif
    #include <asm/topology.h>
        #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)

Therefore, it is better to place it in include/asm-generic/topology.h.
What do you think?

>
> --
> Regards,
> Sudeep

Thanks,
Yunhui

Re: [External] Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[Sudeep Holla]

On Mon, Sep 22, 2025 at 10:18:57AM +0800, yunhui cui wrote:
> Hi Sudeep,
> 
> On Fri, Sep 19, 2025 at 10:05 PM Sudeep Holla <sudeep.holla@arm.com> wrote:

[...]

> >
> > Just thinking if it makes sense keep acpi_cpu_is_threaded generic without
> > the need for weak definition.
> >
> > Additional note: not sure why you haven't moved this under CONFIG_ARM64/RISCV as
> > done with other code.
> >
> > 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, check for architecture
> >          * specific fallback if available
> >          */
> >         if (is_threaded < 0)
> >                 is_threaded = arch_cpu_is_threaded();
> >
> >         return !!is_threaded;
> > }
> >
> > Then you can just have the define in
> >
> > #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
> >
> > in arch/arm64/include/asm/topology.h
> >
> > and
> >
> > +#ifndef arch_cpu_is_threaded
> > +#define arch_cpu_is_threaded           (0)
> > +#endif
> >
> > in include/linux/arch_topology.h
> >
> > Thoughts ?
> 
> If placed in include/linux/arch_topology.h, there is a possibility
> that "arch_cpu_is_threaded" will be redefined.
>

Why is that a problem ? We want arch to override the default definition
if and when required.

-- 
Regards,
Sudeep

Re: [External] Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[yunhui cui]

Hi Sudeep,

On Mon, Sep 22, 2025 at 5:01 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, Sep 22, 2025 at 10:18:57AM +0800, yunhui cui wrote:
> > Hi Sudeep,
> >
> > On Fri, Sep 19, 2025 at 10:05 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> [...]
>
> > >
> > > Just thinking if it makes sense keep acpi_cpu_is_threaded generic without
> > > the need for weak definition.
> > >
> > > Additional note: not sure why you haven't moved this under CONFIG_ARM64/RISCV as
> > > done with other code.
> > >
> > > 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, check for architecture
> > >          * specific fallback if available
> > >          */
> > >         if (is_threaded < 0)
> > >                 is_threaded = arch_cpu_is_threaded();
> > >
> > >         return !!is_threaded;
> > > }
> > >
> > > Then you can just have the define in
> > >
> > > #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
> > >
> > > in arch/arm64/include/asm/topology.h
> > >
> > > and
> > >
> > > +#ifndef arch_cpu_is_threaded
> > > +#define arch_cpu_is_threaded           (0)
> > > +#endif
> > >
> > > in include/linux/arch_topology.h
> > >
> > > Thoughts ?
> >
> > If placed in include/linux/arch_topology.h, there is a possibility
> > that "arch_cpu_is_threaded" will be redefined.
> >
>
> Why is that a problem ? We want arch to override the default definition
> if and when required.

Because include/linux/topology.h first includes linux/arch_topology.h,
and then includes asm/topology.h, a warning will be generated during
compilation:

In file included from ./include/linux/topology.h:37,
                 from ./include/linux/gfp.h:8,
                 from ./include/linux/xarray.h:16,
                 from ./include/linux/list_lru.h:14,
                 from ./include/linux/fs.h:14,
                 from kernel/events/core.c:11:
./arch/arm64/include/asm/topology.h:39: warning:
"arch_cpu_is_threaded" redefined
   39 | #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
      |
In file included from ./include/linux/topology.h:30:
./include/linux/arch_topology.h:94: note: this is the location of the
previous definition
   94 | #define arch_cpu_is_threaded()  (0)

Unless #undef arch_cpu_is_threaded is used in arch/arm64/include/asm/topology.h,

So it's better to place
#ifndef arch_cpu_is_threaded
#define arch_cpu_is_threaded (0)
#endif
in include/asm-generic/topology.h.

>
> --
> Regards,
> Sudeep

Thanks,
Yunhui

Re: [External] Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[Sudeep Holla]

On Mon, Sep 22, 2025 at 07:04:05PM +0800, yunhui cui wrote:
> Hi Sudeep,
> 
> On Mon, Sep 22, 2025 at 5:01 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Mon, Sep 22, 2025 at 10:18:57AM +0800, yunhui cui wrote:
> > > Hi Sudeep,
> > >
> > > On Fri, Sep 19, 2025 at 10:05 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > [...]
> >
> > > >
> > > > Just thinking if it makes sense keep acpi_cpu_is_threaded generic without
> > > > the need for weak definition.
> > > >
> > > > Additional note: not sure why you haven't moved this under CONFIG_ARM64/RISCV as
> > > > done with other code.
> > > >
> > > > 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, check for architecture
> > > >          * specific fallback if available
> > > >          */
> > > >         if (is_threaded < 0)
> > > >                 is_threaded = arch_cpu_is_threaded();
> > > >
> > > >         return !!is_threaded;
> > > > }
> > > >
> > > > Then you can just have the define in
> > > >
> > > > #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
> > > >
> > > > in arch/arm64/include/asm/topology.h
> > > >
> > > > and
> > > >
> > > > +#ifndef arch_cpu_is_threaded
> > > > +#define arch_cpu_is_threaded           (0)
> > > > +#endif
> > > >
> > > > in include/linux/arch_topology.h
> > > >
> > > > Thoughts ?
> > >
> > > If placed in include/linux/arch_topology.h, there is a possibility
> > > that "arch_cpu_is_threaded" will be redefined.
> > >
> >
> > Why is that a problem ? We want arch to override the default definition
> > if and when required.
> 
> Because include/linux/topology.h first includes linux/arch_topology.h,
> and then includes asm/topology.h, a warning will be generated during
> compilation:
> 
> In file included from ./include/linux/topology.h:37,
>                  from ./include/linux/gfp.h:8,
>                  from ./include/linux/xarray.h:16,
>                  from ./include/linux/list_lru.h:14,
>                  from ./include/linux/fs.h:14,
>                  from kernel/events/core.c:11:
> ./arch/arm64/include/asm/topology.h:39: warning:
> "arch_cpu_is_threaded" redefined
>    39 | #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
>       |
> In file included from ./include/linux/topology.h:30:
> ./include/linux/arch_topology.h:94: note: this is the location of the
> previous definition
>    94 | #define arch_cpu_is_threaded()  (0)
> 
> Unless #undef arch_cpu_is_threaded is used in arch/arm64/include/asm/topology.h,
> 

The above way of #undef first should be fine.

> So it's better to place
> #ifndef arch_cpu_is_threaded
> #define arch_cpu_is_threaded (0)
> #endif
> in include/asm-generic/topology.h.
>

I don't think asm-generic/topology.h is the right place and also you are
again better on order of inclusion.

-- 
Regards,
Sudeep