lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8c55c680-48c8-0ba3-c2a1-56dc72929a8d@loongson.cn>
Date: Tue, 22 Oct 2024 09:17:05 +0800
From: maobibo <maobibo@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Jianmin Lv <lvjianmin@...ngson.cn>, loongarch@...ts.linux.dev,
 linux-kernel@...r.kernel.org, lixianglai@...ngson.cn,
 WANG Xuerui <kernel@...0n.name>
Subject: Re: [PATCH v2] LoongArch: Fix cpu hotplug issue



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@...ngson.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@...ngson.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
>>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ