[v2] LoongArch: Fix cpu hotplug issue

[PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Bibo Mao 1 month ago

On LoongArch system, there are two places to set cpu numa node. One
is in arch specified function smp_prepare_boot_cpu(), the other is
in generic function early_numa_node_init(). The latter will overwrite
the numa node information.

With hot-added cpu without numa information, cpu_logical_map() fails
to its physical cpuid at beginning since it is not enabled in ACPI
MADT table. So function early_cpu_to_node() also fails to get its
numa node for hot-added cpu, and generic function
early_numa_node_init() will overwrite with incorrect numa node.

APIs topo_get_cpu() and topo_add_cpu() is added here, like other
architectures logic cpu is allocated when parsing MADT table. When
parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
all allocated logical cpu with matched physical id. It solves such
problems such as:
  1. Boot cpu is not the first entry in MADT table, the first entry
will be overwritten with later boot cpu.
  2. Physical cpu id not presented in MADT table is invalid, in later
SRAT/hot-add cpu parsing, invalid physical cpu detected is added
  3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
is correct for hot-add cpu.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
v1 ... v2:
  1. Like other architectures, allocate logic cpu when parsing MADT table.
  2. Add invalid or duplicated physical cpuid parsing with SRAT table or
hot-add cpu DSDT information.
---
 arch/loongarch/include/asm/smp.h |  3 ++
 arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
 arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
 arch/loongarch/kernel/smp.c      |  9 +++---
 4 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
index 3383c9d24e94..c61b75937a77 100644
--- a/arch/loongarch/include/asm/smp.h
+++ b/arch/loongarch/include/asm/smp.h
@@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
 #define cpu_logical_map(cpu)	0
 #endif /* CONFIG_SMP */
 
+int topo_add_cpu(int physid);
+int topo_get_cpu(int physid);
+
 #endif /* __ASM_SMP_H */
diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
index f1a74b80f22c..84d9812d5f38 100644
--- a/arch/loongarch/kernel/acpi.c
+++ b/arch/loongarch/kernel/acpi.c
@@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
 		return -ENODEV;
 
 	}
-	if (cpuid == loongson_sysconf.boot_cpu_id)
-		cpu = 0;
-	else
-		cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
+
+	cpu = topo_add_cpu(cpuid);
+	if (cpu < 0)
+		return -EEXIST;
 
 	if (!cpu_enumerated)
 		set_cpu_possible(cpu, true);
@@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
 		goto fdt_earlycon;
 	}
 
-	loongson_sysconf.boot_cpu_id = read_csr_cpuid();
-
 	/*
 	 * Process the Multiple APIC Description Table (MADT), if present
 	 */
@@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
 void __init
 acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 {
-	int pxm, node;
+	int pxm, node, cpu;
 
 	if (srat_disabled())
 		return;
@@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 		return;
 	}
 
+	cpu = topo_get_cpu(pa->apic_id);
+	/* Check whether apic_id exists in MADT table */
+	if (cpu < 0)
+		return;
+
 	early_numa_add_cpu(pa->apic_id, node);
 
 	set_cpuid_to_node(pa->apic_id, node);
@@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
 {
 	int cpu;
 
-	cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
+	cpu = topo_get_cpu(physid);
+	/* Check whether apic_id exists in MADT table */
 	if (cpu < 0) {
 		pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
 		return cpu;
 	}
 
+	num_processors++;
+	set_cpu_present(cpu, true);
+	__cpu_number_map[physid] = cpu;
+	__cpu_logical_map[cpu] = physid;
 	acpi_map_cpu2node(handle, cpu, physid);
 
 	*pcpu = cpu;
diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 00e307203ddb..649e98640076 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
 
 struct loongson_board_info b_info;
 static const char dmi_empty_string[] = "        ";
+static int possible_cpus;
+static bool bsp_added;
 
 /*
  * Setup information
@@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
 	*cmdline_p = boot_command_line;
 }
 
+int topo_get_cpu(int physid)
+{
+	int i;
+
+	for (i = 0; i < possible_cpus; i++)
+		if (cpu_logical_map(i) == physid)
+			break;
+
+	if (i == possible_cpus)
+		return -ENOENT;
+
+	return i;
+}
+
+int topo_add_cpu(int physid)
+{
+	int cpu;
+
+	if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
+		bsp_added = true;
+		return 0;
+	}
+
+	cpu = topo_get_cpu(physid);
+	if (cpu >= 0) {
+		pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
+		return -EEXIST;
+	}
+
+	if (possible_cpus >= nr_cpu_ids)
+		return -ERANGE;
+
+	__cpu_logical_map[possible_cpus] = physid;
+	cpu = possible_cpus++;
+	return cpu;
+}
+
+static void __init topo_init(void)
+{
+	loongson_sysconf.boot_cpu_id = read_csr_cpuid();
+	__cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
+	possible_cpus++;
+}
+
 void __init platform_init(void)
 {
 	arch_reserve_vmcore();
 	arch_reserve_crashkernel();
+	topo_init();
 
 #ifdef CONFIG_ACPI
 	acpi_table_upgrade();
diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index 9afc2d8b3414..a3f466b89179 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
 		if (cpuid >= nr_cpu_ids)
 			continue;
 
-		if (cpuid == loongson_sysconf.boot_cpu_id)
-			cpu = 0;
-		else
-			cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
+		cpu = topo_add_cpu(cpuid);
+		if (cpu < 0)
+			continue;
 
 		num_processors++;
 		set_cpu_possible(cpu, true);
@@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
 		__cpu_number_map[cpuid] = cpu;
 		__cpu_logical_map[cpu] = cpuid;
 
-		early_numa_add_cpu(cpu, 0);
+		early_numa_add_cpu(cpuid, 0);
 		set_cpuid_to_node(cpuid, 0);
 	}
 

base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
-- 
2.39.3

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by kernel test robot 1 month ago

Hi Bibo,

kernel test robot noticed the following build errors:

[auto build test ERROR on 42f7652d3eb527d03665b09edac47f85fb600924]

url:    https://github.com/intel-lab-lkp/linux/commits/Bibo-Mao/LoongArch-Fix-cpu-hotplug-issue/20241021-160525
base:   42f7652d3eb527d03665b09edac47f85fb600924
patch link:    https://lore.kernel.org/r/20241021080418.644342-1-maobibo%40loongson.cn
patch subject: [PATCH v2] LoongArch: Fix cpu hotplug issue
config: loongarch-allnoconfig (https://download.01.org/0day-ci/archive/20241022/202410220508.9OO5jJgk-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241022/202410220508.9OO5jJgk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410220508.9OO5jJgk-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/loongarch/kernel/setup.c: In function 'topo_add_cpu':
>> arch/loongarch/kernel/setup.c:383:9: error: '__cpu_logical_map' undeclared (first use in this function); did you mean 'cpu_logical_map'?
     383 |         __cpu_logical_map[possible_cpus] = physid;
         |         ^~~~~~~~~~~~~~~~~
         |         cpu_logical_map
   arch/loongarch/kernel/setup.c:383:9: note: each undeclared identifier is reported only once for each function it appears in
   arch/loongarch/kernel/setup.c: In function 'topo_init':
   arch/loongarch/kernel/setup.c:391:9: error: '__cpu_logical_map' undeclared (first use in this function); did you mean 'cpu_logical_map'?
     391 |         __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
         |         ^~~~~~~~~~~~~~~~~
         |         cpu_logical_map


vim +383 arch/loongarch/kernel/setup.c

   364	
   365	int topo_add_cpu(int physid)
   366	{
   367		int cpu;
   368	
   369		if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
   370			bsp_added = true;
   371			return 0;
   372		}
   373	
   374		cpu = topo_get_cpu(physid);
   375		if (cpu >= 0) {
   376			pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
   377			return -EEXIST;
   378		}
   379	
   380		if (possible_cpus >= nr_cpu_ids)
   381			return -ERANGE;
   382	
 > 383		__cpu_logical_map[possible_cpus] = physid;
   384		cpu = possible_cpus++;
   385		return cpu;
   386	}
   387	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 1 month ago

Hi, Bibo,

This version still doesn't touch the round-robin method, but it
doesn't matter, I think I misunderstood something since V1...

Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
smp_prepare_boot_cpu() the round-robin node ids only apply to
cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
early_cpu_to_node() returns NUMA_NO_NODE not because
__cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
< 0.

If the above is correct, we don't need so complicated, because the
correct and simplest way is:
https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465


Huacai

On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> On LoongArch system, there are two places to set cpu numa node. One
> is in arch specified function smp_prepare_boot_cpu(), the other is
> in generic function early_numa_node_init(). The latter will overwrite
> the numa node information.
>
> With hot-added cpu without numa information, cpu_logical_map() fails
> to its physical cpuid at beginning since it is not enabled in ACPI
> MADT table. So function early_cpu_to_node() also fails to get its
> numa node for hot-added cpu, and generic function
> early_numa_node_init() will overwrite with incorrect numa node.
>
> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> architectures logic cpu is allocated when parsing MADT table. When
> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> all allocated logical cpu with matched physical id. It solves such
> problems such as:
>   1. Boot cpu is not the first entry in MADT table, the first entry
> will be overwritten with later boot cpu.
>   2. Physical cpu id not presented in MADT table is invalid, in later
> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>   3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> is correct for hot-add cpu.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> v1 ... v2:
>   1. Like other architectures, allocate logic cpu when parsing MADT table.
>   2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> hot-add cpu DSDT information.
> ---
>  arch/loongarch/include/asm/smp.h |  3 ++
>  arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>  arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>  arch/loongarch/kernel/smp.c      |  9 +++---
>  4 files changed, 70 insertions(+), 13 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> index 3383c9d24e94..c61b75937a77 100644
> --- a/arch/loongarch/include/asm/smp.h
> +++ b/arch/loongarch/include/asm/smp.h
> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>  #define cpu_logical_map(cpu)   0
>  #endif /* CONFIG_SMP */
>
> +int topo_add_cpu(int physid);
> +int topo_get_cpu(int physid);
> +
>  #endif /* __ASM_SMP_H */
> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> index f1a74b80f22c..84d9812d5f38 100644
> --- a/arch/loongarch/kernel/acpi.c
> +++ b/arch/loongarch/kernel/acpi.c
> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>                 return -ENODEV;
>
>         }
> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> -               cpu = 0;
> -       else
> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> +
> +       cpu = topo_add_cpu(cpuid);
> +       if (cpu < 0)
> +               return -EEXIST;
>
>         if (!cpu_enumerated)
>                 set_cpu_possible(cpu, true);
> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>                 goto fdt_earlycon;
>         }
>
> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> -
>         /*
>          * Process the Multiple APIC Description Table (MADT), if present
>          */
> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>  void __init
>  acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>  {
> -       int pxm, node;
> +       int pxm, node, cpu;
>
>         if (srat_disabled())
>                 return;
> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>                 return;
>         }
>
> +       cpu = topo_get_cpu(pa->apic_id);
> +       /* Check whether apic_id exists in MADT table */
> +       if (cpu < 0)
> +               return;
> +
>         early_numa_add_cpu(pa->apic_id, node);
>
>         set_cpuid_to_node(pa->apic_id, node);
> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>  {
>         int cpu;
>
> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> +       cpu = topo_get_cpu(physid);
> +       /* Check whether apic_id exists in MADT table */
>         if (cpu < 0) {
>                 pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>                 return cpu;
>         }
>
> +       num_processors++;
> +       set_cpu_present(cpu, true);
> +       __cpu_number_map[physid] = cpu;
> +       __cpu_logical_map[cpu] = physid;
>         acpi_map_cpu2node(handle, cpu, physid);
>
>         *pcpu = cpu;
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 00e307203ddb..649e98640076 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>
>  struct loongson_board_info b_info;
>  static const char dmi_empty_string[] = "        ";
> +static int possible_cpus;
> +static bool bsp_added;
>
>  /*
>   * Setup information
> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>         *cmdline_p = boot_command_line;
>  }
>
> +int topo_get_cpu(int physid)
> +{
> +       int i;
> +
> +       for (i = 0; i < possible_cpus; i++)
> +               if (cpu_logical_map(i) == physid)
> +                       break;
> +
> +       if (i == possible_cpus)
> +               return -ENOENT;
> +
> +       return i;
> +}
> +
> +int topo_add_cpu(int physid)
> +{
> +       int cpu;
> +
> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> +               bsp_added = true;
> +               return 0;
> +       }
> +
> +       cpu = topo_get_cpu(physid);
> +       if (cpu >= 0) {
> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> +               return -EEXIST;
> +       }
> +
> +       if (possible_cpus >= nr_cpu_ids)
> +               return -ERANGE;
> +
> +       __cpu_logical_map[possible_cpus] = physid;
> +       cpu = possible_cpus++;
> +       return cpu;
> +}
> +
> +static void __init topo_init(void)
> +{
> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> +       possible_cpus++;
> +}
> +
>  void __init platform_init(void)
>  {
>         arch_reserve_vmcore();
>         arch_reserve_crashkernel();
> +       topo_init();
>
>  #ifdef CONFIG_ACPI
>         acpi_table_upgrade();
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index 9afc2d8b3414..a3f466b89179 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>                 if (cpuid >= nr_cpu_ids)
>                         continue;
>
> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> -                       cpu = 0;
> -               else
> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> +               cpu = topo_add_cpu(cpuid);
> +               if (cpu < 0)
> +                       continue;
>
>                 num_processors++;
>                 set_cpu_possible(cpu, true);
> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>                 __cpu_number_map[cpuid] = cpu;
>                 __cpu_logical_map[cpu] = cpuid;
>
> -               early_numa_add_cpu(cpu, 0);
> +               early_numa_add_cpu(cpuid, 0);
>                 set_cpuid_to_node(cpuid, 0);
>         }
>
>
> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> --
> 2.39.3
>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 1 month ago


On 2024/10/21 下午10:32, Huacai Chen wrote:
> Hi, Bibo,
> 
> This version still doesn't touch the round-robin method, but it
> doesn't matter, I think I misunderstood something since V1...
I do not understand why round-robin method need be modified, SRAT may be 
disabled with general function disable_srat(). Then round-robin method 
is required.

> 
> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> smp_prepare_boot_cpu() the round-robin node ids only apply to
> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> early_cpu_to_node() returns NUMA_NO_NODE not because
> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> < 0.
> 
> If the above is correct, we don't need so complicated, because the
> correct and simplest way is:
> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> 
It works also. Only that LoongArch kernel parsing about SRAT/MADT is 
badly. If you do not mind, I do not mind neither. It is not my duty for 
kernel side.

Bibo Mao
> 
> Huacai
> 
> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> On LoongArch system, there are two places to set cpu numa node. One
>> is in arch specified function smp_prepare_boot_cpu(), the other is
>> in generic function early_numa_node_init(). The latter will overwrite
>> the numa node information.
>>
>> With hot-added cpu without numa information, cpu_logical_map() fails
>> to its physical cpuid at beginning since it is not enabled in ACPI
>> MADT table. So function early_cpu_to_node() also fails to get its
>> numa node for hot-added cpu, and generic function
>> early_numa_node_init() will overwrite with incorrect numa node.
>>
>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>> architectures logic cpu is allocated when parsing MADT table. When
>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>> all allocated logical cpu with matched physical id. It solves such
>> problems such as:
>>    1. Boot cpu is not the first entry in MADT table, the first entry
>> will be overwritten with later boot cpu.
>>    2. Physical cpu id not presented in MADT table is invalid, in later
>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>    3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>> is correct for hot-add cpu.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> v1 ... v2:
>>    1. Like other architectures, allocate logic cpu when parsing MADT table.
>>    2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>> hot-add cpu DSDT information.
>> ---
>>   arch/loongarch/include/asm/smp.h |  3 ++
>>   arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>   arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>   arch/loongarch/kernel/smp.c      |  9 +++---
>>   4 files changed, 70 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>> index 3383c9d24e94..c61b75937a77 100644
>> --- a/arch/loongarch/include/asm/smp.h
>> +++ b/arch/loongarch/include/asm/smp.h
>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>   #define cpu_logical_map(cpu)   0
>>   #endif /* CONFIG_SMP */
>>
>> +int topo_add_cpu(int physid);
>> +int topo_get_cpu(int physid);
>> +
>>   #endif /* __ASM_SMP_H */
>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>> index f1a74b80f22c..84d9812d5f38 100644
>> --- a/arch/loongarch/kernel/acpi.c
>> +++ b/arch/loongarch/kernel/acpi.c
>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>                  return -ENODEV;
>>
>>          }
>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>> -               cpu = 0;
>> -       else
>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>> +
>> +       cpu = topo_add_cpu(cpuid);
>> +       if (cpu < 0)
>> +               return -EEXIST;
>>
>>          if (!cpu_enumerated)
>>                  set_cpu_possible(cpu, true);
>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>                  goto fdt_earlycon;
>>          }
>>
>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>> -
>>          /*
>>           * Process the Multiple APIC Description Table (MADT), if present
>>           */
>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>   void __init
>>   acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>   {
>> -       int pxm, node;
>> +       int pxm, node, cpu;
>>
>>          if (srat_disabled())
>>                  return;
>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>                  return;
>>          }
>>
>> +       cpu = topo_get_cpu(pa->apic_id);
>> +       /* Check whether apic_id exists in MADT table */
>> +       if (cpu < 0)
>> +               return;
>> +
>>          early_numa_add_cpu(pa->apic_id, node);
>>
>>          set_cpuid_to_node(pa->apic_id, node);
>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>   {
>>          int cpu;
>>
>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>> +       cpu = topo_get_cpu(physid);
>> +       /* Check whether apic_id exists in MADT table */
>>          if (cpu < 0) {
>>                  pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>                  return cpu;
>>          }
>>
>> +       num_processors++;
>> +       set_cpu_present(cpu, true);
>> +       __cpu_number_map[physid] = cpu;
>> +       __cpu_logical_map[cpu] = physid;
>>          acpi_map_cpu2node(handle, cpu, physid);
>>
>>          *pcpu = cpu;
>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>> index 00e307203ddb..649e98640076 100644
>> --- a/arch/loongarch/kernel/setup.c
>> +++ b/arch/loongarch/kernel/setup.c
>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>
>>   struct loongson_board_info b_info;
>>   static const char dmi_empty_string[] = "        ";
>> +static int possible_cpus;
>> +static bool bsp_added;
>>
>>   /*
>>    * Setup information
>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>          *cmdline_p = boot_command_line;
>>   }
>>
>> +int topo_get_cpu(int physid)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < possible_cpus; i++)
>> +               if (cpu_logical_map(i) == physid)
>> +                       break;
>> +
>> +       if (i == possible_cpus)
>> +               return -ENOENT;
>> +
>> +       return i;
>> +}
>> +
>> +int topo_add_cpu(int physid)
>> +{
>> +       int cpu;
>> +
>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>> +               bsp_added = true;
>> +               return 0;
>> +       }
>> +
>> +       cpu = topo_get_cpu(physid);
>> +       if (cpu >= 0) {
>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>> +               return -EEXIST;
>> +       }
>> +
>> +       if (possible_cpus >= nr_cpu_ids)
>> +               return -ERANGE;
>> +
>> +       __cpu_logical_map[possible_cpus] = physid;
>> +       cpu = possible_cpus++;
>> +       return cpu;
>> +}
>> +
>> +static void __init topo_init(void)
>> +{
>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>> +       possible_cpus++;
>> +}
>> +
>>   void __init platform_init(void)
>>   {
>>          arch_reserve_vmcore();
>>          arch_reserve_crashkernel();
>> +       topo_init();
>>
>>   #ifdef CONFIG_ACPI
>>          acpi_table_upgrade();
>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>> index 9afc2d8b3414..a3f466b89179 100644
>> --- a/arch/loongarch/kernel/smp.c
>> +++ b/arch/loongarch/kernel/smp.c
>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>                  if (cpuid >= nr_cpu_ids)
>>                          continue;
>>
>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>> -                       cpu = 0;
>> -               else
>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>> +               cpu = topo_add_cpu(cpuid);
>> +               if (cpu < 0)
>> +                       continue;
>>
>>                  num_processors++;
>>                  set_cpu_possible(cpu, true);
>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>                  __cpu_number_map[cpuid] = cpu;
>>                  __cpu_logical_map[cpu] = cpuid;
>>
>> -               early_numa_add_cpu(cpu, 0);
>> +               early_numa_add_cpu(cpuid, 0);
>>                  set_cpuid_to_node(cpuid, 0);
>>          }
>>
>>
>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>> --
>> 2.39.3
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 1 month ago

On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/10/21 下午10:32, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > This version still doesn't touch the round-robin method, but it
> > doesn't matter, I think I misunderstood something since V1...
> I do not understand why round-robin method need be modified, SRAT may be
> disabled with general function disable_srat(). Then round-robin method
> is required.
I don't mean round-robin should be modified, I mean I misunderstand round-robin.

>
> >
> > Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> > smp_prepare_boot_cpu() the round-robin node ids only apply to
> > cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> > early_cpu_to_node() returns NUMA_NO_NODE not because
> > __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> > < 0.
> >
> > If the above is correct, we don't need so complicated, because the
> > correct and simplest way is:
> > https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> >
> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
> badly. If you do not mind, I do not mind neither. It is not my duty for
> kernel side.
Yes, I don't mind, please use that simplest way.

Huacai

>
> Bibo Mao
> >
> > Huacai
> >
> > On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> On LoongArch system, there are two places to set cpu numa node. One
> >> is in arch specified function smp_prepare_boot_cpu(), the other is
> >> in generic function early_numa_node_init(). The latter will overwrite
> >> the numa node information.
> >>
> >> With hot-added cpu without numa information, cpu_logical_map() fails
> >> to its physical cpuid at beginning since it is not enabled in ACPI
> >> MADT table. So function early_cpu_to_node() also fails to get its
> >> numa node for hot-added cpu, and generic function
> >> early_numa_node_init() will overwrite with incorrect numa node.
> >>
> >> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> >> architectures logic cpu is allocated when parsing MADT table. When
> >> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> >> all allocated logical cpu with matched physical id. It solves such
> >> problems such as:
> >>    1. Boot cpu is not the first entry in MADT table, the first entry
> >> will be overwritten with later boot cpu.
> >>    2. Physical cpu id not presented in MADT table is invalid, in later
> >> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
> >>    3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> >> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> >> is correct for hot-add cpu.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >> v1 ... v2:
> >>    1. Like other architectures, allocate logic cpu when parsing MADT table.
> >>    2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> >> hot-add cpu DSDT information.
> >> ---
> >>   arch/loongarch/include/asm/smp.h |  3 ++
> >>   arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
> >>   arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
> >>   arch/loongarch/kernel/smp.c      |  9 +++---
> >>   4 files changed, 70 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >> index 3383c9d24e94..c61b75937a77 100644
> >> --- a/arch/loongarch/include/asm/smp.h
> >> +++ b/arch/loongarch/include/asm/smp.h
> >> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
> >>   #define cpu_logical_map(cpu)   0
> >>   #endif /* CONFIG_SMP */
> >>
> >> +int topo_add_cpu(int physid);
> >> +int topo_get_cpu(int physid);
> >> +
> >>   #endif /* __ASM_SMP_H */
> >> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> >> index f1a74b80f22c..84d9812d5f38 100644
> >> --- a/arch/loongarch/kernel/acpi.c
> >> +++ b/arch/loongarch/kernel/acpi.c
> >> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
> >>                  return -ENODEV;
> >>
> >>          }
> >> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> >> -               cpu = 0;
> >> -       else
> >> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >> +
> >> +       cpu = topo_add_cpu(cpuid);
> >> +       if (cpu < 0)
> >> +               return -EEXIST;
> >>
> >>          if (!cpu_enumerated)
> >>                  set_cpu_possible(cpu, true);
> >> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
> >>                  goto fdt_earlycon;
> >>          }
> >>
> >> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >> -
> >>          /*
> >>           * Process the Multiple APIC Description Table (MADT), if present
> >>           */
> >> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
> >>   void __init
> >>   acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>   {
> >> -       int pxm, node;
> >> +       int pxm, node, cpu;
> >>
> >>          if (srat_disabled())
> >>                  return;
> >> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>                  return;
> >>          }
> >>
> >> +       cpu = topo_get_cpu(pa->apic_id);
> >> +       /* Check whether apic_id exists in MADT table */
> >> +       if (cpu < 0)
> >> +               return;
> >> +
> >>          early_numa_add_cpu(pa->apic_id, node);
> >>
> >>          set_cpuid_to_node(pa->apic_id, node);
> >> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
> >>   {
> >>          int cpu;
> >>
> >> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> >> +       cpu = topo_get_cpu(physid);
> >> +       /* Check whether apic_id exists in MADT table */
> >>          if (cpu < 0) {
> >>                  pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
> >>                  return cpu;
> >>          }
> >>
> >> +       num_processors++;
> >> +       set_cpu_present(cpu, true);
> >> +       __cpu_number_map[physid] = cpu;
> >> +       __cpu_logical_map[cpu] = physid;
> >>          acpi_map_cpu2node(handle, cpu, physid);
> >>
> >>          *pcpu = cpu;
> >> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> >> index 00e307203ddb..649e98640076 100644
> >> --- a/arch/loongarch/kernel/setup.c
> >> +++ b/arch/loongarch/kernel/setup.c
> >> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
> >>
> >>   struct loongson_board_info b_info;
> >>   static const char dmi_empty_string[] = "        ";
> >> +static int possible_cpus;
> >> +static bool bsp_added;
> >>
> >>   /*
> >>    * Setup information
> >> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
> >>          *cmdline_p = boot_command_line;
> >>   }
> >>
> >> +int topo_get_cpu(int physid)
> >> +{
> >> +       int i;
> >> +
> >> +       for (i = 0; i < possible_cpus; i++)
> >> +               if (cpu_logical_map(i) == physid)
> >> +                       break;
> >> +
> >> +       if (i == possible_cpus)
> >> +               return -ENOENT;
> >> +
> >> +       return i;
> >> +}
> >> +
> >> +int topo_add_cpu(int physid)
> >> +{
> >> +       int cpu;
> >> +
> >> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> >> +               bsp_added = true;
> >> +               return 0;
> >> +       }
> >> +
> >> +       cpu = topo_get_cpu(physid);
> >> +       if (cpu >= 0) {
> >> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> >> +               return -EEXIST;
> >> +       }
> >> +
> >> +       if (possible_cpus >= nr_cpu_ids)
> >> +               return -ERANGE;
> >> +
> >> +       __cpu_logical_map[possible_cpus] = physid;
> >> +       cpu = possible_cpus++;
> >> +       return cpu;
> >> +}
> >> +
> >> +static void __init topo_init(void)
> >> +{
> >> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> >> +       possible_cpus++;
> >> +}
> >> +
> >>   void __init platform_init(void)
> >>   {
> >>          arch_reserve_vmcore();
> >>          arch_reserve_crashkernel();
> >> +       topo_init();
> >>
> >>   #ifdef CONFIG_ACPI
> >>          acpi_table_upgrade();
> >> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >> index 9afc2d8b3414..a3f466b89179 100644
> >> --- a/arch/loongarch/kernel/smp.c
> >> +++ b/arch/loongarch/kernel/smp.c
> >> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
> >>                  if (cpuid >= nr_cpu_ids)
> >>                          continue;
> >>
> >> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> >> -                       cpu = 0;
> >> -               else
> >> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >> +               cpu = topo_add_cpu(cpuid);
> >> +               if (cpu < 0)
> >> +                       continue;
> >>
> >>                  num_processors++;
> >>                  set_cpu_possible(cpu, true);
> >> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
> >>                  __cpu_number_map[cpuid] = cpu;
> >>                  __cpu_logical_map[cpu] = cpuid;
> >>
> >> -               early_numa_add_cpu(cpu, 0);
> >> +               early_numa_add_cpu(cpuid, 0);
> >>                  set_cpuid_to_node(cpuid, 0);
> >>          }
> >>
> >>
> >> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> >> --
> >> 2.39.3
> >>
>
>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 4 weeks ago

Hi Huacai,

On 2024/10/22 上午9:31, Huacai Chen wrote:
> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/10/21 下午10:32, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> This version still doesn't touch the round-robin method, but it
>>> doesn't matter, I think I misunderstood something since V1...
>> I do not understand why round-robin method need be modified, SRAT may be
>> disabled with general function disable_srat(). Then round-robin method
>> is required.
> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
> 
>>
>>>
>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
>>> early_cpu_to_node() returns NUMA_NO_NODE not because
>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
>>> < 0.
>>>
>>> If the above is correct, we don't need so complicated, because the
>>> correct and simplest way is:
>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
>>>
>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
>> badly. If you do not mind, I do not mind neither. It is not my duty for
>> kernel side.
> Yes, I don't mind, please use that simplest way.
There is another problem with the simple way. eiointc reports error when 
cpu is online. The error message is:
   Loongson-64bit Processor probed (LA464 Core)
   CPU2 revision is: 0014c010 (Loongson-64bit)
   FPU2 revision is: 00000001
   eiointc: Error: invalid nodemap!
   CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)

The problem is that node_map of eiointc is problematic,


static int cpu_to_eio_node(int cpu)
{
         return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
}

static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
                                u64 node_map)
{
         int i;

         node_map = node_map ? node_map : -1ULL;
         for_each_possible_cpu(i) {
                 if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
                         node_set(cpu_to_eio_node(i), priv->node_map);
          ...
The cause is that for possible not present cpu, *cpu_logical_map(cpu)* 
is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is 
problematic.

So cpu_logical_map(cpu) should be set during MADT parsing even if it is 
not enabled at beginning, it should not be set at hotplug runtime.

Regards
Bibo Mao


> 
> Huacai
> 
>>
>> Bibo Mao
>>>
>>> Huacai
>>>
>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> On LoongArch system, there are two places to set cpu numa node. One
>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
>>>> in generic function early_numa_node_init(). The latter will overwrite
>>>> the numa node information.
>>>>
>>>> With hot-added cpu without numa information, cpu_logical_map() fails
>>>> to its physical cpuid at beginning since it is not enabled in ACPI
>>>> MADT table. So function early_cpu_to_node() also fails to get its
>>>> numa node for hot-added cpu, and generic function
>>>> early_numa_node_init() will overwrite with incorrect numa node.
>>>>
>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>>>> architectures logic cpu is allocated when parsing MADT table. When
>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>>>> all allocated logical cpu with matched physical id. It solves such
>>>> problems such as:
>>>>     1. Boot cpu is not the first entry in MADT table, the first entry
>>>> will be overwritten with later boot cpu.
>>>>     2. Physical cpu id not presented in MADT table is invalid, in later
>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>>>     3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>>>> is correct for hot-add cpu.
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>> v1 ... v2:
>>>>     1. Like other architectures, allocate logic cpu when parsing MADT table.
>>>>     2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>>>> hot-add cpu DSDT information.
>>>> ---
>>>>    arch/loongarch/include/asm/smp.h |  3 ++
>>>>    arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>>>    arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>>>    arch/loongarch/kernel/smp.c      |  9 +++---
>>>>    4 files changed, 70 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>> index 3383c9d24e94..c61b75937a77 100644
>>>> --- a/arch/loongarch/include/asm/smp.h
>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>>>    #define cpu_logical_map(cpu)   0
>>>>    #endif /* CONFIG_SMP */
>>>>
>>>> +int topo_add_cpu(int physid);
>>>> +int topo_get_cpu(int physid);
>>>> +
>>>>    #endif /* __ASM_SMP_H */
>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>>>> index f1a74b80f22c..84d9812d5f38 100644
>>>> --- a/arch/loongarch/kernel/acpi.c
>>>> +++ b/arch/loongarch/kernel/acpi.c
>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>>>                   return -ENODEV;
>>>>
>>>>           }
>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>>>> -               cpu = 0;
>>>> -       else
>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>> +
>>>> +       cpu = topo_add_cpu(cpuid);
>>>> +       if (cpu < 0)
>>>> +               return -EEXIST;
>>>>
>>>>           if (!cpu_enumerated)
>>>>                   set_cpu_possible(cpu, true);
>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>>>                   goto fdt_earlycon;
>>>>           }
>>>>
>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>> -
>>>>           /*
>>>>            * Process the Multiple APIC Description Table (MADT), if present
>>>>            */
>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>>>    void __init
>>>>    acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>    {
>>>> -       int pxm, node;
>>>> +       int pxm, node, cpu;
>>>>
>>>>           if (srat_disabled())
>>>>                   return;
>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>                   return;
>>>>           }
>>>>
>>>> +       cpu = topo_get_cpu(pa->apic_id);
>>>> +       /* Check whether apic_id exists in MADT table */
>>>> +       if (cpu < 0)
>>>> +               return;
>>>> +
>>>>           early_numa_add_cpu(pa->apic_id, node);
>>>>
>>>>           set_cpuid_to_node(pa->apic_id, node);
>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>>>    {
>>>>           int cpu;
>>>>
>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>>>> +       cpu = topo_get_cpu(physid);
>>>> +       /* Check whether apic_id exists in MADT table */
>>>>           if (cpu < 0) {
>>>>                   pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>>>                   return cpu;
>>>>           }
>>>>
>>>> +       num_processors++;
>>>> +       set_cpu_present(cpu, true);
>>>> +       __cpu_number_map[physid] = cpu;
>>>> +       __cpu_logical_map[cpu] = physid;
>>>>           acpi_map_cpu2node(handle, cpu, physid);
>>>>
>>>>           *pcpu = cpu;
>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>> index 00e307203ddb..649e98640076 100644
>>>> --- a/arch/loongarch/kernel/setup.c
>>>> +++ b/arch/loongarch/kernel/setup.c
>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>>>
>>>>    struct loongson_board_info b_info;
>>>>    static const char dmi_empty_string[] = "        ";
>>>> +static int possible_cpus;
>>>> +static bool bsp_added;
>>>>
>>>>    /*
>>>>     * Setup information
>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>>>           *cmdline_p = boot_command_line;
>>>>    }
>>>>
>>>> +int topo_get_cpu(int physid)
>>>> +{
>>>> +       int i;
>>>> +
>>>> +       for (i = 0; i < possible_cpus; i++)
>>>> +               if (cpu_logical_map(i) == physid)
>>>> +                       break;
>>>> +
>>>> +       if (i == possible_cpus)
>>>> +               return -ENOENT;
>>>> +
>>>> +       return i;
>>>> +}
>>>> +
>>>> +int topo_add_cpu(int physid)
>>>> +{
>>>> +       int cpu;
>>>> +
>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>>>> +               bsp_added = true;
>>>> +               return 0;
>>>> +       }
>>>> +
>>>> +       cpu = topo_get_cpu(physid);
>>>> +       if (cpu >= 0) {
>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>>>> +               return -EEXIST;
>>>> +       }
>>>> +
>>>> +       if (possible_cpus >= nr_cpu_ids)
>>>> +               return -ERANGE;
>>>> +
>>>> +       __cpu_logical_map[possible_cpus] = physid;
>>>> +       cpu = possible_cpus++;
>>>> +       return cpu;
>>>> +}
>>>> +
>>>> +static void __init topo_init(void)
>>>> +{
>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>>>> +       possible_cpus++;
>>>> +}
>>>> +
>>>>    void __init platform_init(void)
>>>>    {
>>>>           arch_reserve_vmcore();
>>>>           arch_reserve_crashkernel();
>>>> +       topo_init();
>>>>
>>>>    #ifdef CONFIG_ACPI
>>>>           acpi_table_upgrade();
>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>> index 9afc2d8b3414..a3f466b89179 100644
>>>> --- a/arch/loongarch/kernel/smp.c
>>>> +++ b/arch/loongarch/kernel/smp.c
>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>>>                   if (cpuid >= nr_cpu_ids)
>>>>                           continue;
>>>>
>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>>>> -                       cpu = 0;
>>>> -               else
>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>> +               cpu = topo_add_cpu(cpuid);
>>>> +               if (cpu < 0)
>>>> +                       continue;
>>>>
>>>>                   num_processors++;
>>>>                   set_cpu_possible(cpu, true);
>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>>>                   __cpu_number_map[cpuid] = cpu;
>>>>                   __cpu_logical_map[cpu] = cpuid;
>>>>
>>>> -               early_numa_add_cpu(cpu, 0);
>>>> +               early_numa_add_cpu(cpuid, 0);
>>>>                   set_cpuid_to_node(cpuid, 0);
>>>>           }
>>>>
>>>>
>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>>>> --
>>>> 2.39.3
>>>>
>>
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 3 weeks, 6 days ago

On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
>
> Hi Huacai,
>
> On 2024/10/22 上午9:31, Huacai Chen wrote:
> > On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/10/21 下午10:32, Huacai Chen wrote:
> >>> Hi, Bibo,
> >>>
> >>> This version still doesn't touch the round-robin method, but it
> >>> doesn't matter, I think I misunderstood something since V1...
> >> I do not understand why round-robin method need be modified, SRAT may be
> >> disabled with general function disable_srat(). Then round-robin method
> >> is required.
> > I don't mean round-robin should be modified, I mean I misunderstand round-robin.
> >
> >>
> >>>
> >>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> >>> smp_prepare_boot_cpu() the round-robin node ids only apply to
> >>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> >>> early_cpu_to_node() returns NUMA_NO_NODE not because
> >>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> >>> < 0.
> >>>
> >>> If the above is correct, we don't need so complicated, because the
> >>> correct and simplest way is:
> >>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> >>>
> >> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
> >> badly. If you do not mind, I do not mind neither. It is not my duty for
> >> kernel side.
> > Yes, I don't mind, please use that simplest way.
> There is another problem with the simple way. eiointc reports error when
> cpu is online. The error message is:
>    Loongson-64bit Processor probed (LA464 Core)
>    CPU2 revision is: 0014c010 (Loongson-64bit)
>    FPU2 revision is: 00000001
>    eiointc: Error: invalid nodemap!
>    CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
>
> The problem is that node_map of eiointc is problematic,
>
>
> static int cpu_to_eio_node(int cpu)
> {
>          return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
> }
>
> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
>                                 u64 node_map)
> {
>          int i;
>
>          node_map = node_map ? node_map : -1ULL;
>          for_each_possible_cpu(i) {
>                  if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
>                          node_set(cpu_to_eio_node(i), priv->node_map);
>           ...
> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
> problematic.
The error message seems from eiointc_router_init(), but it is a little
strange. Physical hot-add should be before logical hot-add. So
acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
set_processor_mask() to setup logical-physical mapping, so in
eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
should work well.

Maybe in your case a whole node is hot-added? I don't think the
eiointc design can work with this case...

>
> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
> not enabled at beginning, it should not be set at hotplug runtime.
This will cause the logical cpu number be not continuous after boot.
Physical numbers have no requirement, but logical numbers should be
continuous.

Huacai

>
> Regards
> Bibo Mao
>
>
> >
> > Huacai
> >
> >>
> >> Bibo Mao
> >>>
> >>> Huacai
> >>>
> >>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> On LoongArch system, there are two places to set cpu numa node. One
> >>>> is in arch specified function smp_prepare_boot_cpu(), the other is
> >>>> in generic function early_numa_node_init(). The latter will overwrite
> >>>> the numa node information.
> >>>>
> >>>> With hot-added cpu without numa information, cpu_logical_map() fails
> >>>> to its physical cpuid at beginning since it is not enabled in ACPI
> >>>> MADT table. So function early_cpu_to_node() also fails to get its
> >>>> numa node for hot-added cpu, and generic function
> >>>> early_numa_node_init() will overwrite with incorrect numa node.
> >>>>
> >>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> >>>> architectures logic cpu is allocated when parsing MADT table. When
> >>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> >>>> all allocated logical cpu with matched physical id. It solves such
> >>>> problems such as:
> >>>>     1. Boot cpu is not the first entry in MADT table, the first entry
> >>>> will be overwritten with later boot cpu.
> >>>>     2. Physical cpu id not presented in MADT table is invalid, in later
> >>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
> >>>>     3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> >>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> >>>> is correct for hot-add cpu.
> >>>>
> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>> ---
> >>>> v1 ... v2:
> >>>>     1. Like other architectures, allocate logic cpu when parsing MADT table.
> >>>>     2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> >>>> hot-add cpu DSDT information.
> >>>> ---
> >>>>    arch/loongarch/include/asm/smp.h |  3 ++
> >>>>    arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
> >>>>    arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
> >>>>    arch/loongarch/kernel/smp.c      |  9 +++---
> >>>>    4 files changed, 70 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >>>> index 3383c9d24e94..c61b75937a77 100644
> >>>> --- a/arch/loongarch/include/asm/smp.h
> >>>> +++ b/arch/loongarch/include/asm/smp.h
> >>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
> >>>>    #define cpu_logical_map(cpu)   0
> >>>>    #endif /* CONFIG_SMP */
> >>>>
> >>>> +int topo_add_cpu(int physid);
> >>>> +int topo_get_cpu(int physid);
> >>>> +
> >>>>    #endif /* __ASM_SMP_H */
> >>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> >>>> index f1a74b80f22c..84d9812d5f38 100644
> >>>> --- a/arch/loongarch/kernel/acpi.c
> >>>> +++ b/arch/loongarch/kernel/acpi.c
> >>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
> >>>>                   return -ENODEV;
> >>>>
> >>>>           }
> >>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>> -               cpu = 0;
> >>>> -       else
> >>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>> +
> >>>> +       cpu = topo_add_cpu(cpuid);
> >>>> +       if (cpu < 0)
> >>>> +               return -EEXIST;
> >>>>
> >>>>           if (!cpu_enumerated)
> >>>>                   set_cpu_possible(cpu, true);
> >>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
> >>>>                   goto fdt_earlycon;
> >>>>           }
> >>>>
> >>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>> -
> >>>>           /*
> >>>>            * Process the Multiple APIC Description Table (MADT), if present
> >>>>            */
> >>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
> >>>>    void __init
> >>>>    acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>    {
> >>>> -       int pxm, node;
> >>>> +       int pxm, node, cpu;
> >>>>
> >>>>           if (srat_disabled())
> >>>>                   return;
> >>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>                   return;
> >>>>           }
> >>>>
> >>>> +       cpu = topo_get_cpu(pa->apic_id);
> >>>> +       /* Check whether apic_id exists in MADT table */
> >>>> +       if (cpu < 0)
> >>>> +               return;
> >>>> +
> >>>>           early_numa_add_cpu(pa->apic_id, node);
> >>>>
> >>>>           set_cpuid_to_node(pa->apic_id, node);
> >>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
> >>>>    {
> >>>>           int cpu;
> >>>>
> >>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> >>>> +       cpu = topo_get_cpu(physid);
> >>>> +       /* Check whether apic_id exists in MADT table */
> >>>>           if (cpu < 0) {
> >>>>                   pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
> >>>>                   return cpu;
> >>>>           }
> >>>>
> >>>> +       num_processors++;
> >>>> +       set_cpu_present(cpu, true);
> >>>> +       __cpu_number_map[physid] = cpu;
> >>>> +       __cpu_logical_map[cpu] = physid;
> >>>>           acpi_map_cpu2node(handle, cpu, physid);
> >>>>
> >>>>           *pcpu = cpu;
> >>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> >>>> index 00e307203ddb..649e98640076 100644
> >>>> --- a/arch/loongarch/kernel/setup.c
> >>>> +++ b/arch/loongarch/kernel/setup.c
> >>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
> >>>>
> >>>>    struct loongson_board_info b_info;
> >>>>    static const char dmi_empty_string[] = "        ";
> >>>> +static int possible_cpus;
> >>>> +static bool bsp_added;
> >>>>
> >>>>    /*
> >>>>     * Setup information
> >>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
> >>>>           *cmdline_p = boot_command_line;
> >>>>    }
> >>>>
> >>>> +int topo_get_cpu(int physid)
> >>>> +{
> >>>> +       int i;
> >>>> +
> >>>> +       for (i = 0; i < possible_cpus; i++)
> >>>> +               if (cpu_logical_map(i) == physid)
> >>>> +                       break;
> >>>> +
> >>>> +       if (i == possible_cpus)
> >>>> +               return -ENOENT;
> >>>> +
> >>>> +       return i;
> >>>> +}
> >>>> +
> >>>> +int topo_add_cpu(int physid)
> >>>> +{
> >>>> +       int cpu;
> >>>> +
> >>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> >>>> +               bsp_added = true;
> >>>> +               return 0;
> >>>> +       }
> >>>> +
> >>>> +       cpu = topo_get_cpu(physid);
> >>>> +       if (cpu >= 0) {
> >>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> >>>> +               return -EEXIST;
> >>>> +       }
> >>>> +
> >>>> +       if (possible_cpus >= nr_cpu_ids)
> >>>> +               return -ERANGE;
> >>>> +
> >>>> +       __cpu_logical_map[possible_cpus] = physid;
> >>>> +       cpu = possible_cpus++;
> >>>> +       return cpu;
> >>>> +}
> >>>> +
> >>>> +static void __init topo_init(void)
> >>>> +{
> >>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> >>>> +       possible_cpus++;
> >>>> +}
> >>>> +
> >>>>    void __init platform_init(void)
> >>>>    {
> >>>>           arch_reserve_vmcore();
> >>>>           arch_reserve_crashkernel();
> >>>> +       topo_init();
> >>>>
> >>>>    #ifdef CONFIG_ACPI
> >>>>           acpi_table_upgrade();
> >>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>>> index 9afc2d8b3414..a3f466b89179 100644
> >>>> --- a/arch/loongarch/kernel/smp.c
> >>>> +++ b/arch/loongarch/kernel/smp.c
> >>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
> >>>>                   if (cpuid >= nr_cpu_ids)
> >>>>                           continue;
> >>>>
> >>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>> -                       cpu = 0;
> >>>> -               else
> >>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>> +               cpu = topo_add_cpu(cpuid);
> >>>> +               if (cpu < 0)
> >>>> +                       continue;
> >>>>
> >>>>                   num_processors++;
> >>>>                   set_cpu_possible(cpu, true);
> >>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
> >>>>                   __cpu_number_map[cpuid] = cpu;
> >>>>                   __cpu_logical_map[cpu] = cpuid;
> >>>>
> >>>> -               early_numa_add_cpu(cpu, 0);
> >>>> +               early_numa_add_cpu(cpuid, 0);
> >>>>                   set_cpuid_to_node(cpuid, 0);
> >>>>           }
> >>>>
> >>>>
> >>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> >>>> --
> >>>> 2.39.3
> >>>>
> >>
> >>
>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 3 weeks, 6 days ago


On 2024/10/29 下午6:36, Huacai Chen wrote:
> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
>>
>> Hi Huacai,
>>
>> On 2024/10/22 上午9:31, Huacai Chen wrote:
>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
>>>>> Hi, Bibo,
>>>>>
>>>>> This version still doesn't touch the round-robin method, but it
>>>>> doesn't matter, I think I misunderstood something since V1...
>>>> I do not understand why round-robin method need be modified, SRAT may be
>>>> disabled with general function disable_srat(). Then round-robin method
>>>> is required.
>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
>>>
>>>>
>>>>>
>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
>>>>> < 0.
>>>>>
>>>>> If the above is correct, we don't need so complicated, because the
>>>>> correct and simplest way is:
>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
>>>>>
>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
>>>> kernel side.
>>> Yes, I don't mind, please use that simplest way.
>> There is another problem with the simple way. eiointc reports error when
>> cpu is online. The error message is:
>>     Loongson-64bit Processor probed (LA464 Core)
>>     CPU2 revision is: 0014c010 (Loongson-64bit)
>>     FPU2 revision is: 00000001
>>     eiointc: Error: invalid nodemap!
>>     CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
>>
>> The problem is that node_map of eiointc is problematic,
>>
>>
>> static int cpu_to_eio_node(int cpu)
>> {
>>           return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
>> }
>>
>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
>>                                  u64 node_map)
>> {
>>           int i;
>>
>>           node_map = node_map ? node_map : -1ULL;
>>           for_each_possible_cpu(i) {
>>                   if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
>>                           node_set(cpu_to_eio_node(i), priv->node_map);
>>            ...
>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
>> problematic.
> The error message seems from eiointc_router_init(), but it is a little
> strange. Physical hot-add should be before logical hot-add. So
> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
> set_processor_mask() to setup logical-physical mapping, so in
> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
> should work well.
cpu hot-adding works in run-time stage, however eiointc_router_init() 
runs early in booting stage. Setting cpu_logical_map() in runtime is too 
late for function eiointc_router_init(), since eiointc_router_init() is 
only called in early kernel boot stage.

Regards
Bibo Mao
> 
> Maybe in your case a whole node is hot-added? I don't think the
> eiointc design can work with this case...
> 
>>
>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
>> not enabled at beginning, it should not be set at hotplug runtime.
> This will cause the logical cpu number be not continuous after boot.
> Physical numbers have no requirement, but logical numbers should be
> continuous.
> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Bibo Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> On LoongArch system, there are two places to set cpu numa node. One
>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
>>>>>> in generic function early_numa_node_init(). The latter will overwrite
>>>>>> the numa node information.
>>>>>>
>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
>>>>>> numa node for hot-added cpu, and generic function
>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
>>>>>>
>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>>>>>> architectures logic cpu is allocated when parsing MADT table. When
>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>>>>>> all allocated logical cpu with matched physical id. It solves such
>>>>>> problems such as:
>>>>>>      1. Boot cpu is not the first entry in MADT table, the first entry
>>>>>> will be overwritten with later boot cpu.
>>>>>>      2. Physical cpu id not presented in MADT table is invalid, in later
>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>>>>>      3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>>>>>> is correct for hot-add cpu.
>>>>>>
>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>> ---
>>>>>> v1 ... v2:
>>>>>>      1. Like other architectures, allocate logic cpu when parsing MADT table.
>>>>>>      2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>>>>>> hot-add cpu DSDT information.
>>>>>> ---
>>>>>>     arch/loongarch/include/asm/smp.h |  3 ++
>>>>>>     arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>>>>>     arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>>>>>     arch/loongarch/kernel/smp.c      |  9 +++---
>>>>>>     4 files changed, 70 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>>>> index 3383c9d24e94..c61b75937a77 100644
>>>>>> --- a/arch/loongarch/include/asm/smp.h
>>>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>>>>>     #define cpu_logical_map(cpu)   0
>>>>>>     #endif /* CONFIG_SMP */
>>>>>>
>>>>>> +int topo_add_cpu(int physid);
>>>>>> +int topo_get_cpu(int physid);
>>>>>> +
>>>>>>     #endif /* __ASM_SMP_H */
>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>>>>>> index f1a74b80f22c..84d9812d5f38 100644
>>>>>> --- a/arch/loongarch/kernel/acpi.c
>>>>>> +++ b/arch/loongarch/kernel/acpi.c
>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>>>>>                    return -ENODEV;
>>>>>>
>>>>>>            }
>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>> -               cpu = 0;
>>>>>> -       else
>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>> +
>>>>>> +       cpu = topo_add_cpu(cpuid);
>>>>>> +       if (cpu < 0)
>>>>>> +               return -EEXIST;
>>>>>>
>>>>>>            if (!cpu_enumerated)
>>>>>>                    set_cpu_possible(cpu, true);
>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>>>>>                    goto fdt_earlycon;
>>>>>>            }
>>>>>>
>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>> -
>>>>>>            /*
>>>>>>             * Process the Multiple APIC Description Table (MADT), if present
>>>>>>             */
>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>>>>>     void __init
>>>>>>     acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>     {
>>>>>> -       int pxm, node;
>>>>>> +       int pxm, node, cpu;
>>>>>>
>>>>>>            if (srat_disabled())
>>>>>>                    return;
>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>                    return;
>>>>>>            }
>>>>>>
>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>> +       if (cpu < 0)
>>>>>> +               return;
>>>>>> +
>>>>>>            early_numa_add_cpu(pa->apic_id, node);
>>>>>>
>>>>>>            set_cpuid_to_node(pa->apic_id, node);
>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>>>>>     {
>>>>>>            int cpu;
>>>>>>
>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>            if (cpu < 0) {
>>>>>>                    pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>>>>>                    return cpu;
>>>>>>            }
>>>>>>
>>>>>> +       num_processors++;
>>>>>> +       set_cpu_present(cpu, true);
>>>>>> +       __cpu_number_map[physid] = cpu;
>>>>>> +       __cpu_logical_map[cpu] = physid;
>>>>>>            acpi_map_cpu2node(handle, cpu, physid);
>>>>>>
>>>>>>            *pcpu = cpu;
>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>>>> index 00e307203ddb..649e98640076 100644
>>>>>> --- a/arch/loongarch/kernel/setup.c
>>>>>> +++ b/arch/loongarch/kernel/setup.c
>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>>>>>
>>>>>>     struct loongson_board_info b_info;
>>>>>>     static const char dmi_empty_string[] = "        ";
>>>>>> +static int possible_cpus;
>>>>>> +static bool bsp_added;
>>>>>>
>>>>>>     /*
>>>>>>      * Setup information
>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>>>>>            *cmdline_p = boot_command_line;
>>>>>>     }
>>>>>>
>>>>>> +int topo_get_cpu(int physid)
>>>>>> +{
>>>>>> +       int i;
>>>>>> +
>>>>>> +       for (i = 0; i < possible_cpus; i++)
>>>>>> +               if (cpu_logical_map(i) == physid)
>>>>>> +                       break;
>>>>>> +
>>>>>> +       if (i == possible_cpus)
>>>>>> +               return -ENOENT;
>>>>>> +
>>>>>> +       return i;
>>>>>> +}
>>>>>> +
>>>>>> +int topo_add_cpu(int physid)
>>>>>> +{
>>>>>> +       int cpu;
>>>>>> +
>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>>>>>> +               bsp_added = true;
>>>>>> +               return 0;
>>>>>> +       }
>>>>>> +
>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>> +       if (cpu >= 0) {
>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>>>>>> +               return -EEXIST;
>>>>>> +       }
>>>>>> +
>>>>>> +       if (possible_cpus >= nr_cpu_ids)
>>>>>> +               return -ERANGE;
>>>>>> +
>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
>>>>>> +       cpu = possible_cpus++;
>>>>>> +       return cpu;
>>>>>> +}
>>>>>> +
>>>>>> +static void __init topo_init(void)
>>>>>> +{
>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>>>>>> +       possible_cpus++;
>>>>>> +}
>>>>>> +
>>>>>>     void __init platform_init(void)
>>>>>>     {
>>>>>>            arch_reserve_vmcore();
>>>>>>            arch_reserve_crashkernel();
>>>>>> +       topo_init();
>>>>>>
>>>>>>     #ifdef CONFIG_ACPI
>>>>>>            acpi_table_upgrade();
>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>>>> index 9afc2d8b3414..a3f466b89179 100644
>>>>>> --- a/arch/loongarch/kernel/smp.c
>>>>>> +++ b/arch/loongarch/kernel/smp.c
>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>>>>>                    if (cpuid >= nr_cpu_ids)
>>>>>>                            continue;
>>>>>>
>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>> -                       cpu = 0;
>>>>>> -               else
>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>> +               cpu = topo_add_cpu(cpuid);
>>>>>> +               if (cpu < 0)
>>>>>> +                       continue;
>>>>>>
>>>>>>                    num_processors++;
>>>>>>                    set_cpu_possible(cpu, true);
>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>>>>>                    __cpu_number_map[cpuid] = cpu;
>>>>>>                    __cpu_logical_map[cpu] = cpuid;
>>>>>>
>>>>>> -               early_numa_add_cpu(cpu, 0);
>>>>>> +               early_numa_add_cpu(cpuid, 0);
>>>>>>                    set_cpuid_to_node(cpuid, 0);
>>>>>>            }
>>>>>>
>>>>>>
>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>>>>>> --
>>>>>> 2.39.3
>>>>>>
>>>>
>>>>
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 3 weeks, 6 days ago


On 2024/10/29 下午6:36, Huacai Chen wrote:
> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
>>
>> Hi Huacai,
>>
>> On 2024/10/22 上午9:31, Huacai Chen wrote:
>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
>>>>> Hi, Bibo,
>>>>>
>>>>> This version still doesn't touch the round-robin method, but it
>>>>> doesn't matter, I think I misunderstood something since V1...
>>>> I do not understand why round-robin method need be modified, SRAT may be
>>>> disabled with general function disable_srat(). Then round-robin method
>>>> is required.
>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
>>>
>>>>
>>>>>
>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
>>>>> < 0.
>>>>>
>>>>> If the above is correct, we don't need so complicated, because the
>>>>> correct and simplest way is:
>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
>>>>>
>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
>>>> kernel side.
>>> Yes, I don't mind, please use that simplest way.
>> There is another problem with the simple way. eiointc reports error when
>> cpu is online. The error message is:
>>     Loongson-64bit Processor probed (LA464 Core)
>>     CPU2 revision is: 0014c010 (Loongson-64bit)
>>     FPU2 revision is: 00000001
>>     eiointc: Error: invalid nodemap!
>>     CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
>>
>> The problem is that node_map of eiointc is problematic,
>>
>>
>> static int cpu_to_eio_node(int cpu)
>> {
>>           return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
>> }
>>
>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
>>                                  u64 node_map)
>> {
>>           int i;
>>
>>           node_map = node_map ? node_map : -1ULL;
>>           for_each_possible_cpu(i) {
>>                   if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
>>                           node_set(cpu_to_eio_node(i), priv->node_map);
>>            ...
>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
>> problematic.
> The error message seems from eiointc_router_init(), but it is a little
> strange. Physical hot-add should be before logical hot-add. So
> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
> set_processor_mask() to setup logical-physical mapping, so in
> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
> should work well.
> 
> Maybe in your case a whole node is hot-added? I don't think the
> eiointc design can work with this case...
> 
>>
>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
>> not enabled at beginning, it should not be set at hotplug runtime.
> This will cause the logical cpu number be not continuous after boot.
> Physical numbers have no requirement, but logical numbers should be
> continuous.
I do not understand such requirement about logical cpu should be 
continuous. You can check logical cpu allocation method on other 
architectures, or what does the requirement about logical cpu continuous 
come from.

Regards
Bibo Mao

> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Bibo Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> On LoongArch system, there are two places to set cpu numa node. One
>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
>>>>>> in generic function early_numa_node_init(). The latter will overwrite
>>>>>> the numa node information.
>>>>>>
>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
>>>>>> numa node for hot-added cpu, and generic function
>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
>>>>>>
>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>>>>>> architectures logic cpu is allocated when parsing MADT table. When
>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>>>>>> all allocated logical cpu with matched physical id. It solves such
>>>>>> problems such as:
>>>>>>      1. Boot cpu is not the first entry in MADT table, the first entry
>>>>>> will be overwritten with later boot cpu.
>>>>>>      2. Physical cpu id not presented in MADT table is invalid, in later
>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>>>>>      3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>>>>>> is correct for hot-add cpu.
>>>>>>
>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>> ---
>>>>>> v1 ... v2:
>>>>>>      1. Like other architectures, allocate logic cpu when parsing MADT table.
>>>>>>      2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>>>>>> hot-add cpu DSDT information.
>>>>>> ---
>>>>>>     arch/loongarch/include/asm/smp.h |  3 ++
>>>>>>     arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>>>>>     arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>>>>>     arch/loongarch/kernel/smp.c      |  9 +++---
>>>>>>     4 files changed, 70 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>>>> index 3383c9d24e94..c61b75937a77 100644
>>>>>> --- a/arch/loongarch/include/asm/smp.h
>>>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>>>>>     #define cpu_logical_map(cpu)   0
>>>>>>     #endif /* CONFIG_SMP */
>>>>>>
>>>>>> +int topo_add_cpu(int physid);
>>>>>> +int topo_get_cpu(int physid);
>>>>>> +
>>>>>>     #endif /* __ASM_SMP_H */
>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>>>>>> index f1a74b80f22c..84d9812d5f38 100644
>>>>>> --- a/arch/loongarch/kernel/acpi.c
>>>>>> +++ b/arch/loongarch/kernel/acpi.c
>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>>>>>                    return -ENODEV;
>>>>>>
>>>>>>            }
>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>> -               cpu = 0;
>>>>>> -       else
>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>> +
>>>>>> +       cpu = topo_add_cpu(cpuid);
>>>>>> +       if (cpu < 0)
>>>>>> +               return -EEXIST;
>>>>>>
>>>>>>            if (!cpu_enumerated)
>>>>>>                    set_cpu_possible(cpu, true);
>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>>>>>                    goto fdt_earlycon;
>>>>>>            }
>>>>>>
>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>> -
>>>>>>            /*
>>>>>>             * Process the Multiple APIC Description Table (MADT), if present
>>>>>>             */
>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>>>>>     void __init
>>>>>>     acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>     {
>>>>>> -       int pxm, node;
>>>>>> +       int pxm, node, cpu;
>>>>>>
>>>>>>            if (srat_disabled())
>>>>>>                    return;
>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>                    return;
>>>>>>            }
>>>>>>
>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>> +       if (cpu < 0)
>>>>>> +               return;
>>>>>> +
>>>>>>            early_numa_add_cpu(pa->apic_id, node);
>>>>>>
>>>>>>            set_cpuid_to_node(pa->apic_id, node);
>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>>>>>     {
>>>>>>            int cpu;
>>>>>>
>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>            if (cpu < 0) {
>>>>>>                    pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>>>>>                    return cpu;
>>>>>>            }
>>>>>>
>>>>>> +       num_processors++;
>>>>>> +       set_cpu_present(cpu, true);
>>>>>> +       __cpu_number_map[physid] = cpu;
>>>>>> +       __cpu_logical_map[cpu] = physid;
>>>>>>            acpi_map_cpu2node(handle, cpu, physid);
>>>>>>
>>>>>>            *pcpu = cpu;
>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>>>> index 00e307203ddb..649e98640076 100644
>>>>>> --- a/arch/loongarch/kernel/setup.c
>>>>>> +++ b/arch/loongarch/kernel/setup.c
>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>>>>>
>>>>>>     struct loongson_board_info b_info;
>>>>>>     static const char dmi_empty_string[] = "        ";
>>>>>> +static int possible_cpus;
>>>>>> +static bool bsp_added;
>>>>>>
>>>>>>     /*
>>>>>>      * Setup information
>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>>>>>            *cmdline_p = boot_command_line;
>>>>>>     }
>>>>>>
>>>>>> +int topo_get_cpu(int physid)
>>>>>> +{
>>>>>> +       int i;
>>>>>> +
>>>>>> +       for (i = 0; i < possible_cpus; i++)
>>>>>> +               if (cpu_logical_map(i) == physid)
>>>>>> +                       break;
>>>>>> +
>>>>>> +       if (i == possible_cpus)
>>>>>> +               return -ENOENT;
>>>>>> +
>>>>>> +       return i;
>>>>>> +}
>>>>>> +
>>>>>> +int topo_add_cpu(int physid)
>>>>>> +{
>>>>>> +       int cpu;
>>>>>> +
>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>>>>>> +               bsp_added = true;
>>>>>> +               return 0;
>>>>>> +       }
>>>>>> +
>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>> +       if (cpu >= 0) {
>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>>>>>> +               return -EEXIST;
>>>>>> +       }
>>>>>> +
>>>>>> +       if (possible_cpus >= nr_cpu_ids)
>>>>>> +               return -ERANGE;
>>>>>> +
>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
>>>>>> +       cpu = possible_cpus++;
>>>>>> +       return cpu;
>>>>>> +}
>>>>>> +
>>>>>> +static void __init topo_init(void)
>>>>>> +{
>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>>>>>> +       possible_cpus++;
>>>>>> +}
>>>>>> +
>>>>>>     void __init platform_init(void)
>>>>>>     {
>>>>>>            arch_reserve_vmcore();
>>>>>>            arch_reserve_crashkernel();
>>>>>> +       topo_init();
>>>>>>
>>>>>>     #ifdef CONFIG_ACPI
>>>>>>            acpi_table_upgrade();
>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>>>> index 9afc2d8b3414..a3f466b89179 100644
>>>>>> --- a/arch/loongarch/kernel/smp.c
>>>>>> +++ b/arch/loongarch/kernel/smp.c
>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>>>>>                    if (cpuid >= nr_cpu_ids)
>>>>>>                            continue;
>>>>>>
>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>> -                       cpu = 0;
>>>>>> -               else
>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>> +               cpu = topo_add_cpu(cpuid);
>>>>>> +               if (cpu < 0)
>>>>>> +                       continue;
>>>>>>
>>>>>>                    num_processors++;
>>>>>>                    set_cpu_possible(cpu, true);
>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>>>>>                    __cpu_number_map[cpuid] = cpu;
>>>>>>                    __cpu_logical_map[cpu] = cpuid;
>>>>>>
>>>>>> -               early_numa_add_cpu(cpu, 0);
>>>>>> +               early_numa_add_cpu(cpuid, 0);
>>>>>>                    set_cpuid_to_node(cpuid, 0);
>>>>>>            }
>>>>>>
>>>>>>
>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>>>>>> --
>>>>>> 2.39.3
>>>>>>
>>>>
>>>>
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 3 weeks, 5 days ago

On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/10/29 下午6:36, Huacai Chen wrote:
> > On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >> Hi Huacai,
> >>
> >> On 2024/10/22 上午9:31, Huacai Chen wrote:
> >>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
> >>>>> Hi, Bibo,
> >>>>>
> >>>>> This version still doesn't touch the round-robin method, but it
> >>>>> doesn't matter, I think I misunderstood something since V1...
> >>>> I do not understand why round-robin method need be modified, SRAT may be
> >>>> disabled with general function disable_srat(). Then round-robin method
> >>>> is required.
> >>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
> >>>
> >>>>
> >>>>>
> >>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> >>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
> >>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> >>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
> >>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> >>>>> < 0.
> >>>>>
> >>>>> If the above is correct, we don't need so complicated, because the
> >>>>> correct and simplest way is:
> >>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> >>>>>
> >>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
> >>>> badly. If you do not mind, I do not mind neither. It is not my duty for
> >>>> kernel side.
> >>> Yes, I don't mind, please use that simplest way.
> >> There is another problem with the simple way. eiointc reports error when
> >> cpu is online. The error message is:
> >>     Loongson-64bit Processor probed (LA464 Core)
> >>     CPU2 revision is: 0014c010 (Loongson-64bit)
> >>     FPU2 revision is: 00000001
> >>     eiointc: Error: invalid nodemap!
> >>     CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
> >>
> >> The problem is that node_map of eiointc is problematic,
> >>
> >>
> >> static int cpu_to_eio_node(int cpu)
> >> {
> >>           return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
> >> }
> >>
> >> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
> >>                                  u64 node_map)
> >> {
> >>           int i;
> >>
> >>           node_map = node_map ? node_map : -1ULL;
> >>           for_each_possible_cpu(i) {
> >>                   if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
> >>                           node_set(cpu_to_eio_node(i), priv->node_map);
> >>            ...
> >> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
> >> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
> >> problematic.
> > The error message seems from eiointc_router_init(), but it is a little
> > strange. Physical hot-add should be before logical hot-add. So
> > acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
> > set_processor_mask() to setup logical-physical mapping, so in
> > eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
> > should work well.
> >
> > Maybe in your case a whole node is hot-added? I don't think the
> > eiointc design can work with this case...
> >
> >>
> >> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
> >> not enabled at beginning, it should not be set at hotplug runtime.
> > This will cause the logical cpu number be not continuous after boot.
> > Physical numbers have no requirement, but logical numbers should be
> > continuous.
> I do not understand such requirement about logical cpu should be
> continuous. You can check logical cpu allocation method on other
> architectures, or what does the requirement about logical cpu continuous
> come from.
1, In an internal conference, it is said that non-continuous cpu
numbers make users think our processors have bugs.
2, See prefill_possible_map(), it assumes logical numbers continuous
cpu_possible_mask and cpu_present_mask, which make it convenient for
"nr_cpus=xxx".
3, Can you show me an example in a real machine that "processor" in
/proc/cpuinfo non-continues after boot and before soft hotplug?




Huacai
>
> Regards
> Bibo Mao
>
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Bibo Mao
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>> On LoongArch system, there are two places to set cpu numa node. One
> >>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
> >>>>>> in generic function early_numa_node_init(). The latter will overwrite
> >>>>>> the numa node information.
> >>>>>>
> >>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
> >>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
> >>>>>> MADT table. So function early_cpu_to_node() also fails to get its
> >>>>>> numa node for hot-added cpu, and generic function
> >>>>>> early_numa_node_init() will overwrite with incorrect numa node.
> >>>>>>
> >>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> >>>>>> architectures logic cpu is allocated when parsing MADT table. When
> >>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> >>>>>> all allocated logical cpu with matched physical id. It solves such
> >>>>>> problems such as:
> >>>>>>      1. Boot cpu is not the first entry in MADT table, the first entry
> >>>>>> will be overwritten with later boot cpu.
> >>>>>>      2. Physical cpu id not presented in MADT table is invalid, in later
> >>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
> >>>>>>      3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> >>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> >>>>>> is correct for hot-add cpu.
> >>>>>>
> >>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>> ---
> >>>>>> v1 ... v2:
> >>>>>>      1. Like other architectures, allocate logic cpu when parsing MADT table.
> >>>>>>      2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> >>>>>> hot-add cpu DSDT information.
> >>>>>> ---
> >>>>>>     arch/loongarch/include/asm/smp.h |  3 ++
> >>>>>>     arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
> >>>>>>     arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
> >>>>>>     arch/loongarch/kernel/smp.c      |  9 +++---
> >>>>>>     4 files changed, 70 insertions(+), 13 deletions(-)
> >>>>>>
> >>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >>>>>> index 3383c9d24e94..c61b75937a77 100644
> >>>>>> --- a/arch/loongarch/include/asm/smp.h
> >>>>>> +++ b/arch/loongarch/include/asm/smp.h
> >>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
> >>>>>>     #define cpu_logical_map(cpu)   0
> >>>>>>     #endif /* CONFIG_SMP */
> >>>>>>
> >>>>>> +int topo_add_cpu(int physid);
> >>>>>> +int topo_get_cpu(int physid);
> >>>>>> +
> >>>>>>     #endif /* __ASM_SMP_H */
> >>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> >>>>>> index f1a74b80f22c..84d9812d5f38 100644
> >>>>>> --- a/arch/loongarch/kernel/acpi.c
> >>>>>> +++ b/arch/loongarch/kernel/acpi.c
> >>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
> >>>>>>                    return -ENODEV;
> >>>>>>
> >>>>>>            }
> >>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>> -               cpu = 0;
> >>>>>> -       else
> >>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>> +
> >>>>>> +       cpu = topo_add_cpu(cpuid);
> >>>>>> +       if (cpu < 0)
> >>>>>> +               return -EEXIST;
> >>>>>>
> >>>>>>            if (!cpu_enumerated)
> >>>>>>                    set_cpu_possible(cpu, true);
> >>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
> >>>>>>                    goto fdt_earlycon;
> >>>>>>            }
> >>>>>>
> >>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>> -
> >>>>>>            /*
> >>>>>>             * Process the Multiple APIC Description Table (MADT), if present
> >>>>>>             */
> >>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
> >>>>>>     void __init
> >>>>>>     acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>     {
> >>>>>> -       int pxm, node;
> >>>>>> +       int pxm, node, cpu;
> >>>>>>
> >>>>>>            if (srat_disabled())
> >>>>>>                    return;
> >>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>                    return;
> >>>>>>            }
> >>>>>>
> >>>>>> +       cpu = topo_get_cpu(pa->apic_id);
> >>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>> +       if (cpu < 0)
> >>>>>> +               return;
> >>>>>> +
> >>>>>>            early_numa_add_cpu(pa->apic_id, node);
> >>>>>>
> >>>>>>            set_cpuid_to_node(pa->apic_id, node);
> >>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
> >>>>>>     {
> >>>>>>            int cpu;
> >>>>>>
> >>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> >>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>            if (cpu < 0) {
> >>>>>>                    pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
> >>>>>>                    return cpu;
> >>>>>>            }
> >>>>>>
> >>>>>> +       num_processors++;
> >>>>>> +       set_cpu_present(cpu, true);
> >>>>>> +       __cpu_number_map[physid] = cpu;
> >>>>>> +       __cpu_logical_map[cpu] = physid;
> >>>>>>            acpi_map_cpu2node(handle, cpu, physid);
> >>>>>>
> >>>>>>            *pcpu = cpu;
> >>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> >>>>>> index 00e307203ddb..649e98640076 100644
> >>>>>> --- a/arch/loongarch/kernel/setup.c
> >>>>>> +++ b/arch/loongarch/kernel/setup.c
> >>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
> >>>>>>
> >>>>>>     struct loongson_board_info b_info;
> >>>>>>     static const char dmi_empty_string[] = "        ";
> >>>>>> +static int possible_cpus;
> >>>>>> +static bool bsp_added;
> >>>>>>
> >>>>>>     /*
> >>>>>>      * Setup information
> >>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
> >>>>>>            *cmdline_p = boot_command_line;
> >>>>>>     }
> >>>>>>
> >>>>>> +int topo_get_cpu(int physid)
> >>>>>> +{
> >>>>>> +       int i;
> >>>>>> +
> >>>>>> +       for (i = 0; i < possible_cpus; i++)
> >>>>>> +               if (cpu_logical_map(i) == physid)
> >>>>>> +                       break;
> >>>>>> +
> >>>>>> +       if (i == possible_cpus)
> >>>>>> +               return -ENOENT;
> >>>>>> +
> >>>>>> +       return i;
> >>>>>> +}
> >>>>>> +
> >>>>>> +int topo_add_cpu(int physid)
> >>>>>> +{
> >>>>>> +       int cpu;
> >>>>>> +
> >>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> >>>>>> +               bsp_added = true;
> >>>>>> +               return 0;
> >>>>>> +       }
> >>>>>> +
> >>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>> +       if (cpu >= 0) {
> >>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> >>>>>> +               return -EEXIST;
> >>>>>> +       }
> >>>>>> +
> >>>>>> +       if (possible_cpus >= nr_cpu_ids)
> >>>>>> +               return -ERANGE;
> >>>>>> +
> >>>>>> +       __cpu_logical_map[possible_cpus] = physid;
> >>>>>> +       cpu = possible_cpus++;
> >>>>>> +       return cpu;
> >>>>>> +}
> >>>>>> +
> >>>>>> +static void __init topo_init(void)
> >>>>>> +{
> >>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> >>>>>> +       possible_cpus++;
> >>>>>> +}
> >>>>>> +
> >>>>>>     void __init platform_init(void)
> >>>>>>     {
> >>>>>>            arch_reserve_vmcore();
> >>>>>>            arch_reserve_crashkernel();
> >>>>>> +       topo_init();
> >>>>>>
> >>>>>>     #ifdef CONFIG_ACPI
> >>>>>>            acpi_table_upgrade();
> >>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>>>>> index 9afc2d8b3414..a3f466b89179 100644
> >>>>>> --- a/arch/loongarch/kernel/smp.c
> >>>>>> +++ b/arch/loongarch/kernel/smp.c
> >>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
> >>>>>>                    if (cpuid >= nr_cpu_ids)
> >>>>>>                            continue;
> >>>>>>
> >>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>> -                       cpu = 0;
> >>>>>> -               else
> >>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>> +               cpu = topo_add_cpu(cpuid);
> >>>>>> +               if (cpu < 0)
> >>>>>> +                       continue;
> >>>>>>
> >>>>>>                    num_processors++;
> >>>>>>                    set_cpu_possible(cpu, true);
> >>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
> >>>>>>                    __cpu_number_map[cpuid] = cpu;
> >>>>>>                    __cpu_logical_map[cpu] = cpuid;
> >>>>>>
> >>>>>> -               early_numa_add_cpu(cpu, 0);
> >>>>>> +               early_numa_add_cpu(cpuid, 0);
> >>>>>>                    set_cpuid_to_node(cpuid, 0);
> >>>>>>            }
> >>>>>>
> >>>>>>
> >>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> >>>>>> --
> >>>>>> 2.39.3
> >>>>>>
> >>>>
> >>>>
> >>
>
>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 3 weeks, 5 days ago


On 2024/10/30 下午4:12, Huacai Chen wrote:
> On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/10/29 下午6:36, Huacai Chen wrote:
>>> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>> Hi Huacai,
>>>>
>>>> On 2024/10/22 上午9:31, Huacai Chen wrote:
>>>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
>>>>>>> Hi, Bibo,
>>>>>>>
>>>>>>> This version still doesn't touch the round-robin method, but it
>>>>>>> doesn't matter, I think I misunderstood something since V1...
>>>>>> I do not understand why round-robin method need be modified, SRAT may be
>>>>>> disabled with general function disable_srat(). Then round-robin method
>>>>>> is required.
>>>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
>>>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
>>>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
>>>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
>>>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
>>>>>>> < 0.
>>>>>>>
>>>>>>> If the above is correct, we don't need so complicated, because the
>>>>>>> correct and simplest way is:
>>>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
>>>>>>>
>>>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
>>>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
>>>>>> kernel side.
>>>>> Yes, I don't mind, please use that simplest way.
>>>> There is another problem with the simple way. eiointc reports error when
>>>> cpu is online. The error message is:
>>>>      Loongson-64bit Processor probed (LA464 Core)
>>>>      CPU2 revision is: 0014c010 (Loongson-64bit)
>>>>      FPU2 revision is: 00000001
>>>>      eiointc: Error: invalid nodemap!
>>>>      CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
>>>>
>>>> The problem is that node_map of eiointc is problematic,
>>>>
>>>>
>>>> static int cpu_to_eio_node(int cpu)
>>>> {
>>>>            return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
>>>> }
>>>>
>>>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
>>>>                                   u64 node_map)
>>>> {
>>>>            int i;
>>>>
>>>>            node_map = node_map ? node_map : -1ULL;
>>>>            for_each_possible_cpu(i) {
>>>>                    if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
>>>>                            node_set(cpu_to_eio_node(i), priv->node_map);
>>>>             ...
>>>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
>>>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
>>>> problematic.
>>> The error message seems from eiointc_router_init(), but it is a little
>>> strange. Physical hot-add should be before logical hot-add. So
>>> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
>>> set_processor_mask() to setup logical-physical mapping, so in
>>> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
>>> should work well.
>>>
>>> Maybe in your case a whole node is hot-added? I don't think the
>>> eiointc design can work with this case...
>>>
>>>>
>>>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
>>>> not enabled at beginning, it should not be set at hotplug runtime.
>>> This will cause the logical cpu number be not continuous after boot.
>>> Physical numbers have no requirement, but logical numbers should be
>>> continuous.
>> I do not understand such requirement about logical cpu should be
>> continuous. You can check logical cpu allocation method on other
>> architectures, or what does the requirement about logical cpu continuous
>> come from.
> 1, In an internal conference, it is said that non-continuous cpu
> numbers make users think our processors have bugs.
> 2, See prefill_possible_map(), it assumes logical numbers continuous
> cpu_possible_mask and cpu_present_mask, which make it convenient for
> "nr_cpus=xxx".
> 3, Can you show me an example in a real machine that "processor" in
> /proc/cpuinfo non-continues after boot and before soft hotplug?
It is really wasting my time to discuss with you. You does not 
investigating implementation of other architectures, fully thinking in 
yourself way.

Does the real machines support real cpu hotplug and memory hotplug?

Regards
Bibo Mao
> 
> 
> 
> 
> Huacai
>>
>> Regards
>> Bibo Mao
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Bibo Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>> On LoongArch system, there are two places to set cpu numa node. One
>>>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
>>>>>>>> in generic function early_numa_node_init(). The latter will overwrite
>>>>>>>> the numa node information.
>>>>>>>>
>>>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
>>>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
>>>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
>>>>>>>> numa node for hot-added cpu, and generic function
>>>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
>>>>>>>>
>>>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>>>>>>>> architectures logic cpu is allocated when parsing MADT table. When
>>>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>>>>>>>> all allocated logical cpu with matched physical id. It solves such
>>>>>>>> problems such as:
>>>>>>>>       1. Boot cpu is not the first entry in MADT table, the first entry
>>>>>>>> will be overwritten with later boot cpu.
>>>>>>>>       2. Physical cpu id not presented in MADT table is invalid, in later
>>>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>>>>>>>       3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>>>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>>>>>>>> is correct for hot-add cpu.
>>>>>>>>
>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>> ---
>>>>>>>> v1 ... v2:
>>>>>>>>       1. Like other architectures, allocate logic cpu when parsing MADT table.
>>>>>>>>       2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>>>>>>>> hot-add cpu DSDT information.
>>>>>>>> ---
>>>>>>>>      arch/loongarch/include/asm/smp.h |  3 ++
>>>>>>>>      arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>>>>>>>      arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>>>>>>>      arch/loongarch/kernel/smp.c      |  9 +++---
>>>>>>>>      4 files changed, 70 insertions(+), 13 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>>>>>> index 3383c9d24e94..c61b75937a77 100644
>>>>>>>> --- a/arch/loongarch/include/asm/smp.h
>>>>>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>>>>>>>      #define cpu_logical_map(cpu)   0
>>>>>>>>      #endif /* CONFIG_SMP */
>>>>>>>>
>>>>>>>> +int topo_add_cpu(int physid);
>>>>>>>> +int topo_get_cpu(int physid);
>>>>>>>> +
>>>>>>>>      #endif /* __ASM_SMP_H */
>>>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>>>>>>>> index f1a74b80f22c..84d9812d5f38 100644
>>>>>>>> --- a/arch/loongarch/kernel/acpi.c
>>>>>>>> +++ b/arch/loongarch/kernel/acpi.c
>>>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>>>>>>>                     return -ENODEV;
>>>>>>>>
>>>>>>>>             }
>>>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>>>> -               cpu = 0;
>>>>>>>> -       else
>>>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>>>> +
>>>>>>>> +       cpu = topo_add_cpu(cpuid);
>>>>>>>> +       if (cpu < 0)
>>>>>>>> +               return -EEXIST;
>>>>>>>>
>>>>>>>>             if (!cpu_enumerated)
>>>>>>>>                     set_cpu_possible(cpu, true);
>>>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>>>>>>>                     goto fdt_earlycon;
>>>>>>>>             }
>>>>>>>>
>>>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>>>> -
>>>>>>>>             /*
>>>>>>>>              * Process the Multiple APIC Description Table (MADT), if present
>>>>>>>>              */
>>>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>>>>>>>      void __init
>>>>>>>>      acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>>>      {
>>>>>>>> -       int pxm, node;
>>>>>>>> +       int pxm, node, cpu;
>>>>>>>>
>>>>>>>>             if (srat_disabled())
>>>>>>>>                     return;
>>>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>>>                     return;
>>>>>>>>             }
>>>>>>>>
>>>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
>>>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>>> +       if (cpu < 0)
>>>>>>>> +               return;
>>>>>>>> +
>>>>>>>>             early_numa_add_cpu(pa->apic_id, node);
>>>>>>>>
>>>>>>>>             set_cpuid_to_node(pa->apic_id, node);
>>>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>>>>>>>      {
>>>>>>>>             int cpu;
>>>>>>>>
>>>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>>>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>>>             if (cpu < 0) {
>>>>>>>>                     pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>>>>>>>                     return cpu;
>>>>>>>>             }
>>>>>>>>
>>>>>>>> +       num_processors++;
>>>>>>>> +       set_cpu_present(cpu, true);
>>>>>>>> +       __cpu_number_map[physid] = cpu;
>>>>>>>> +       __cpu_logical_map[cpu] = physid;
>>>>>>>>             acpi_map_cpu2node(handle, cpu, physid);
>>>>>>>>
>>>>>>>>             *pcpu = cpu;
>>>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>>>>>> index 00e307203ddb..649e98640076 100644
>>>>>>>> --- a/arch/loongarch/kernel/setup.c
>>>>>>>> +++ b/arch/loongarch/kernel/setup.c
>>>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>>>>>>>
>>>>>>>>      struct loongson_board_info b_info;
>>>>>>>>      static const char dmi_empty_string[] = "        ";
>>>>>>>> +static int possible_cpus;
>>>>>>>> +static bool bsp_added;
>>>>>>>>
>>>>>>>>      /*
>>>>>>>>       * Setup information
>>>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>>>>>>>             *cmdline_p = boot_command_line;
>>>>>>>>      }
>>>>>>>>
>>>>>>>> +int topo_get_cpu(int physid)
>>>>>>>> +{
>>>>>>>> +       int i;
>>>>>>>> +
>>>>>>>> +       for (i = 0; i < possible_cpus; i++)
>>>>>>>> +               if (cpu_logical_map(i) == physid)
>>>>>>>> +                       break;
>>>>>>>> +
>>>>>>>> +       if (i == possible_cpus)
>>>>>>>> +               return -ENOENT;
>>>>>>>> +
>>>>>>>> +       return i;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +int topo_add_cpu(int physid)
>>>>>>>> +{
>>>>>>>> +       int cpu;
>>>>>>>> +
>>>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>>>>>>>> +               bsp_added = true;
>>>>>>>> +               return 0;
>>>>>>>> +       }
>>>>>>>> +
>>>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>>>> +       if (cpu >= 0) {
>>>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>>>>>>>> +               return -EEXIST;
>>>>>>>> +       }
>>>>>>>> +
>>>>>>>> +       if (possible_cpus >= nr_cpu_ids)
>>>>>>>> +               return -ERANGE;
>>>>>>>> +
>>>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
>>>>>>>> +       cpu = possible_cpus++;
>>>>>>>> +       return cpu;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void __init topo_init(void)
>>>>>>>> +{
>>>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>>>>>>>> +       possible_cpus++;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>      void __init platform_init(void)
>>>>>>>>      {
>>>>>>>>             arch_reserve_vmcore();
>>>>>>>>             arch_reserve_crashkernel();
>>>>>>>> +       topo_init();
>>>>>>>>
>>>>>>>>      #ifdef CONFIG_ACPI
>>>>>>>>             acpi_table_upgrade();
>>>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>>>>>> index 9afc2d8b3414..a3f466b89179 100644
>>>>>>>> --- a/arch/loongarch/kernel/smp.c
>>>>>>>> +++ b/arch/loongarch/kernel/smp.c
>>>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>>>>>>>                     if (cpuid >= nr_cpu_ids)
>>>>>>>>                             continue;
>>>>>>>>
>>>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>>>> -                       cpu = 0;
>>>>>>>> -               else
>>>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>>>> +               cpu = topo_add_cpu(cpuid);
>>>>>>>> +               if (cpu < 0)
>>>>>>>> +                       continue;
>>>>>>>>
>>>>>>>>                     num_processors++;
>>>>>>>>                     set_cpu_possible(cpu, true);
>>>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>>>>>>>                     __cpu_number_map[cpuid] = cpu;
>>>>>>>>                     __cpu_logical_map[cpu] = cpuid;
>>>>>>>>
>>>>>>>> -               early_numa_add_cpu(cpu, 0);
>>>>>>>> +               early_numa_add_cpu(cpuid, 0);
>>>>>>>>                     set_cpuid_to_node(cpuid, 0);
>>>>>>>>             }
>>>>>>>>
>>>>>>>>
>>>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>>>>>>>> --
>>>>>>>> 2.39.3
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 3 weeks, 5 days ago

On Wed, Oct 30, 2024 at 4:25 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/10/30 下午4:12, Huacai Chen wrote:
> > On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/10/29 下午6:36, Huacai Chen wrote:
> >>> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>> Hi Huacai,
> >>>>
> >>>> On 2024/10/22 上午9:31, Huacai Chen wrote:
> >>>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
> >>>>>>> Hi, Bibo,
> >>>>>>>
> >>>>>>> This version still doesn't touch the round-robin method, but it
> >>>>>>> doesn't matter, I think I misunderstood something since V1...
> >>>>>> I do not understand why round-robin method need be modified, SRAT may be
> >>>>>> disabled with general function disable_srat(). Then round-robin method
> >>>>>> is required.
> >>>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> >>>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
> >>>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> >>>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
> >>>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> >>>>>>> < 0.
> >>>>>>>
> >>>>>>> If the above is correct, we don't need so complicated, because the
> >>>>>>> correct and simplest way is:
> >>>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> >>>>>>>
> >>>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
> >>>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
> >>>>>> kernel side.
> >>>>> Yes, I don't mind, please use that simplest way.
> >>>> There is another problem with the simple way. eiointc reports error when
> >>>> cpu is online. The error message is:
> >>>>      Loongson-64bit Processor probed (LA464 Core)
> >>>>      CPU2 revision is: 0014c010 (Loongson-64bit)
> >>>>      FPU2 revision is: 00000001
> >>>>      eiointc: Error: invalid nodemap!
> >>>>      CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
> >>>>
> >>>> The problem is that node_map of eiointc is problematic,
> >>>>
> >>>>
> >>>> static int cpu_to_eio_node(int cpu)
> >>>> {
> >>>>            return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
> >>>> }
> >>>>
> >>>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
> >>>>                                   u64 node_map)
> >>>> {
> >>>>            int i;
> >>>>
> >>>>            node_map = node_map ? node_map : -1ULL;
> >>>>            for_each_possible_cpu(i) {
> >>>>                    if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
> >>>>                            node_set(cpu_to_eio_node(i), priv->node_map);
> >>>>             ...
> >>>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
> >>>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
> >>>> problematic.
> >>> The error message seems from eiointc_router_init(), but it is a little
> >>> strange. Physical hot-add should be before logical hot-add. So
> >>> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
> >>> set_processor_mask() to setup logical-physical mapping, so in
> >>> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
> >>> should work well.
> >>>
> >>> Maybe in your case a whole node is hot-added? I don't think the
> >>> eiointc design can work with this case...
> >>>
> >>>>
> >>>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
> >>>> not enabled at beginning, it should not be set at hotplug runtime.
> >>> This will cause the logical cpu number be not continuous after boot.
> >>> Physical numbers have no requirement, but logical numbers should be
> >>> continuous.
> >> I do not understand such requirement about logical cpu should be
> >> continuous. You can check logical cpu allocation method on other
> >> architectures, or what does the requirement about logical cpu continuous
> >> come from.
> > 1, In an internal conference, it is said that non-continuous cpu
> > numbers make users think our processors have bugs.
> > 2, See prefill_possible_map(), it assumes logical numbers continuous
> > cpu_possible_mask and cpu_present_mask, which make it convenient for
> > "nr_cpus=xxx".
> > 3, Can you show me an example in a real machine that "processor" in
> > /proc/cpuinfo non-continues after boot and before soft hotplug?
> It is really wasting my time to discuss with you. You does not
> investigating implementation of other architectures, fully thinking in
> yourself way.
Totally wrong, I have implemented what you need, but you should make
other colleagues (not me) agree with your idea.
https://github.com/chenhuacai/linux/commit/d8dcf2844d5878b3ac5a42d074e781fe2ebfbae7

Imagine that the cpu_possible_mask is 0b11111111, cpu_present_mask is
0b10101010 (non-continuous), how to make "nr_cpus=3" work in a simple
way?

>
> Does the real machines support real cpu hotplug and memory hotplug?
ACPI_MADT_ENABLED is designed for virtual machines only?

Huacai

>
> Regards
> Bibo Mao
> >
> >
> >
> >
> > Huacai
> >>
> >> Regards
> >> Bibo Mao
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>
> >>>>
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>> Bibo Mao
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>> On LoongArch system, there are two places to set cpu numa node. One
> >>>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
> >>>>>>>> in generic function early_numa_node_init(). The latter will overwrite
> >>>>>>>> the numa node information.
> >>>>>>>>
> >>>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
> >>>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
> >>>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
> >>>>>>>> numa node for hot-added cpu, and generic function
> >>>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
> >>>>>>>>
> >>>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> >>>>>>>> architectures logic cpu is allocated when parsing MADT table. When
> >>>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> >>>>>>>> all allocated logical cpu with matched physical id. It solves such
> >>>>>>>> problems such as:
> >>>>>>>>       1. Boot cpu is not the first entry in MADT table, the first entry
> >>>>>>>> will be overwritten with later boot cpu.
> >>>>>>>>       2. Physical cpu id not presented in MADT table is invalid, in later
> >>>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
> >>>>>>>>       3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> >>>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> >>>>>>>> is correct for hot-add cpu.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>> ---
> >>>>>>>> v1 ... v2:
> >>>>>>>>       1. Like other architectures, allocate logic cpu when parsing MADT table.
> >>>>>>>>       2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> >>>>>>>> hot-add cpu DSDT information.
> >>>>>>>> ---
> >>>>>>>>      arch/loongarch/include/asm/smp.h |  3 ++
> >>>>>>>>      arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
> >>>>>>>>      arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
> >>>>>>>>      arch/loongarch/kernel/smp.c      |  9 +++---
> >>>>>>>>      4 files changed, 70 insertions(+), 13 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >>>>>>>> index 3383c9d24e94..c61b75937a77 100644
> >>>>>>>> --- a/arch/loongarch/include/asm/smp.h
> >>>>>>>> +++ b/arch/loongarch/include/asm/smp.h
> >>>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
> >>>>>>>>      #define cpu_logical_map(cpu)   0
> >>>>>>>>      #endif /* CONFIG_SMP */
> >>>>>>>>
> >>>>>>>> +int topo_add_cpu(int physid);
> >>>>>>>> +int topo_get_cpu(int physid);
> >>>>>>>> +
> >>>>>>>>      #endif /* __ASM_SMP_H */
> >>>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> >>>>>>>> index f1a74b80f22c..84d9812d5f38 100644
> >>>>>>>> --- a/arch/loongarch/kernel/acpi.c
> >>>>>>>> +++ b/arch/loongarch/kernel/acpi.c
> >>>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
> >>>>>>>>                     return -ENODEV;
> >>>>>>>>
> >>>>>>>>             }
> >>>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>>>> -               cpu = 0;
> >>>>>>>> -       else
> >>>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>>>> +
> >>>>>>>> +       cpu = topo_add_cpu(cpuid);
> >>>>>>>> +       if (cpu < 0)
> >>>>>>>> +               return -EEXIST;
> >>>>>>>>
> >>>>>>>>             if (!cpu_enumerated)
> >>>>>>>>                     set_cpu_possible(cpu, true);
> >>>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
> >>>>>>>>                     goto fdt_earlycon;
> >>>>>>>>             }
> >>>>>>>>
> >>>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>>>> -
> >>>>>>>>             /*
> >>>>>>>>              * Process the Multiple APIC Description Table (MADT), if present
> >>>>>>>>              */
> >>>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
> >>>>>>>>      void __init
> >>>>>>>>      acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>>>      {
> >>>>>>>> -       int pxm, node;
> >>>>>>>> +       int pxm, node, cpu;
> >>>>>>>>
> >>>>>>>>             if (srat_disabled())
> >>>>>>>>                     return;
> >>>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>>>                     return;
> >>>>>>>>             }
> >>>>>>>>
> >>>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
> >>>>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>>> +       if (cpu < 0)
> >>>>>>>> +               return;
> >>>>>>>> +
> >>>>>>>>             early_numa_add_cpu(pa->apic_id, node);
> >>>>>>>>
> >>>>>>>>             set_cpuid_to_node(pa->apic_id, node);
> >>>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
> >>>>>>>>      {
> >>>>>>>>             int cpu;
> >>>>>>>>
> >>>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> >>>>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>>>             if (cpu < 0) {
> >>>>>>>>                     pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
> >>>>>>>>                     return cpu;
> >>>>>>>>             }
> >>>>>>>>
> >>>>>>>> +       num_processors++;
> >>>>>>>> +       set_cpu_present(cpu, true);
> >>>>>>>> +       __cpu_number_map[physid] = cpu;
> >>>>>>>> +       __cpu_logical_map[cpu] = physid;
> >>>>>>>>             acpi_map_cpu2node(handle, cpu, physid);
> >>>>>>>>
> >>>>>>>>             *pcpu = cpu;
> >>>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> >>>>>>>> index 00e307203ddb..649e98640076 100644
> >>>>>>>> --- a/arch/loongarch/kernel/setup.c
> >>>>>>>> +++ b/arch/loongarch/kernel/setup.c
> >>>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
> >>>>>>>>
> >>>>>>>>      struct loongson_board_info b_info;
> >>>>>>>>      static const char dmi_empty_string[] = "        ";
> >>>>>>>> +static int possible_cpus;
> >>>>>>>> +static bool bsp_added;
> >>>>>>>>
> >>>>>>>>      /*
> >>>>>>>>       * Setup information
> >>>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
> >>>>>>>>             *cmdline_p = boot_command_line;
> >>>>>>>>      }
> >>>>>>>>
> >>>>>>>> +int topo_get_cpu(int physid)
> >>>>>>>> +{
> >>>>>>>> +       int i;
> >>>>>>>> +
> >>>>>>>> +       for (i = 0; i < possible_cpus; i++)
> >>>>>>>> +               if (cpu_logical_map(i) == physid)
> >>>>>>>> +                       break;
> >>>>>>>> +
> >>>>>>>> +       if (i == possible_cpus)
> >>>>>>>> +               return -ENOENT;
> >>>>>>>> +
> >>>>>>>> +       return i;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +int topo_add_cpu(int physid)
> >>>>>>>> +{
> >>>>>>>> +       int cpu;
> >>>>>>>> +
> >>>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> >>>>>>>> +               bsp_added = true;
> >>>>>>>> +               return 0;
> >>>>>>>> +       }
> >>>>>>>> +
> >>>>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>>>> +       if (cpu >= 0) {
> >>>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> >>>>>>>> +               return -EEXIST;
> >>>>>>>> +       }
> >>>>>>>> +
> >>>>>>>> +       if (possible_cpus >= nr_cpu_ids)
> >>>>>>>> +               return -ERANGE;
> >>>>>>>> +
> >>>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
> >>>>>>>> +       cpu = possible_cpus++;
> >>>>>>>> +       return cpu;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static void __init topo_init(void)
> >>>>>>>> +{
> >>>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> >>>>>>>> +       possible_cpus++;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>>      void __init platform_init(void)
> >>>>>>>>      {
> >>>>>>>>             arch_reserve_vmcore();
> >>>>>>>>             arch_reserve_crashkernel();
> >>>>>>>> +       topo_init();
> >>>>>>>>
> >>>>>>>>      #ifdef CONFIG_ACPI
> >>>>>>>>             acpi_table_upgrade();
> >>>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>>>>>>> index 9afc2d8b3414..a3f466b89179 100644
> >>>>>>>> --- a/arch/loongarch/kernel/smp.c
> >>>>>>>> +++ b/arch/loongarch/kernel/smp.c
> >>>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
> >>>>>>>>                     if (cpuid >= nr_cpu_ids)
> >>>>>>>>                             continue;
> >>>>>>>>
> >>>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>>>> -                       cpu = 0;
> >>>>>>>> -               else
> >>>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>>>> +               cpu = topo_add_cpu(cpuid);
> >>>>>>>> +               if (cpu < 0)
> >>>>>>>> +                       continue;
> >>>>>>>>
> >>>>>>>>                     num_processors++;
> >>>>>>>>                     set_cpu_possible(cpu, true);
> >>>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
> >>>>>>>>                     __cpu_number_map[cpuid] = cpu;
> >>>>>>>>                     __cpu_logical_map[cpu] = cpuid;
> >>>>>>>>
> >>>>>>>> -               early_numa_add_cpu(cpu, 0);
> >>>>>>>> +               early_numa_add_cpu(cpuid, 0);
> >>>>>>>>                     set_cpuid_to_node(cpuid, 0);
> >>>>>>>>             }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> >>>>>>>> --
> >>>>>>>> 2.39.3
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>
> >>
>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 3 weeks, 5 days ago


On 2024/10/30 下午4:34, Huacai Chen wrote:
> On Wed, Oct 30, 2024 at 4:25 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/10/30 下午4:12, Huacai Chen wrote:
>>> On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/10/29 下午6:36, Huacai Chen wrote:
>>>>> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> Hi Huacai,
>>>>>>
>>>>>> On 2024/10/22 上午9:31, Huacai Chen wrote:
>>>>>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
>>>>>>>>> Hi, Bibo,
>>>>>>>>>
>>>>>>>>> This version still doesn't touch the round-robin method, but it
>>>>>>>>> doesn't matter, I think I misunderstood something since V1...
>>>>>>>> I do not understand why round-robin method need be modified, SRAT may be
>>>>>>>> disabled with general function disable_srat(). Then round-robin method
>>>>>>>> is required.
>>>>>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
>>>>>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
>>>>>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
>>>>>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
>>>>>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
>>>>>>>>> < 0.
>>>>>>>>>
>>>>>>>>> If the above is correct, we don't need so complicated, because the
>>>>>>>>> correct and simplest way is:
>>>>>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
>>>>>>>>>
>>>>>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
>>>>>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
>>>>>>>> kernel side.
>>>>>>> Yes, I don't mind, please use that simplest way.
>>>>>> There is another problem with the simple way. eiointc reports error when
>>>>>> cpu is online. The error message is:
>>>>>>       Loongson-64bit Processor probed (LA464 Core)
>>>>>>       CPU2 revision is: 0014c010 (Loongson-64bit)
>>>>>>       FPU2 revision is: 00000001
>>>>>>       eiointc: Error: invalid nodemap!
>>>>>>       CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
>>>>>>
>>>>>> The problem is that node_map of eiointc is problematic,
>>>>>>
>>>>>>
>>>>>> static int cpu_to_eio_node(int cpu)
>>>>>> {
>>>>>>             return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
>>>>>> }
>>>>>>
>>>>>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
>>>>>>                                    u64 node_map)
>>>>>> {
>>>>>>             int i;
>>>>>>
>>>>>>             node_map = node_map ? node_map : -1ULL;
>>>>>>             for_each_possible_cpu(i) {
>>>>>>                     if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
>>>>>>                             node_set(cpu_to_eio_node(i), priv->node_map);
>>>>>>              ...
>>>>>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
>>>>>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
>>>>>> problematic.
>>>>> The error message seems from eiointc_router_init(), but it is a little
>>>>> strange. Physical hot-add should be before logical hot-add. So
>>>>> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
>>>>> set_processor_mask() to setup logical-physical mapping, so in
>>>>> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
>>>>> should work well.
>>>>>
>>>>> Maybe in your case a whole node is hot-added? I don't think the
>>>>> eiointc design can work with this case...
>>>>>
>>>>>>
>>>>>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
>>>>>> not enabled at beginning, it should not be set at hotplug runtime.
>>>>> This will cause the logical cpu number be not continuous after boot.
>>>>> Physical numbers have no requirement, but logical numbers should be
>>>>> continuous.
>>>> I do not understand such requirement about logical cpu should be
>>>> continuous. You can check logical cpu allocation method on other
>>>> architectures, or what does the requirement about logical cpu continuous
>>>> come from.
>>> 1, In an internal conference, it is said that non-continuous cpu
>>> numbers make users think our processors have bugs.
>>> 2, See prefill_possible_map(), it assumes logical numbers continuous
>>> cpu_possible_mask and cpu_present_mask, which make it convenient for
>>> "nr_cpus=xxx".
>>> 3, Can you show me an example in a real machine that "processor" in
>>> /proc/cpuinfo non-continues after boot and before soft hotplug?
>> It is really wasting my time to discuss with you. You does not
>> investigating implementation of other architectures, fully thinking in
>> yourself way.
> Totally wrong, I have implemented what you need, but you should make
> other colleagues (not me) agree with your idea.
> https://github.com/chenhuacai/linux/commit/d8dcf2844d5878b3ac5a42d074e781fe2ebfbae7
So do you mean we should internal discuss inside and post outside? You 
can not decide this since you do not know. And actual code writer (lv 
jianjin) does not reply to you still :(

> 
> Imagine that the cpu_possible_mask is 0b11111111, cpu_present_mask is
> 0b10101010 (non-continuous), how to make "nr_cpus=3" work in a simple
> way?
if (bitmap_weight(cpu_present_mask) >=  nr_cpus))
    then new cpu fails to add.
> 
>>
>> Does the real machines support real cpu hotplug and memory hotplug?
> ACPI_MADT_ENABLED is designed for virtual machines only?
It is the HW board problem, the HW does not support cpu hotplug, neither 
memory hotplug and PCIE hotplug. HW board does not support.

Regards
Bibo Mao

> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>>
>>>
>>>
>>>
>>> Huacai
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo Mao
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>> Bibo Mao
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> On LoongArch system, there are two places to set cpu numa node. One
>>>>>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
>>>>>>>>>> in generic function early_numa_node_init(). The latter will overwrite
>>>>>>>>>> the numa node information.
>>>>>>>>>>
>>>>>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
>>>>>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
>>>>>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
>>>>>>>>>> numa node for hot-added cpu, and generic function
>>>>>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
>>>>>>>>>>
>>>>>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>>>>>>>>>> architectures logic cpu is allocated when parsing MADT table. When
>>>>>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>>>>>>>>>> all allocated logical cpu with matched physical id. It solves such
>>>>>>>>>> problems such as:
>>>>>>>>>>        1. Boot cpu is not the first entry in MADT table, the first entry
>>>>>>>>>> will be overwritten with later boot cpu.
>>>>>>>>>>        2. Physical cpu id not presented in MADT table is invalid, in later
>>>>>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>>>>>>>>>        3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>>>>>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>>>>>>>>>> is correct for hot-add cpu.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>> ---
>>>>>>>>>> v1 ... v2:
>>>>>>>>>>        1. Like other architectures, allocate logic cpu when parsing MADT table.
>>>>>>>>>>        2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>>>>>>>>>> hot-add cpu DSDT information.
>>>>>>>>>> ---
>>>>>>>>>>       arch/loongarch/include/asm/smp.h |  3 ++
>>>>>>>>>>       arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>>>>>>>>>       arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>>>>>>>>>       arch/loongarch/kernel/smp.c      |  9 +++---
>>>>>>>>>>       4 files changed, 70 insertions(+), 13 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>>>>>>>> index 3383c9d24e94..c61b75937a77 100644
>>>>>>>>>> --- a/arch/loongarch/include/asm/smp.h
>>>>>>>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>>>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>>>>>>>>>       #define cpu_logical_map(cpu)   0
>>>>>>>>>>       #endif /* CONFIG_SMP */
>>>>>>>>>>
>>>>>>>>>> +int topo_add_cpu(int physid);
>>>>>>>>>> +int topo_get_cpu(int physid);
>>>>>>>>>> +
>>>>>>>>>>       #endif /* __ASM_SMP_H */
>>>>>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>>>>>>>>>> index f1a74b80f22c..84d9812d5f38 100644
>>>>>>>>>> --- a/arch/loongarch/kernel/acpi.c
>>>>>>>>>> +++ b/arch/loongarch/kernel/acpi.c
>>>>>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>>>>>>>>>                      return -ENODEV;
>>>>>>>>>>
>>>>>>>>>>              }
>>>>>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>>>>>> -               cpu = 0;
>>>>>>>>>> -       else
>>>>>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>>>>>> +
>>>>>>>>>> +       cpu = topo_add_cpu(cpuid);
>>>>>>>>>> +       if (cpu < 0)
>>>>>>>>>> +               return -EEXIST;
>>>>>>>>>>
>>>>>>>>>>              if (!cpu_enumerated)
>>>>>>>>>>                      set_cpu_possible(cpu, true);
>>>>>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>>>>>>>>>                      goto fdt_earlycon;
>>>>>>>>>>              }
>>>>>>>>>>
>>>>>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>>>>>> -
>>>>>>>>>>              /*
>>>>>>>>>>               * Process the Multiple APIC Description Table (MADT), if present
>>>>>>>>>>               */
>>>>>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>>>>>>>>>       void __init
>>>>>>>>>>       acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>>>>>       {
>>>>>>>>>> -       int pxm, node;
>>>>>>>>>> +       int pxm, node, cpu;
>>>>>>>>>>
>>>>>>>>>>              if (srat_disabled())
>>>>>>>>>>                      return;
>>>>>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>>>>>                      return;
>>>>>>>>>>              }
>>>>>>>>>>
>>>>>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
>>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>>>>> +       if (cpu < 0)
>>>>>>>>>> +               return;
>>>>>>>>>> +
>>>>>>>>>>              early_numa_add_cpu(pa->apic_id, node);
>>>>>>>>>>
>>>>>>>>>>              set_cpuid_to_node(pa->apic_id, node);
>>>>>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>>>>>>>>>       {
>>>>>>>>>>              int cpu;
>>>>>>>>>>
>>>>>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>>>>>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>>>>>              if (cpu < 0) {
>>>>>>>>>>                      pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>>>>>>>>>                      return cpu;
>>>>>>>>>>              }
>>>>>>>>>>
>>>>>>>>>> +       num_processors++;
>>>>>>>>>> +       set_cpu_present(cpu, true);
>>>>>>>>>> +       __cpu_number_map[physid] = cpu;
>>>>>>>>>> +       __cpu_logical_map[cpu] = physid;
>>>>>>>>>>              acpi_map_cpu2node(handle, cpu, physid);
>>>>>>>>>>
>>>>>>>>>>              *pcpu = cpu;
>>>>>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>>>>>>>> index 00e307203ddb..649e98640076 100644
>>>>>>>>>> --- a/arch/loongarch/kernel/setup.c
>>>>>>>>>> +++ b/arch/loongarch/kernel/setup.c
>>>>>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>>>>>>>>>
>>>>>>>>>>       struct loongson_board_info b_info;
>>>>>>>>>>       static const char dmi_empty_string[] = "        ";
>>>>>>>>>> +static int possible_cpus;
>>>>>>>>>> +static bool bsp_added;
>>>>>>>>>>
>>>>>>>>>>       /*
>>>>>>>>>>        * Setup information
>>>>>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>>>>>>>>>              *cmdline_p = boot_command_line;
>>>>>>>>>>       }
>>>>>>>>>>
>>>>>>>>>> +int topo_get_cpu(int physid)
>>>>>>>>>> +{
>>>>>>>>>> +       int i;
>>>>>>>>>> +
>>>>>>>>>> +       for (i = 0; i < possible_cpus; i++)
>>>>>>>>>> +               if (cpu_logical_map(i) == physid)
>>>>>>>>>> +                       break;
>>>>>>>>>> +
>>>>>>>>>> +       if (i == possible_cpus)
>>>>>>>>>> +               return -ENOENT;
>>>>>>>>>> +
>>>>>>>>>> +       return i;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +int topo_add_cpu(int physid)
>>>>>>>>>> +{
>>>>>>>>>> +       int cpu;
>>>>>>>>>> +
>>>>>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>>>>>>>>>> +               bsp_added = true;
>>>>>>>>>> +               return 0;
>>>>>>>>>> +       }
>>>>>>>>>> +
>>>>>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>>>>>> +       if (cpu >= 0) {
>>>>>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>>>>>>>>>> +               return -EEXIST;
>>>>>>>>>> +       }
>>>>>>>>>> +
>>>>>>>>>> +       if (possible_cpus >= nr_cpu_ids)
>>>>>>>>>> +               return -ERANGE;
>>>>>>>>>> +
>>>>>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
>>>>>>>>>> +       cpu = possible_cpus++;
>>>>>>>>>> +       return cpu;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void __init topo_init(void)
>>>>>>>>>> +{
>>>>>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>>>>>>>>>> +       possible_cpus++;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>>       void __init platform_init(void)
>>>>>>>>>>       {
>>>>>>>>>>              arch_reserve_vmcore();
>>>>>>>>>>              arch_reserve_crashkernel();
>>>>>>>>>> +       topo_init();
>>>>>>>>>>
>>>>>>>>>>       #ifdef CONFIG_ACPI
>>>>>>>>>>              acpi_table_upgrade();
>>>>>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>>>>>>>> index 9afc2d8b3414..a3f466b89179 100644
>>>>>>>>>> --- a/arch/loongarch/kernel/smp.c
>>>>>>>>>> +++ b/arch/loongarch/kernel/smp.c
>>>>>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>>>>>>>>>                      if (cpuid >= nr_cpu_ids)
>>>>>>>>>>                              continue;
>>>>>>>>>>
>>>>>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>>>>>> -                       cpu = 0;
>>>>>>>>>> -               else
>>>>>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>>>>>> +               cpu = topo_add_cpu(cpuid);
>>>>>>>>>> +               if (cpu < 0)
>>>>>>>>>> +                       continue;
>>>>>>>>>>
>>>>>>>>>>                      num_processors++;
>>>>>>>>>>                      set_cpu_possible(cpu, true);
>>>>>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>>>>>>>>>                      __cpu_number_map[cpuid] = cpu;
>>>>>>>>>>                      __cpu_logical_map[cpu] = cpuid;
>>>>>>>>>>
>>>>>>>>>> -               early_numa_add_cpu(cpu, 0);
>>>>>>>>>> +               early_numa_add_cpu(cpuid, 0);
>>>>>>>>>>                      set_cpuid_to_node(cpuid, 0);
>>>>>>>>>>              }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>>>>>>>>>> --
>>>>>>>>>> 2.39.3
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>>
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 3 weeks, 5 days ago

On Wed, Oct 30, 2024 at 4:48 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/10/30 下午4:34, Huacai Chen wrote:
> > On Wed, Oct 30, 2024 at 4:25 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/10/30 下午4:12, Huacai Chen wrote:
> >>> On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2024/10/29 下午6:36, Huacai Chen wrote:
> >>>>> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>> Hi Huacai,
> >>>>>>
> >>>>>> On 2024/10/22 上午9:31, Huacai Chen wrote:
> >>>>>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
> >>>>>>>>> Hi, Bibo,
> >>>>>>>>>
> >>>>>>>>> This version still doesn't touch the round-robin method, but it
> >>>>>>>>> doesn't matter, I think I misunderstood something since V1...
> >>>>>>>> I do not understand why round-robin method need be modified, SRAT may be
> >>>>>>>> disabled with general function disable_srat(). Then round-robin method
> >>>>>>>> is required.
> >>>>>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
> >>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> >>>>>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
> >>>>>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> >>>>>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
> >>>>>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> >>>>>>>>> < 0.
> >>>>>>>>>
> >>>>>>>>> If the above is correct, we don't need so complicated, because the
> >>>>>>>>> correct and simplest way is:
> >>>>>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> >>>>>>>>>
> >>>>>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
> >>>>>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
> >>>>>>>> kernel side.
> >>>>>>> Yes, I don't mind, please use that simplest way.
> >>>>>> There is another problem with the simple way. eiointc reports error when
> >>>>>> cpu is online. The error message is:
> >>>>>>       Loongson-64bit Processor probed (LA464 Core)
> >>>>>>       CPU2 revision is: 0014c010 (Loongson-64bit)
> >>>>>>       FPU2 revision is: 00000001
> >>>>>>       eiointc: Error: invalid nodemap!
> >>>>>>       CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
> >>>>>>
> >>>>>> The problem is that node_map of eiointc is problematic,
> >>>>>>
> >>>>>>
> >>>>>> static int cpu_to_eio_node(int cpu)
> >>>>>> {
> >>>>>>             return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
> >>>>>> }
> >>>>>>
> >>>>>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
> >>>>>>                                    u64 node_map)
> >>>>>> {
> >>>>>>             int i;
> >>>>>>
> >>>>>>             node_map = node_map ? node_map : -1ULL;
> >>>>>>             for_each_possible_cpu(i) {
> >>>>>>                     if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
> >>>>>>                             node_set(cpu_to_eio_node(i), priv->node_map);
> >>>>>>              ...
> >>>>>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
> >>>>>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
> >>>>>> problematic.
> >>>>> The error message seems from eiointc_router_init(), but it is a little
> >>>>> strange. Physical hot-add should be before logical hot-add. So
> >>>>> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
> >>>>> set_processor_mask() to setup logical-physical mapping, so in
> >>>>> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
> >>>>> should work well.
> >>>>>
> >>>>> Maybe in your case a whole node is hot-added? I don't think the
> >>>>> eiointc design can work with this case...
> >>>>>
> >>>>>>
> >>>>>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
> >>>>>> not enabled at beginning, it should not be set at hotplug runtime.
> >>>>> This will cause the logical cpu number be not continuous after boot.
> >>>>> Physical numbers have no requirement, but logical numbers should be
> >>>>> continuous.
> >>>> I do not understand such requirement about logical cpu should be
> >>>> continuous. You can check logical cpu allocation method on other
> >>>> architectures, or what does the requirement about logical cpu continuous
> >>>> come from.
> >>> 1, In an internal conference, it is said that non-continuous cpu
> >>> numbers make users think our processors have bugs.
> >>> 2, See prefill_possible_map(), it assumes logical numbers continuous
> >>> cpu_possible_mask and cpu_present_mask, which make it convenient for
> >>> "nr_cpus=xxx".
> >>> 3, Can you show me an example in a real machine that "processor" in
> >>> /proc/cpuinfo non-continues after boot and before soft hotplug?
> >> It is really wasting my time to discuss with you. You does not
> >> investigating implementation of other architectures, fully thinking in
> >> yourself way.
> > Totally wrong, I have implemented what you need, but you should make
> > other colleagues (not me) agree with your idea.
> > https://github.com/chenhuacai/linux/commit/d8dcf2844d5878b3ac5a42d074e781fe2ebfbae7
> So do you mean we should internal discuss inside and post outside? You
> can not decide this since you do not know. And actual code writer (lv
> jianjin) does not reply to you still :(
"Non-continuous number is unacceptable" is an internal decision, if
you want to break this decision, you should let other colleagues
agree, otherwise that is the real thing wastes your time.

>
> >
> > Imagine that the cpu_possible_mask is 0b11111111, cpu_present_mask is
> > 0b10101010 (non-continuous), how to make "nr_cpus=3" work in a simple
> > way?
> if (bitmap_weight(cpu_present_mask) >=  nr_cpus))
>     then new cpu fails to add.
No,  I means cpu_possible_mask = 0b11111111, cpu_present_mask =
0b10101010 after MADT parsing, if you need to make "nr_cpus=3" work,
you should modify them to cpu_possible_mask = 0b10101000,
cpu_present_mask = 0b10101000 before smp_init(). But it is difficult
to implement the modification.

> >
> >>
> >> Does the real machines support real cpu hotplug and memory hotplug?
> > ACPI_MADT_ENABLED is designed for virtual machines only?
> It is the HW board problem, the HW does not support cpu hotplug, neither
> memory hotplug and PCIE hotplug. HW board does not support.
So we cannot see non-continuous CPU numbers just because all HW boards
have problems that don't support CPU hotplug, even on x86?

Huacai

>
> Regards
> Bibo Mao
>
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>>
> >>>
> >>>
> >>> Huacai
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo Mao
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Bibo Mao
> >>>>>>>>>
> >>>>>>>>> Huacai
> >>>>>>>>>
> >>>>>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On LoongArch system, there are two places to set cpu numa node. One
> >>>>>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
> >>>>>>>>>> in generic function early_numa_node_init(). The latter will overwrite
> >>>>>>>>>> the numa node information.
> >>>>>>>>>>
> >>>>>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
> >>>>>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
> >>>>>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
> >>>>>>>>>> numa node for hot-added cpu, and generic function
> >>>>>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
> >>>>>>>>>>
> >>>>>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> >>>>>>>>>> architectures logic cpu is allocated when parsing MADT table. When
> >>>>>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> >>>>>>>>>> all allocated logical cpu with matched physical id. It solves such
> >>>>>>>>>> problems such as:
> >>>>>>>>>>        1. Boot cpu is not the first entry in MADT table, the first entry
> >>>>>>>>>> will be overwritten with later boot cpu.
> >>>>>>>>>>        2. Physical cpu id not presented in MADT table is invalid, in later
> >>>>>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
> >>>>>>>>>>        3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> >>>>>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> >>>>>>>>>> is correct for hot-add cpu.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>>>> ---
> >>>>>>>>>> v1 ... v2:
> >>>>>>>>>>        1. Like other architectures, allocate logic cpu when parsing MADT table.
> >>>>>>>>>>        2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> >>>>>>>>>> hot-add cpu DSDT information.
> >>>>>>>>>> ---
> >>>>>>>>>>       arch/loongarch/include/asm/smp.h |  3 ++
> >>>>>>>>>>       arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
> >>>>>>>>>>       arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
> >>>>>>>>>>       arch/loongarch/kernel/smp.c      |  9 +++---
> >>>>>>>>>>       4 files changed, 70 insertions(+), 13 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >>>>>>>>>> index 3383c9d24e94..c61b75937a77 100644
> >>>>>>>>>> --- a/arch/loongarch/include/asm/smp.h
> >>>>>>>>>> +++ b/arch/loongarch/include/asm/smp.h
> >>>>>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
> >>>>>>>>>>       #define cpu_logical_map(cpu)   0
> >>>>>>>>>>       #endif /* CONFIG_SMP */
> >>>>>>>>>>
> >>>>>>>>>> +int topo_add_cpu(int physid);
> >>>>>>>>>> +int topo_get_cpu(int physid);
> >>>>>>>>>> +
> >>>>>>>>>>       #endif /* __ASM_SMP_H */
> >>>>>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> >>>>>>>>>> index f1a74b80f22c..84d9812d5f38 100644
> >>>>>>>>>> --- a/arch/loongarch/kernel/acpi.c
> >>>>>>>>>> +++ b/arch/loongarch/kernel/acpi.c
> >>>>>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
> >>>>>>>>>>                      return -ENODEV;
> >>>>>>>>>>
> >>>>>>>>>>              }
> >>>>>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>>>>>> -               cpu = 0;
> >>>>>>>>>> -       else
> >>>>>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>>>>>> +
> >>>>>>>>>> +       cpu = topo_add_cpu(cpuid);
> >>>>>>>>>> +       if (cpu < 0)
> >>>>>>>>>> +               return -EEXIST;
> >>>>>>>>>>
> >>>>>>>>>>              if (!cpu_enumerated)
> >>>>>>>>>>                      set_cpu_possible(cpu, true);
> >>>>>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
> >>>>>>>>>>                      goto fdt_earlycon;
> >>>>>>>>>>              }
> >>>>>>>>>>
> >>>>>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>>>>>> -
> >>>>>>>>>>              /*
> >>>>>>>>>>               * Process the Multiple APIC Description Table (MADT), if present
> >>>>>>>>>>               */
> >>>>>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
> >>>>>>>>>>       void __init
> >>>>>>>>>>       acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>>>>>       {
> >>>>>>>>>> -       int pxm, node;
> >>>>>>>>>> +       int pxm, node, cpu;
> >>>>>>>>>>
> >>>>>>>>>>              if (srat_disabled())
> >>>>>>>>>>                      return;
> >>>>>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>>>>>                      return;
> >>>>>>>>>>              }
> >>>>>>>>>>
> >>>>>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
> >>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>>>>> +       if (cpu < 0)
> >>>>>>>>>> +               return;
> >>>>>>>>>> +
> >>>>>>>>>>              early_numa_add_cpu(pa->apic_id, node);
> >>>>>>>>>>
> >>>>>>>>>>              set_cpuid_to_node(pa->apic_id, node);
> >>>>>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
> >>>>>>>>>>       {
> >>>>>>>>>>              int cpu;
> >>>>>>>>>>
> >>>>>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> >>>>>>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>>>>>              if (cpu < 0) {
> >>>>>>>>>>                      pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
> >>>>>>>>>>                      return cpu;
> >>>>>>>>>>              }
> >>>>>>>>>>
> >>>>>>>>>> +       num_processors++;
> >>>>>>>>>> +       set_cpu_present(cpu, true);
> >>>>>>>>>> +       __cpu_number_map[physid] = cpu;
> >>>>>>>>>> +       __cpu_logical_map[cpu] = physid;
> >>>>>>>>>>              acpi_map_cpu2node(handle, cpu, physid);
> >>>>>>>>>>
> >>>>>>>>>>              *pcpu = cpu;
> >>>>>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> >>>>>>>>>> index 00e307203ddb..649e98640076 100644
> >>>>>>>>>> --- a/arch/loongarch/kernel/setup.c
> >>>>>>>>>> +++ b/arch/loongarch/kernel/setup.c
> >>>>>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
> >>>>>>>>>>
> >>>>>>>>>>       struct loongson_board_info b_info;
> >>>>>>>>>>       static const char dmi_empty_string[] = "        ";
> >>>>>>>>>> +static int possible_cpus;
> >>>>>>>>>> +static bool bsp_added;
> >>>>>>>>>>
> >>>>>>>>>>       /*
> >>>>>>>>>>        * Setup information
> >>>>>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
> >>>>>>>>>>              *cmdline_p = boot_command_line;
> >>>>>>>>>>       }
> >>>>>>>>>>
> >>>>>>>>>> +int topo_get_cpu(int physid)
> >>>>>>>>>> +{
> >>>>>>>>>> +       int i;
> >>>>>>>>>> +
> >>>>>>>>>> +       for (i = 0; i < possible_cpus; i++)
> >>>>>>>>>> +               if (cpu_logical_map(i) == physid)
> >>>>>>>>>> +                       break;
> >>>>>>>>>> +
> >>>>>>>>>> +       if (i == possible_cpus)
> >>>>>>>>>> +               return -ENOENT;
> >>>>>>>>>> +
> >>>>>>>>>> +       return i;
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>> +int topo_add_cpu(int physid)
> >>>>>>>>>> +{
> >>>>>>>>>> +       int cpu;
> >>>>>>>>>> +
> >>>>>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> >>>>>>>>>> +               bsp_added = true;
> >>>>>>>>>> +               return 0;
> >>>>>>>>>> +       }
> >>>>>>>>>> +
> >>>>>>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>>>>>> +       if (cpu >= 0) {
> >>>>>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> >>>>>>>>>> +               return -EEXIST;
> >>>>>>>>>> +       }
> >>>>>>>>>> +
> >>>>>>>>>> +       if (possible_cpus >= nr_cpu_ids)
> >>>>>>>>>> +               return -ERANGE;
> >>>>>>>>>> +
> >>>>>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
> >>>>>>>>>> +       cpu = possible_cpus++;
> >>>>>>>>>> +       return cpu;
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>> +static void __init topo_init(void)
> >>>>>>>>>> +{
> >>>>>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> >>>>>>>>>> +       possible_cpus++;
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>>       void __init platform_init(void)
> >>>>>>>>>>       {
> >>>>>>>>>>              arch_reserve_vmcore();
> >>>>>>>>>>              arch_reserve_crashkernel();
> >>>>>>>>>> +       topo_init();
> >>>>>>>>>>
> >>>>>>>>>>       #ifdef CONFIG_ACPI
> >>>>>>>>>>              acpi_table_upgrade();
> >>>>>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>>>>>>>>> index 9afc2d8b3414..a3f466b89179 100644
> >>>>>>>>>> --- a/arch/loongarch/kernel/smp.c
> >>>>>>>>>> +++ b/arch/loongarch/kernel/smp.c
> >>>>>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
> >>>>>>>>>>                      if (cpuid >= nr_cpu_ids)
> >>>>>>>>>>                              continue;
> >>>>>>>>>>
> >>>>>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>>>>>> -                       cpu = 0;
> >>>>>>>>>> -               else
> >>>>>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>>>>>> +               cpu = topo_add_cpu(cpuid);
> >>>>>>>>>> +               if (cpu < 0)
> >>>>>>>>>> +                       continue;
> >>>>>>>>>>
> >>>>>>>>>>                      num_processors++;
> >>>>>>>>>>                      set_cpu_possible(cpu, true);
> >>>>>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
> >>>>>>>>>>                      __cpu_number_map[cpuid] = cpu;
> >>>>>>>>>>                      __cpu_logical_map[cpu] = cpuid;
> >>>>>>>>>>
> >>>>>>>>>> -               early_numa_add_cpu(cpu, 0);
> >>>>>>>>>> +               early_numa_add_cpu(cpuid, 0);
> >>>>>>>>>>                      set_cpuid_to_node(cpuid, 0);
> >>>>>>>>>>              }
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> >>>>>>>>>> --
> >>>>>>>>>> 2.39.3
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by maobibo 3 weeks, 5 days ago


On 2024/10/30 下午4:59, Huacai Chen wrote:
> On Wed, Oct 30, 2024 at 4:48 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/10/30 下午4:34, Huacai Chen wrote:
>>> On Wed, Oct 30, 2024 at 4:25 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/10/30 下午4:12, Huacai Chen wrote:
>>>>> On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/10/29 下午6:36, Huacai Chen wrote:
>>>>>>> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>> Hi Huacai,
>>>>>>>>
>>>>>>>> On 2024/10/22 上午9:31, Huacai Chen wrote:
>>>>>>>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
>>>>>>>>>>> Hi, Bibo,
>>>>>>>>>>>
>>>>>>>>>>> This version still doesn't touch the round-robin method, but it
>>>>>>>>>>> doesn't matter, I think I misunderstood something since V1...
>>>>>>>>>> I do not understand why round-robin method need be modified, SRAT may be
>>>>>>>>>> disabled with general function disable_srat(). Then round-robin method
>>>>>>>>>> is required.
>>>>>>>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
>>>>>>>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
>>>>>>>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
>>>>>>>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
>>>>>>>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
>>>>>>>>>>> < 0.
>>>>>>>>>>>
>>>>>>>>>>> If the above is correct, we don't need so complicated, because the
>>>>>>>>>>> correct and simplest way is:
>>>>>>>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
>>>>>>>>>>>
>>>>>>>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
>>>>>>>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
>>>>>>>>>> kernel side.
>>>>>>>>> Yes, I don't mind, please use that simplest way.
>>>>>>>> There is another problem with the simple way. eiointc reports error when
>>>>>>>> cpu is online. The error message is:
>>>>>>>>        Loongson-64bit Processor probed (LA464 Core)
>>>>>>>>        CPU2 revision is: 0014c010 (Loongson-64bit)
>>>>>>>>        FPU2 revision is: 00000001
>>>>>>>>        eiointc: Error: invalid nodemap!
>>>>>>>>        CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
>>>>>>>>
>>>>>>>> The problem is that node_map of eiointc is problematic,
>>>>>>>>
>>>>>>>>
>>>>>>>> static int cpu_to_eio_node(int cpu)
>>>>>>>> {
>>>>>>>>              return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
>>>>>>>> }
>>>>>>>>
>>>>>>>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
>>>>>>>>                                     u64 node_map)
>>>>>>>> {
>>>>>>>>              int i;
>>>>>>>>
>>>>>>>>              node_map = node_map ? node_map : -1ULL;
>>>>>>>>              for_each_possible_cpu(i) {
>>>>>>>>                      if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
>>>>>>>>                              node_set(cpu_to_eio_node(i), priv->node_map);
>>>>>>>>               ...
>>>>>>>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
>>>>>>>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
>>>>>>>> problematic.
>>>>>>> The error message seems from eiointc_router_init(), but it is a little
>>>>>>> strange. Physical hot-add should be before logical hot-add. So
>>>>>>> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
>>>>>>> set_processor_mask() to setup logical-physical mapping, so in
>>>>>>> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
>>>>>>> should work well.
>>>>>>>
>>>>>>> Maybe in your case a whole node is hot-added? I don't think the
>>>>>>> eiointc design can work with this case...
>>>>>>>
>>>>>>>>
>>>>>>>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
>>>>>>>> not enabled at beginning, it should not be set at hotplug runtime.
>>>>>>> This will cause the logical cpu number be not continuous after boot.
>>>>>>> Physical numbers have no requirement, but logical numbers should be
>>>>>>> continuous.
>>>>>> I do not understand such requirement about logical cpu should be
>>>>>> continuous. You can check logical cpu allocation method on other
>>>>>> architectures, or what does the requirement about logical cpu continuous
>>>>>> come from.
>>>>> 1, In an internal conference, it is said that non-continuous cpu
>>>>> numbers make users think our processors have bugs.
>>>>> 2, See prefill_possible_map(), it assumes logical numbers continuous
>>>>> cpu_possible_mask and cpu_present_mask, which make it convenient for
>>>>> "nr_cpus=xxx".
>>>>> 3, Can you show me an example in a real machine that "processor" in
>>>>> /proc/cpuinfo non-continues after boot and before soft hotplug?
>>>> It is really wasting my time to discuss with you. You does not
>>>> investigating implementation of other architectures, fully thinking in
>>>> yourself way.
>>> Totally wrong, I have implemented what you need, but you should make
>>> other colleagues (not me) agree with your idea.
>>> https://github.com/chenhuacai/linux/commit/d8dcf2844d5878b3ac5a42d074e781fe2ebfbae7
>> So do you mean we should internal discuss inside and post outside? You
>> can not decide this since you do not know. And actual code writer (lv
>> jianjin) does not reply to you still :(
> "Non-continuous number is unacceptable" is an internal decision, if
> you want to break this decision, you should let other colleagues
> agree, otherwise that is the real thing wastes your time.
If so I will not touch any kernel code any more, please write the patch 
to fix it to keep your maintainer position stable. Is that ok for you?


> 
>>
>>>
>>> Imagine that the cpu_possible_mask is 0b11111111, cpu_present_mask is
>>> 0b10101010 (non-continuous), how to make "nr_cpus=3" work in a simple
>>> way?
>> if (bitmap_weight(cpu_present_mask) >=  nr_cpus))
>>      then new cpu fails to add.
> No,  I means cpu_possible_mask = 0b11111111, cpu_present_mask =
> 0b10101010 after MADT parsing, if you need to make "nr_cpus=3" work,
> you should modify them to cpu_possible_mask = 0b10101000,
> cpu_present_mask = 0b10101000 before smp_init(). But it is difficult
> to implement the modification.
> 
>>>
>>>>
>>>> Does the real machines support real cpu hotplug and memory hotplug?
>>> ACPI_MADT_ENABLED is designed for virtual machines only?
>> It is the HW board problem, the HW does not support cpu hotplug, neither
>> memory hotplug and PCIE hotplug. HW board does not support.
> So we cannot see non-continuous CPU numbers just because all HW boards
> have problems that don't support CPU hotplug, even on x86?
> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Huacai
>>>>>>
>>>>>> Regards
>>>>>> Bibo Mao
>>>>>>
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Bibo Mao
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bibo Mao
>>>>>>>>>>>
>>>>>>>>>>> Huacai
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On LoongArch system, there are two places to set cpu numa node. One
>>>>>>>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
>>>>>>>>>>>> in generic function early_numa_node_init(). The latter will overwrite
>>>>>>>>>>>> the numa node information.
>>>>>>>>>>>>
>>>>>>>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
>>>>>>>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
>>>>>>>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
>>>>>>>>>>>> numa node for hot-added cpu, and generic function
>>>>>>>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
>>>>>>>>>>>>
>>>>>>>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
>>>>>>>>>>>> architectures logic cpu is allocated when parsing MADT table. When
>>>>>>>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
>>>>>>>>>>>> all allocated logical cpu with matched physical id. It solves such
>>>>>>>>>>>> problems such as:
>>>>>>>>>>>>         1. Boot cpu is not the first entry in MADT table, the first entry
>>>>>>>>>>>> will be overwritten with later boot cpu.
>>>>>>>>>>>>         2. Physical cpu id not presented in MADT table is invalid, in later
>>>>>>>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
>>>>>>>>>>>>         3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
>>>>>>>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
>>>>>>>>>>>> is correct for hot-add cpu.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>>> ---
>>>>>>>>>>>> v1 ... v2:
>>>>>>>>>>>>         1. Like other architectures, allocate logic cpu when parsing MADT table.
>>>>>>>>>>>>         2. Add invalid or duplicated physical cpuid parsing with SRAT table or
>>>>>>>>>>>> hot-add cpu DSDT information.
>>>>>>>>>>>> ---
>>>>>>>>>>>>        arch/loongarch/include/asm/smp.h |  3 ++
>>>>>>>>>>>>        arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
>>>>>>>>>>>>        arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>        arch/loongarch/kernel/smp.c      |  9 +++---
>>>>>>>>>>>>        4 files changed, 70 insertions(+), 13 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>>>>>>>>>>>> index 3383c9d24e94..c61b75937a77 100644
>>>>>>>>>>>> --- a/arch/loongarch/include/asm/smp.h
>>>>>>>>>>>> +++ b/arch/loongarch/include/asm/smp.h
>>>>>>>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
>>>>>>>>>>>>        #define cpu_logical_map(cpu)   0
>>>>>>>>>>>>        #endif /* CONFIG_SMP */
>>>>>>>>>>>>
>>>>>>>>>>>> +int topo_add_cpu(int physid);
>>>>>>>>>>>> +int topo_get_cpu(int physid);
>>>>>>>>>>>> +
>>>>>>>>>>>>        #endif /* __ASM_SMP_H */
>>>>>>>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>>>>>>>>>>>> index f1a74b80f22c..84d9812d5f38 100644
>>>>>>>>>>>> --- a/arch/loongarch/kernel/acpi.c
>>>>>>>>>>>> +++ b/arch/loongarch/kernel/acpi.c
>>>>>>>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
>>>>>>>>>>>>                       return -ENODEV;
>>>>>>>>>>>>
>>>>>>>>>>>>               }
>>>>>>>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>>>>>>>> -               cpu = 0;
>>>>>>>>>>>> -       else
>>>>>>>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>>>>>>>> +
>>>>>>>>>>>> +       cpu = topo_add_cpu(cpuid);
>>>>>>>>>>>> +       if (cpu < 0)
>>>>>>>>>>>> +               return -EEXIST;
>>>>>>>>>>>>
>>>>>>>>>>>>               if (!cpu_enumerated)
>>>>>>>>>>>>                       set_cpu_possible(cpu, true);
>>>>>>>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
>>>>>>>>>>>>                       goto fdt_earlycon;
>>>>>>>>>>>>               }
>>>>>>>>>>>>
>>>>>>>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>>>>>>>> -
>>>>>>>>>>>>               /*
>>>>>>>>>>>>                * Process the Multiple APIC Description Table (MADT), if present
>>>>>>>>>>>>                */
>>>>>>>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
>>>>>>>>>>>>        void __init
>>>>>>>>>>>>        acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>>>>>>>        {
>>>>>>>>>>>> -       int pxm, node;
>>>>>>>>>>>> +       int pxm, node, cpu;
>>>>>>>>>>>>
>>>>>>>>>>>>               if (srat_disabled())
>>>>>>>>>>>>                       return;
>>>>>>>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>>>>>>>>>>>>                       return;
>>>>>>>>>>>>               }
>>>>>>>>>>>>
>>>>>>>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
>>>>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>>>>>>> +       if (cpu < 0)
>>>>>>>>>>>> +               return;
>>>>>>>>>>>> +
>>>>>>>>>>>>               early_numa_add_cpu(pa->apic_id, node);
>>>>>>>>>>>>
>>>>>>>>>>>>               set_cpuid_to_node(pa->apic_id, node);
>>>>>>>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
>>>>>>>>>>>>        {
>>>>>>>>>>>>               int cpu;
>>>>>>>>>>>>
>>>>>>>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
>>>>>>>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
>>>>>>>>>>>>               if (cpu < 0) {
>>>>>>>>>>>>                       pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
>>>>>>>>>>>>                       return cpu;
>>>>>>>>>>>>               }
>>>>>>>>>>>>
>>>>>>>>>>>> +       num_processors++;
>>>>>>>>>>>> +       set_cpu_present(cpu, true);
>>>>>>>>>>>> +       __cpu_number_map[physid] = cpu;
>>>>>>>>>>>> +       __cpu_logical_map[cpu] = physid;
>>>>>>>>>>>>               acpi_map_cpu2node(handle, cpu, physid);
>>>>>>>>>>>>
>>>>>>>>>>>>               *pcpu = cpu;
>>>>>>>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>>>>>>>>>> index 00e307203ddb..649e98640076 100644
>>>>>>>>>>>> --- a/arch/loongarch/kernel/setup.c
>>>>>>>>>>>> +++ b/arch/loongarch/kernel/setup.c
>>>>>>>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
>>>>>>>>>>>>
>>>>>>>>>>>>        struct loongson_board_info b_info;
>>>>>>>>>>>>        static const char dmi_empty_string[] = "        ";
>>>>>>>>>>>> +static int possible_cpus;
>>>>>>>>>>>> +static bool bsp_added;
>>>>>>>>>>>>
>>>>>>>>>>>>        /*
>>>>>>>>>>>>         * Setup information
>>>>>>>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
>>>>>>>>>>>>               *cmdline_p = boot_command_line;
>>>>>>>>>>>>        }
>>>>>>>>>>>>
>>>>>>>>>>>> +int topo_get_cpu(int physid)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +       int i;
>>>>>>>>>>>> +
>>>>>>>>>>>> +       for (i = 0; i < possible_cpus; i++)
>>>>>>>>>>>> +               if (cpu_logical_map(i) == physid)
>>>>>>>>>>>> +                       break;
>>>>>>>>>>>> +
>>>>>>>>>>>> +       if (i == possible_cpus)
>>>>>>>>>>>> +               return -ENOENT;
>>>>>>>>>>>> +
>>>>>>>>>>>> +       return i;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> +int topo_add_cpu(int physid)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +       int cpu;
>>>>>>>>>>>> +
>>>>>>>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
>>>>>>>>>>>> +               bsp_added = true;
>>>>>>>>>>>> +               return 0;
>>>>>>>>>>>> +       }
>>>>>>>>>>>> +
>>>>>>>>>>>> +       cpu = topo_get_cpu(physid);
>>>>>>>>>>>> +       if (cpu >= 0) {
>>>>>>>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
>>>>>>>>>>>> +               return -EEXIST;
>>>>>>>>>>>> +       }
>>>>>>>>>>>> +
>>>>>>>>>>>> +       if (possible_cpus >= nr_cpu_ids)
>>>>>>>>>>>> +               return -ERANGE;
>>>>>>>>>>>> +
>>>>>>>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
>>>>>>>>>>>> +       cpu = possible_cpus++;
>>>>>>>>>>>> +       return cpu;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> +static void __init topo_init(void)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
>>>>>>>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
>>>>>>>>>>>> +       possible_cpus++;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>>        void __init platform_init(void)
>>>>>>>>>>>>        {
>>>>>>>>>>>>               arch_reserve_vmcore();
>>>>>>>>>>>>               arch_reserve_crashkernel();
>>>>>>>>>>>> +       topo_init();
>>>>>>>>>>>>
>>>>>>>>>>>>        #ifdef CONFIG_ACPI
>>>>>>>>>>>>               acpi_table_upgrade();
>>>>>>>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>>>>>>>>>>> index 9afc2d8b3414..a3f466b89179 100644
>>>>>>>>>>>> --- a/arch/loongarch/kernel/smp.c
>>>>>>>>>>>> +++ b/arch/loongarch/kernel/smp.c
>>>>>>>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
>>>>>>>>>>>>                       if (cpuid >= nr_cpu_ids)
>>>>>>>>>>>>                               continue;
>>>>>>>>>>>>
>>>>>>>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
>>>>>>>>>>>> -                       cpu = 0;
>>>>>>>>>>>> -               else
>>>>>>>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
>>>>>>>>>>>> +               cpu = topo_add_cpu(cpuid);
>>>>>>>>>>>> +               if (cpu < 0)
>>>>>>>>>>>> +                       continue;
>>>>>>>>>>>>
>>>>>>>>>>>>                       num_processors++;
>>>>>>>>>>>>                       set_cpu_possible(cpu, true);
>>>>>>>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
>>>>>>>>>>>>                       __cpu_number_map[cpuid] = cpu;
>>>>>>>>>>>>                       __cpu_logical_map[cpu] = cpuid;
>>>>>>>>>>>>
>>>>>>>>>>>> -               early_numa_add_cpu(cpu, 0);
>>>>>>>>>>>> +               early_numa_add_cpu(cpuid, 0);
>>>>>>>>>>>>                       set_cpuid_to_node(cpuid, 0);
>>>>>>>>>>>>               }
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
>>>>>>>>>>>> --
>>>>>>>>>>>> 2.39.3
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>

Re: [PATCH v2] LoongArch: Fix cpu hotplug issue

Posted by Huacai Chen 3 weeks, 5 days ago

On Wed, Oct 30, 2024 at 5:10 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/10/30 下午4:59, Huacai Chen wrote:
> > On Wed, Oct 30, 2024 at 4:48 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/10/30 下午4:34, Huacai Chen wrote:
> >>> On Wed, Oct 30, 2024 at 4:25 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2024/10/30 下午4:12, Huacai Chen wrote:
> >>>>> On Tue, Oct 29, 2024 at 7:49 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2024/10/29 下午6:36, Huacai Chen wrote:
> >>>>>>> On Mon, Oct 28, 2024 at 8:38 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>> Hi Huacai,
> >>>>>>>>
> >>>>>>>> On 2024/10/22 上午9:31, Huacai Chen wrote:
> >>>>>>>>> On Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On 2024/10/21 下午10:32, Huacai Chen wrote:
> >>>>>>>>>>> Hi, Bibo,
> >>>>>>>>>>>
> >>>>>>>>>>> This version still doesn't touch the round-robin method, but it
> >>>>>>>>>>> doesn't matter, I think I misunderstood something since V1...
> >>>>>>>>>> I do not understand why round-robin method need be modified, SRAT may be
> >>>>>>>>>> disabled with general function disable_srat(). Then round-robin method
> >>>>>>>>>> is required.
> >>>>>>>>> I don't mean round-robin should be modified, I mean I misunderstand round-robin.
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Please correct me if I'm wrong: For cpus without ACPI_MADT_ENABLED, in
> >>>>>>>>>>> smp_prepare_boot_cpu() the round-robin node ids only apply to
> >>>>>>>>>>> cpu_to_node(), but __cpuid_to_node[] still record the right node ids.
> >>>>>>>>>>> early_cpu_to_node() returns NUMA_NO_NODE not because
> >>>>>>>>>>> __cpuid_to_node[] records NUMA_NO_NODE, but because cpu_logical_map()
> >>>>>>>>>>> < 0.
> >>>>>>>>>>>
> >>>>>>>>>>> If the above is correct, we don't need so complicated, because the
> >>>>>>>>>>> correct and simplest way is:
> >>>>>>>>>>> https://lore.kernel.org/loongarch/6b2b3e89-5a46-2d20-3dfb-7aae33839f49@loongson.cn/T/#m950eead5250e5992cc703bbe69622348cecfa465
> >>>>>>>>>>>
> >>>>>>>>>> It works also. Only that LoongArch kernel parsing about SRAT/MADT is
> >>>>>>>>>> badly. If you do not mind, I do not mind neither. It is not my duty for
> >>>>>>>>>> kernel side.
> >>>>>>>>> Yes, I don't mind, please use that simplest way.
> >>>>>>>> There is another problem with the simple way. eiointc reports error when
> >>>>>>>> cpu is online. The error message is:
> >>>>>>>>        Loongson-64bit Processor probed (LA464 Core)
> >>>>>>>>        CPU2 revision is: 0014c010 (Loongson-64bit)
> >>>>>>>>        FPU2 revision is: 00000001
> >>>>>>>>        eiointc: Error: invalid nodemap!
> >>>>>>>>        CPU 2 UP state irqchip/loongarch/eiointc:starting (100) failed (-1)
> >>>>>>>>
> >>>>>>>> The problem is that node_map of eiointc is problematic,
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> static int cpu_to_eio_node(int cpu)
> >>>>>>>> {
> >>>>>>>>              return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
> >>>>>>>>                                     u64 node_map)
> >>>>>>>> {
> >>>>>>>>              int i;
> >>>>>>>>
> >>>>>>>>              node_map = node_map ? node_map : -1ULL;
> >>>>>>>>              for_each_possible_cpu(i) {
> >>>>>>>>                      if (node_map & (1ULL << (cpu_to_eio_node(i)))) {
> >>>>>>>>                              node_set(cpu_to_eio_node(i), priv->node_map);
> >>>>>>>>               ...
> >>>>>>>> The cause is that for possible not present cpu, *cpu_logical_map(cpu)*
> >>>>>>>> is -1, cpu_to_eio_node(i) will be equal to -1, so node_map of eiointc is
> >>>>>>>> problematic.
> >>>>>>> The error message seems from eiointc_router_init(), but it is a little
> >>>>>>> strange. Physical hot-add should be before logical hot-add. So
> >>>>>>> acpi_map_cpu() is before cpu_up(). acpi_map_cpu() calls
> >>>>>>> set_processor_mask() to setup logical-physical mapping, so in
> >>>>>>> eiointc_router_init() which is called by cpu_up(), cpu_logical_map()
> >>>>>>> should work well.
> >>>>>>>
> >>>>>>> Maybe in your case a whole node is hot-added? I don't think the
> >>>>>>> eiointc design can work with this case...
> >>>>>>>
> >>>>>>>>
> >>>>>>>> So cpu_logical_map(cpu) should be set during MADT parsing even if it is
> >>>>>>>> not enabled at beginning, it should not be set at hotplug runtime.
> >>>>>>> This will cause the logical cpu number be not continuous after boot.
> >>>>>>> Physical numbers have no requirement, but logical numbers should be
> >>>>>>> continuous.
> >>>>>> I do not understand such requirement about logical cpu should be
> >>>>>> continuous. You can check logical cpu allocation method on other
> >>>>>> architectures, or what does the requirement about logical cpu continuous
> >>>>>> come from.
> >>>>> 1, In an internal conference, it is said that non-continuous cpu
> >>>>> numbers make users think our processors have bugs.
> >>>>> 2, See prefill_possible_map(), it assumes logical numbers continuous
> >>>>> cpu_possible_mask and cpu_present_mask, which make it convenient for
> >>>>> "nr_cpus=xxx".
> >>>>> 3, Can you show me an example in a real machine that "processor" in
> >>>>> /proc/cpuinfo non-continues after boot and before soft hotplug?
> >>>> It is really wasting my time to discuss with you. You does not
> >>>> investigating implementation of other architectures, fully thinking in
> >>>> yourself way.
> >>> Totally wrong, I have implemented what you need, but you should make
> >>> other colleagues (not me) agree with your idea.
> >>> https://github.com/chenhuacai/linux/commit/d8dcf2844d5878b3ac5a42d074e781fe2ebfbae7
> >> So do you mean we should internal discuss inside and post outside? You
> >> can not decide this since you do not know. And actual code writer (lv
> >> jianjin) does not reply to you still :(
> > "Non-continuous number is unacceptable" is an internal decision, if
> > you want to break this decision, you should let other colleagues
> > agree, otherwise that is the real thing wastes your time.
> If so I will not touch any kernel code any more, please write the patch
> to fix it to keep your maintainer position stable. Is that ok for you?
So you mean you are pushing me to do what violates the team decision,
and your original goal is make my position unstable?

Of course I don't really think so, but your sentences look like this.

Please check whether this solution can solve your problem, then let
others agree your idea.
https://github.com/chenhuacai/linux/commit/d8dcf2844d5878b3ac5a42d074e781fe2ebfbae7

Huacai

>
>
> >
> >>
> >>>
> >>> Imagine that the cpu_possible_mask is 0b11111111, cpu_present_mask is
> >>> 0b10101010 (non-continuous), how to make "nr_cpus=3" work in a simple
> >>> way?
> >> if (bitmap_weight(cpu_present_mask) >=  nr_cpus))
> >>      then new cpu fails to add.
> > No,  I means cpu_possible_mask = 0b11111111, cpu_present_mask =
> > 0b10101010 after MADT parsing, if you need to make "nr_cpus=3" work,
> > you should modify them to cpu_possible_mask = 0b10101000,
> > cpu_present_mask = 0b10101000 before smp_init(). But it is difficult
> > to implement the modification.
> >
> >>>
> >>>>
> >>>> Does the real machines support real cpu hotplug and memory hotplug?
> >>> ACPI_MADT_ENABLED is designed for virtual machines only?
> >> It is the HW board problem, the HW does not support cpu hotplug, neither
> >> memory hotplug and PCIE hotplug. HW board does not support.
> > So we cannot see non-continuous CPU numbers just because all HW boards
> > have problems that don't support CPU hotplug, even on x86?
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> Huacai
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo Mao
> >>>>>>
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Regards
> >>>>>>>> Bibo Mao
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Huacai
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Bibo Mao
> >>>>>>>>>>>
> >>>>>>>>>>> Huacai
> >>>>>>>>>>>
> >>>>>>>>>>> On Mon, Oct 21, 2024 at 4:04 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> On LoongArch system, there are two places to set cpu numa node. One
> >>>>>>>>>>>> is in arch specified function smp_prepare_boot_cpu(), the other is
> >>>>>>>>>>>> in generic function early_numa_node_init(). The latter will overwrite
> >>>>>>>>>>>> the numa node information.
> >>>>>>>>>>>>
> >>>>>>>>>>>> With hot-added cpu without numa information, cpu_logical_map() fails
> >>>>>>>>>>>> to its physical cpuid at beginning since it is not enabled in ACPI
> >>>>>>>>>>>> MADT table. So function early_cpu_to_node() also fails to get its
> >>>>>>>>>>>> numa node for hot-added cpu, and generic function
> >>>>>>>>>>>> early_numa_node_init() will overwrite with incorrect numa node.
> >>>>>>>>>>>>
> >>>>>>>>>>>> APIs topo_get_cpu() and topo_add_cpu() is added here, like other
> >>>>>>>>>>>> architectures logic cpu is allocated when parsing MADT table. When
> >>>>>>>>>>>> parsing SRAT table or hot-add cpu, logic cpu is acquired by searching
> >>>>>>>>>>>> all allocated logical cpu with matched physical id. It solves such
> >>>>>>>>>>>> problems such as:
> >>>>>>>>>>>>         1. Boot cpu is not the first entry in MADT table, the first entry
> >>>>>>>>>>>> will be overwritten with later boot cpu.
> >>>>>>>>>>>>         2. Physical cpu id not presented in MADT table is invalid, in later
> >>>>>>>>>>>> SRAT/hot-add cpu parsing, invalid physical cpu detected is added
> >>>>>>>>>>>>         3. For hot-add cpu, its logic cpu is allocated in MADT table parsing,
> >>>>>>>>>>>> so early_cpu_to_node() can be used for hot-add cpu and cpu_to_node()
> >>>>>>>>>>>> is correct for hot-add cpu.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>> v1 ... v2:
> >>>>>>>>>>>>         1. Like other architectures, allocate logic cpu when parsing MADT table.
> >>>>>>>>>>>>         2. Add invalid or duplicated physical cpuid parsing with SRAT table or
> >>>>>>>>>>>> hot-add cpu DSDT information.
> >>>>>>>>>>>> ---
> >>>>>>>>>>>>        arch/loongarch/include/asm/smp.h |  3 ++
> >>>>>>>>>>>>        arch/loongarch/kernel/acpi.c     | 24 ++++++++++------
> >>>>>>>>>>>>        arch/loongarch/kernel/setup.c    | 47 ++++++++++++++++++++++++++++++++
> >>>>>>>>>>>>        arch/loongarch/kernel/smp.c      |  9 +++---
> >>>>>>>>>>>>        4 files changed, 70 insertions(+), 13 deletions(-)
> >>>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> >>>>>>>>>>>> index 3383c9d24e94..c61b75937a77 100644
> >>>>>>>>>>>> --- a/arch/loongarch/include/asm/smp.h
> >>>>>>>>>>>> +++ b/arch/loongarch/include/asm/smp.h
> >>>>>>>>>>>> @@ -119,4 +119,7 @@ static inline void __cpu_die(unsigned int cpu)
> >>>>>>>>>>>>        #define cpu_logical_map(cpu)   0
> >>>>>>>>>>>>        #endif /* CONFIG_SMP */
> >>>>>>>>>>>>
> >>>>>>>>>>>> +int topo_add_cpu(int physid);
> >>>>>>>>>>>> +int topo_get_cpu(int physid);
> >>>>>>>>>>>> +
> >>>>>>>>>>>>        #endif /* __ASM_SMP_H */
> >>>>>>>>>>>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> >>>>>>>>>>>> index f1a74b80f22c..84d9812d5f38 100644
> >>>>>>>>>>>> --- a/arch/loongarch/kernel/acpi.c
> >>>>>>>>>>>> +++ b/arch/loongarch/kernel/acpi.c
> >>>>>>>>>>>> @@ -78,10 +78,10 @@ static int set_processor_mask(u32 id, u32 flags)
> >>>>>>>>>>>>                       return -ENODEV;
> >>>>>>>>>>>>
> >>>>>>>>>>>>               }
> >>>>>>>>>>>> -       if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>>>>>>>> -               cpu = 0;
> >>>>>>>>>>>> -       else
> >>>>>>>>>>>> -               cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       cpu = topo_add_cpu(cpuid);
> >>>>>>>>>>>> +       if (cpu < 0)
> >>>>>>>>>>>> +               return -EEXIST;
> >>>>>>>>>>>>
> >>>>>>>>>>>>               if (!cpu_enumerated)
> >>>>>>>>>>>>                       set_cpu_possible(cpu, true);
> >>>>>>>>>>>> @@ -203,8 +203,6 @@ void __init acpi_boot_table_init(void)
> >>>>>>>>>>>>                       goto fdt_earlycon;
> >>>>>>>>>>>>               }
> >>>>>>>>>>>>
> >>>>>>>>>>>> -       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>>>>>>>> -
> >>>>>>>>>>>>               /*
> >>>>>>>>>>>>                * Process the Multiple APIC Description Table (MADT), if present
> >>>>>>>>>>>>                */
> >>>>>>>>>>>> @@ -257,7 +255,7 @@ void __init numa_set_distance(int from, int to, int distance)
> >>>>>>>>>>>>        void __init
> >>>>>>>>>>>>        acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>>>>>>>        {
> >>>>>>>>>>>> -       int pxm, node;
> >>>>>>>>>>>> +       int pxm, node, cpu;
> >>>>>>>>>>>>
> >>>>>>>>>>>>               if (srat_disabled())
> >>>>>>>>>>>>                       return;
> >>>>>>>>>>>> @@ -286,6 +284,11 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
> >>>>>>>>>>>>                       return;
> >>>>>>>>>>>>               }
> >>>>>>>>>>>>
> >>>>>>>>>>>> +       cpu = topo_get_cpu(pa->apic_id);
> >>>>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>>>>>>> +       if (cpu < 0)
> >>>>>>>>>>>> +               return;
> >>>>>>>>>>>> +
> >>>>>>>>>>>>               early_numa_add_cpu(pa->apic_id, node);
> >>>>>>>>>>>>
> >>>>>>>>>>>>               set_cpuid_to_node(pa->apic_id, node);
> >>>>>>>>>>>> @@ -324,12 +327,17 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int *pcpu
> >>>>>>>>>>>>        {
> >>>>>>>>>>>>               int cpu;
> >>>>>>>>>>>>
> >>>>>>>>>>>> -       cpu = set_processor_mask(physid, ACPI_MADT_ENABLED);
> >>>>>>>>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>>>>>>>> +       /* Check whether apic_id exists in MADT table */
> >>>>>>>>>>>>               if (cpu < 0) {
> >>>>>>>>>>>>                       pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
> >>>>>>>>>>>>                       return cpu;
> >>>>>>>>>>>>               }
> >>>>>>>>>>>>
> >>>>>>>>>>>> +       num_processors++;
> >>>>>>>>>>>> +       set_cpu_present(cpu, true);
> >>>>>>>>>>>> +       __cpu_number_map[physid] = cpu;
> >>>>>>>>>>>> +       __cpu_logical_map[cpu] = physid;
> >>>>>>>>>>>>               acpi_map_cpu2node(handle, cpu, physid);
> >>>>>>>>>>>>
> >>>>>>>>>>>>               *pcpu = cpu;
> >>>>>>>>>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> >>>>>>>>>>>> index 00e307203ddb..649e98640076 100644
> >>>>>>>>>>>> --- a/arch/loongarch/kernel/setup.c
> >>>>>>>>>>>> +++ b/arch/loongarch/kernel/setup.c
> >>>>>>>>>>>> @@ -65,6 +65,8 @@ EXPORT_SYMBOL(cpu_data);
> >>>>>>>>>>>>
> >>>>>>>>>>>>        struct loongson_board_info b_info;
> >>>>>>>>>>>>        static const char dmi_empty_string[] = "        ";
> >>>>>>>>>>>> +static int possible_cpus;
> >>>>>>>>>>>> +static bool bsp_added;
> >>>>>>>>>>>>
> >>>>>>>>>>>>        /*
> >>>>>>>>>>>>         * Setup information
> >>>>>>>>>>>> @@ -346,10 +348,55 @@ static void __init bootcmdline_init(char **cmdline_p)
> >>>>>>>>>>>>               *cmdline_p = boot_command_line;
> >>>>>>>>>>>>        }
> >>>>>>>>>>>>
> >>>>>>>>>>>> +int topo_get_cpu(int physid)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> +       int i;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       for (i = 0; i < possible_cpus; i++)
> >>>>>>>>>>>> +               if (cpu_logical_map(i) == physid)
> >>>>>>>>>>>> +                       break;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       if (i == possible_cpus)
> >>>>>>>>>>>> +               return -ENOENT;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       return i;
> >>>>>>>>>>>> +}
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +int topo_add_cpu(int physid)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> +       int cpu;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       if (!bsp_added && (physid == loongson_sysconf.boot_cpu_id)) {
> >>>>>>>>>>>> +               bsp_added = true;
> >>>>>>>>>>>> +               return 0;
> >>>>>>>>>>>> +       }
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       cpu = topo_get_cpu(physid);
> >>>>>>>>>>>> +       if (cpu >= 0) {
> >>>>>>>>>>>> +               pr_warn("Adding duplicated physical cpuid 0x%x\n", physid);
> >>>>>>>>>>>> +               return -EEXIST;
> >>>>>>>>>>>> +       }
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       if (possible_cpus >= nr_cpu_ids)
> >>>>>>>>>>>> +               return -ERANGE;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       __cpu_logical_map[possible_cpus] = physid;
> >>>>>>>>>>>> +       cpu = possible_cpus++;
> >>>>>>>>>>>> +       return cpu;
> >>>>>>>>>>>> +}
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +static void __init topo_init(void)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> +       loongson_sysconf.boot_cpu_id = read_csr_cpuid();
> >>>>>>>>>>>> +       __cpu_logical_map[0] = loongson_sysconf.boot_cpu_id;
> >>>>>>>>>>>> +       possible_cpus++;
> >>>>>>>>>>>> +}
> >>>>>>>>>>>> +
> >>>>>>>>>>>>        void __init platform_init(void)
> >>>>>>>>>>>>        {
> >>>>>>>>>>>>               arch_reserve_vmcore();
> >>>>>>>>>>>>               arch_reserve_crashkernel();
> >>>>>>>>>>>> +       topo_init();
> >>>>>>>>>>>>
> >>>>>>>>>>>>        #ifdef CONFIG_ACPI
> >>>>>>>>>>>>               acpi_table_upgrade();
> >>>>>>>>>>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>>>>>>>>>>> index 9afc2d8b3414..a3f466b89179 100644
> >>>>>>>>>>>> --- a/arch/loongarch/kernel/smp.c
> >>>>>>>>>>>> +++ b/arch/loongarch/kernel/smp.c
> >>>>>>>>>>>> @@ -291,10 +291,9 @@ static void __init fdt_smp_setup(void)
> >>>>>>>>>>>>                       if (cpuid >= nr_cpu_ids)
> >>>>>>>>>>>>                               continue;
> >>>>>>>>>>>>
> >>>>>>>>>>>> -               if (cpuid == loongson_sysconf.boot_cpu_id)
> >>>>>>>>>>>> -                       cpu = 0;
> >>>>>>>>>>>> -               else
> >>>>>>>>>>>> -                       cpu = find_first_zero_bit(cpumask_bits(cpu_present_mask), NR_CPUS);
> >>>>>>>>>>>> +               cpu = topo_add_cpu(cpuid);
> >>>>>>>>>>>> +               if (cpu < 0)
> >>>>>>>>>>>> +                       continue;
> >>>>>>>>>>>>
> >>>>>>>>>>>>                       num_processors++;
> >>>>>>>>>>>>                       set_cpu_possible(cpu, true);
> >>>>>>>>>>>> @@ -302,7 +301,7 @@ static void __init fdt_smp_setup(void)
> >>>>>>>>>>>>                       __cpu_number_map[cpuid] = cpu;
> >>>>>>>>>>>>                       __cpu_logical_map[cpu] = cpuid;
> >>>>>>>>>>>>
> >>>>>>>>>>>> -               early_numa_add_cpu(cpu, 0);
> >>>>>>>>>>>> +               early_numa_add_cpu(cpuid, 0);
> >>>>>>>>>>>>                       set_cpuid_to_node(cpuid, 0);
> >>>>>>>>>>>>               }
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> >>>>>>>>>>>> --
> >>>>>>>>>>>> 2.39.3
> >>>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>
>