2023-07-19 05:46:22

by Vasant Hegde

[permalink] [raw]
Subject: [PATCH] x86/smp: Validate APIC ID before parking CPU in INIT

Below commit is causing kexec to hang in certain scenarios with >255 CPUs.

Reproduce steps:
- We are using 2 socket system with 384 CPUs
- Booting first kernel with kernel command line intremap=off
This disabled x2apic in kernel and booted with apic mode
- During kexec it tries to send INIT to all CPUs except boot CPU
If APIC ID is 0x100 (like in our case) then it will send CPU0
to INIT mode and system hangs (in APIC mode DEST field is 8bit)

Fix this issue by adding apic->apic_id_valid() check before sending
INIT sequence.

Fixes: 45e34c8af58f ("x86/smp: Put CPUs into INIT on shutdown if possible")
Reported-by: Dheeraj Kumar Srivastava <[email protected]>
Tested-by: Dheeraj Kumar Srivastava <[email protected]>
Signed-off-by: Vasant Hegde <[email protected]>
---
arch/x86/kernel/smpboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e1aa2cd7734b..e5ca0689c4dd 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1360,7 +1360,7 @@ bool smp_park_other_cpus_in_init(void)
if (cpu == this_cpu)
continue;
apicid = apic->cpu_present_to_apicid(cpu);
- if (apicid == BAD_APICID)
+ if (apicid == BAD_APICID || !apic->apic_id_valid(apicid))
continue;
send_init_sequence(apicid);
}
--
2.31.1



2023-08-03 17:27:41

by Vasant Hegde

[permalink] [raw]
Subject: Re: [PATCH] x86/smp: Validate APIC ID before parking CPU in INIT

Hi

Did you get a chance to look into this patch?


-Vasant

On 7/19/2023 10:43 AM, Vasant Hegde wrote:
> Below commit is causing kexec to hang in certain scenarios with >255 CPUs.
>
> Reproduce steps:
> - We are using 2 socket system with 384 CPUs
> - Booting first kernel with kernel command line intremap=off
> This disabled x2apic in kernel and booted with apic mode
> - During kexec it tries to send INIT to all CPUs except boot CPU
> If APIC ID is 0x100 (like in our case) then it will send CPU0
> to INIT mode and system hangs (in APIC mode DEST field is 8bit)
>
> Fix this issue by adding apic->apic_id_valid() check before sending
> INIT sequence.
>
> Fixes: 45e34c8af58f ("x86/smp: Put CPUs into INIT on shutdown if possible")
> Reported-by: Dheeraj Kumar Srivastava <[email protected]>
> Tested-by: Dheeraj Kumar Srivastava <[email protected]>
> Signed-off-by: Vasant Hegde <[email protected]>
> ---
> arch/x86/kernel/smpboot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index e1aa2cd7734b..e5ca0689c4dd 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1360,7 +1360,7 @@ bool smp_park_other_cpus_in_init(void)
> if (cpu == this_cpu)
> continue;
> apicid = apic->cpu_present_to_apicid(cpu);
> - if (apicid == BAD_APICID)
> + if (apicid == BAD_APICID || !apic->apic_id_valid(apicid))
> continue;
> send_init_sequence(apicid);
> }

2023-08-09 19:36:12

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] x86/smp: Validate APIC ID before parking CPU in INIT

On Wed, Aug 09 2023 at 20:42, Thomas Gleixner wrote:
> On Wed, Jul 19 2023 at 05:13, Vasant Hegde wrote:
>> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
>> index e1aa2cd7734b..e5ca0689c4dd 100644
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -1360,7 +1360,7 @@ bool smp_park_other_cpus_in_init(void)
>> if (cpu == this_cpu)
>> continue;
>> apicid = apic->cpu_present_to_apicid(cpu);
>> - if (apicid == BAD_APICID)
>> + if (apicid == BAD_APICID || !apic->apic_id_valid(apicid))
>> continue;
>> send_init_sequence(apicid);
>> }

I think this papers over the underlying problem that this sends INIT to
an APIC which was never booted. The below is curing the root cause.

Thanks,

tglx
---
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1356,7 +1356,7 @@ bool smp_park_other_cpus_in_init(void)
if (this_cpu)
return false;

- for_each_present_cpu(cpu) {
+ for_each_cpu_and(cpu, &cpus_booted_once_mask, cpu_present_mask) {
if (cpu == this_cpu)
continue;
apicid = apic->cpu_present_to_apicid(cpu);

2023-08-09 19:54:00

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] x86/smp: Validate APIC ID before parking CPU in INIT

On Wed, Jul 19 2023 at 05:13, Vasant Hegde wrote:
> Below commit is causing kexec to hang in certain scenarios with >255 CPUs.
>
> Reproduce steps:
> - We are using 2 socket system with 384 CPUs
> - Booting first kernel with kernel command line intremap=off
> This disabled x2apic in kernel and booted with apic mode
> - During kexec it tries to send INIT to all CPUs except boot CPU
> If APIC ID is 0x100 (like in our case) then it will send CPU0
> to INIT mode and system hangs (in APIC mode DEST field is 8bit)

It took me a while to decode the above.

> Fix this issue by adding apic->apic_id_valid() check before sending
> INIT sequence.

Sigh, yes.

> Fixes: 45e34c8af58f ("x86/smp: Put CPUs into INIT on shutdown if possible")
> Reported-by: Dheeraj Kumar Srivastava <[email protected]>
> Tested-by: Dheeraj Kumar Srivastava <[email protected]>
> Signed-off-by: Vasant Hegde <[email protected]>
> ---
> arch/x86/kernel/smpboot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index e1aa2cd7734b..e5ca0689c4dd 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1360,7 +1360,7 @@ bool smp_park_other_cpus_in_init(void)
> if (cpu == this_cpu)
> continue;
> apicid = apic->cpu_present_to_apicid(cpu);
> - if (apicid == BAD_APICID)
> + if (apicid == BAD_APICID || !apic->apic_id_valid(apicid))
> continue;
> send_init_sequence(apicid);
> }

2023-08-10 11:42:54

by Vasant Hegde

[permalink] [raw]
Subject: Re: [PATCH] x86/smp: Validate APIC ID before parking CPU in INIT

Hi,


On 8/10/2023 12:22 AM, Thomas Gleixner wrote:
> On Wed, Aug 09 2023 at 20:42, Thomas Gleixner wrote:
>> On Wed, Jul 19 2023 at 05:13, Vasant Hegde wrote:
>>> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
>>> index e1aa2cd7734b..e5ca0689c4dd 100644
>>> --- a/arch/x86/kernel/smpboot.c
>>> +++ b/arch/x86/kernel/smpboot.c
>>> @@ -1360,7 +1360,7 @@ bool smp_park_other_cpus_in_init(void)
>>> if (cpu == this_cpu)
>>> continue;
>>> apicid = apic->cpu_present_to_apicid(cpu);
>>> - if (apicid == BAD_APICID)
>>> + if (apicid == BAD_APICID || !apic->apic_id_valid(apicid))
>>> continue;
>>> send_init_sequence(apicid);
>>> }
>
> I think this papers over the underlying problem that this sends INIT to
> an APIC which was never booted. The below is curing the root cause.

I have tested below patch and it fixes the issue. Thanks

Tested-by: Vasant Hegde <[email protected]>

-Vasant

>
> Thanks,
>
> tglx
> ---
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1356,7 +1356,7 @@ bool smp_park_other_cpus_in_init(void)
> if (this_cpu)
> return false;
>
> - for_each_present_cpu(cpu) {
> + for_each_cpu_and(cpu, &cpus_booted_once_mask, cpu_present_mask) {
> if (cpu == this_cpu)
> continue;
> apicid = apic->cpu_present_to_apicid(cpu);