[v1] x86/cpu/topology: remove limit of CPUs due to noapic on x86_64

[PATCH RESEND] x86/cpu/topology: remove limit of CPUs due to noapic on x86_64

Posted by Fernando Fernandez Mancera 1 year, 2 months ago

On x86_64 the command line parameter "noapic" should not limit the
number of possible CPUs, as it only limits the use of IRQ sharing or
device IRQ remapping. Only on x86_32 the command line parameter
"nolapic" limits the number of possible CPUs to one. This restores the
behavior previous to the rework of possible CPU management.

Fixes: 7c0edad3643f ("x86/cpu/topology: Rework possible CPU management")
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
RESEND: original patch https://lkml.org/lkml/2024/9/7/160
---
 arch/x86/kernel/cpu/topology.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
index 621a151ccf7d..5f10a010e35a 100644
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -428,10 +428,16 @@ void __init topology_apply_cmdline_limits_early(void)
 {
 	unsigned int possible = nr_cpu_ids;
 
-	/* 'maxcpus=0' 'nosmp' 'nolapic' 'disableapic' 'noapic' */
-	if (!setup_max_cpus || ioapic_is_disabled || apic_is_disabled)
+	/* 'maxcpus=0' 'nosmp' */
+	if (!setup_max_cpus)
 		possible = 1;
 
+#if defined(CONFIG_X86_32)
+	/* 'nolapic' 'disableapic' 'noapic' */
+	if (apic_is_disabled || ioapic_is_disabled)
+		possible = 1;
+#endif
+
 	/* 'possible_cpus=N' */
 	possible = min_t(unsigned int, max_possible_cpus, possible);
 
@@ -443,8 +449,14 @@ void __init topology_apply_cmdline_limits_early(void)
 
 static __init bool restrict_to_up(void)
 {
-	if (!smp_found_config || ioapic_is_disabled)
+	if (!smp_found_config)
 		return true;
+
+#if defined(CONFIG_X86_32)
+	if (ioapic_is_disabled)
+		return true;
+#endif
+
 	/*
 	 * XEN PV is special as it does not advertise the local APIC
 	 * properly, but provides a fake topology for it so that the
-- 
2.46.2

Re: [PATCH RESEND] x86/cpu/topology: remove limit of CPUs due to noapic on x86_64

Posted by Thomas Gleixner 1 year, 2 months ago

On Sun, Nov 24 2024 at 18:45, Fernando Fernandez Mancera wrote:
> On x86_64 the command line parameter "noapic" should not limit the
> number of possible CPUs, as it only limits the use of IRQ sharing or
> device IRQ remapping. Only on x86_32 the command line parameter
> "nolapic" limits the number of possible CPUs to one. This restores the
> behavior previous to the rework of possible CPU management.
>
> Fixes: 7c0edad3643f ("x86/cpu/topology: Rework possible CPU management")
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
> ---
> RESEND: original patch https://lkml.org/lkml/2024/9/7/160

Please use https://lore.kernel.org/all/$MSGID references.

> ---
>  arch/x86/kernel/cpu/topology.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
> index 621a151ccf7d..5f10a010e35a 100644
> --- a/arch/x86/kernel/cpu/topology.c
> +++ b/arch/x86/kernel/cpu/topology.c
> @@ -428,10 +428,16 @@ void __init topology_apply_cmdline_limits_early(void)
>  {
>  	unsigned int possible = nr_cpu_ids;
>  
> -	/* 'maxcpus=0' 'nosmp' 'nolapic' 'disableapic' 'noapic' */
> -	if (!setup_max_cpus || ioapic_is_disabled || apic_is_disabled)
> +	/* 'maxcpus=0' 'nosmp' */
> +	if (!setup_max_cpus)

That's wrong. If the local APIC is disabled, SMP is not possible.

>  		possible = 1;
>  
> +#if defined(CONFIG_X86_32)
> +	/* 'nolapic' 'disableapic' 'noapic' */
> +	if (apic_is_disabled || ioapic_is_disabled)

I double checked the original behaviour. A disabled IOAPIC did not
prevent SMP on 32bit either.

> +		possible = 1;
> +#endif

So the condition wants to be:

	if (!setup_max_cpus || apic_is_disabled)


>  	/* 'possible_cpus=N' */
>  	possible = min_t(unsigned int, max_possible_cpus, possible);
>  
> @@ -443,8 +449,14 @@ void __init topology_apply_cmdline_limits_early(void)
>  
>  static __init bool restrict_to_up(void)
>  {
> -	if (!smp_found_config || ioapic_is_disabled)
> +	if (!smp_found_config)
>  		return true;
> +
> +#if defined(CONFIG_X86_32)
> +	if (ioapic_is_disabled)
> +		return true;
> +#endif

That wants to go away too.

Thanks,

        tglx

Re: [PATCH RESEND] x86/cpu/topology: remove limit of CPUs due to noapic on x86_64

Posted by Ingo Molnar 1 year, 2 months ago

* Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:

> On x86_64 the command line parameter "noapic" should not limit the
> number of possible CPUs, as it only limits the use of IRQ sharing or
> device IRQ remapping. Only on x86_32 the command line parameter
> "nolapic" limits the number of possible CPUs to one. This restores the
> behavior previous to the rework of possible CPU management.

So what's the motivation? Arguably the x86-64 boot option behavior was 
weird: a working local APIC is very much needed to have an SMP system.

If we want to disable IRQ sharing or device IRQ remapping, then that 
should have an appropriately named boot command line option. Does some 
system require that perhaps?

Thanks,

	Ingo

Re: [PATCH RESEND] x86/cpu/topology: remove limit of CPUs due to noapic on x86_64

Posted by Fernando F. Mancera 1 year, 2 months ago

On 25/11/2024 09:37, Ingo Molnar wrote:
> 
> * Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:
> 
>> On x86_64 the command line parameter "noapic" should not limit the
>> number of possible CPUs, as it only limits the use of IRQ sharing or
>> device IRQ remapping. Only on x86_32 the command line parameter
>> "nolapic" limits the number of possible CPUs to one. This restores the
>> behavior previous to the rework of possible CPU management.
> 
> So what's the motivation? Arguably the x86-64 boot option behavior was
> weird: a working local APIC is very much needed to have an SMP system.
> 

Sorry if I am wrong here but I am not an expert on the matter. I 
believed that "noapic" disables I/O APIC which handles external 
interrupts while the local APICs are still enabled as they are managed 
by "nolapic". If that is the case, SMP should still be possible.

If both I/O APIC and Local APICs are disabled then the kernel should 
fallback to a single CPU mode. That is the behavior that kernel has with 
my patch.

The motivation is to fix multiple users with systems that requires 
"noapic" to work and after the rework their systems are using a single CPU.

References:
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2295026
[2] 
https://discussion.fedoraproject.org/t/fedora-sees-only-1-cpu-core-after-updating-the-kernel-from-6-8-x-to-6-9-x/121385/17

> If we want to disable IRQ sharing or device IRQ remapping, then that
> should have an appropriately named boot command line option. Does some
> system require that perhaps?
> 
> Thanks,
> 
> 	Ingo

Re: [PATCH RESEND] x86/cpu/topology: remove limit of CPUs due to noapic on x86_64

Posted by Thomas Gleixner 1 year, 2 months ago

On Mon, Nov 25 2024 at 16:13, Fernando F. Mancera wrote:
> On 25/11/2024 09:37, Ingo Molnar wrote:
>> So what's the motivation? Arguably the x86-64 boot option behavior was
>> weird: a working local APIC is very much needed to have an SMP system.
>> 
>
> Sorry if I am wrong here but I am not an expert on the matter. I 
> believed that "noapic" disables I/O APIC which handles external 
> interrupts while the local APICs are still enabled as they are managed 
> by "nolapic". If that is the case, SMP should still be possible.
>
> If both I/O APIC and Local APICs are disabled then the kernel should 
> fallback to a single CPU mode. That is the behavior that kernel has with 
> my patch.
>
> The motivation is to fix multiple users with systems that requires 
> "noapic" to work and after the rework their systems are using a single CPU.

Sorry, my bad. I messed up the "noapic" option handling.