| 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
| ||
|
Message-ID: <CAAhV-H4wD5fGVgxwmRVpRgvQ-jyUY0t=ewJANbe50vj9_TZDUQ@mail.gmail.com>
Date: Tue, 22 Oct 2024 09:31:50 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: maobibo <maobibo@...ngson.cn>
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 Tue, Oct 22, 2024 at 9:17 AM maobibo <maobibo@...ngson.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@...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