x86: Two fixes related to Xen PV guest mode

[PATCH 1/2] x86/cpu: fix BSP detection when running as Xen PV guest

Posted by Juergen Gross 1 year, 6 months ago

When booting as a Xen PV guest the boot processor isn't detected
correctly and the following message is shown:

  CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1

Additionally this results in one CPU being ignored.

Fix that by calling the BSP detection logic when registering the boot
CPU's APIC, too.

Fixes: 5c5682b9f87a ("x86/cpu: Detect real BSP on crash kernels")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/kernel/cpu/topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
index aaca8d235dc2..23c3db5e6396 100644
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -255,7 +255,7 @@ void __init topology_register_boot_apic(u32 apic_id)
 	WARN_ON_ONCE(topo_info.boot_cpu_apic_id != BAD_APICID);
 
 	topo_info.boot_cpu_apic_id = apic_id;
-	topo_register_apic(apic_id, CPU_ACPIID_INVALID, true);
+	topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);
 }
 
 /**
-- 
2.35.3

Re: [PATCH 1/2] x86/cpu: fix BSP detection when running as Xen PV guest

Posted by Thomas Gleixner 1 year, 5 months ago

On Fri, Apr 05 2024 at 14:34, Juergen Gross wrote:
> When booting as a Xen PV guest the boot processor isn't detected
> correctly and the following message is shown:
>
>   CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1
>
> Additionally this results in one CPU being ignored.
>
> Fix that by calling the BSP detection logic when registering the boot
> CPU's APIC, too.
>
> Fixes: 5c5682b9f87a ("x86/cpu: Detect real BSP on crash kernels")
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/kernel/cpu/topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
> index aaca8d235dc2..23c3db5e6396 100644
> --- a/arch/x86/kernel/cpu/topology.c
> +++ b/arch/x86/kernel/cpu/topology.c
> @@ -255,7 +255,7 @@ void __init topology_register_boot_apic(u32 apic_id)
>  	WARN_ON_ONCE(topo_info.boot_cpu_apic_id != BAD_APICID);
>  
>  	topo_info.boot_cpu_apic_id = apic_id;
> -	topo_register_apic(apic_id, CPU_ACPIID_INVALID, true);
> +	topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);

No. This does not fix anything at all. It just papers over the
underlying problem.

Thanks,

        tglx
---
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index 27d1a5b7f571..ac41d83b38d3 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -154,9 +154,9 @@ static void __init xen_pv_smp_config(void)
 	u32 apicid = 0;
 	int i;
 
-	topology_register_boot_apic(apicid++);
+	topology_register_boot_apic(apicid);
 
-	for (i = 1; i < nr_cpu_ids; i++)
+	for (i = 0; i < nr_cpu_ids; i++)
 		topology_register_apic(apicid++, CPU_ACPIID_INVALID, true);
 
 	/* Pretend to be a proper enumerated system */

Re: [PATCH 1/2] x86/cpu: fix BSP detection when running as Xen PV guest

Posted by Jürgen Groß 1 year, 5 months ago

On 30.04.24 18:13, Thomas Gleixner wrote:
> On Fri, Apr 05 2024 at 14:34, Juergen Gross wrote:
>> When booting as a Xen PV guest the boot processor isn't detected
>> correctly and the following message is shown:
>>
>>    CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1
>>
>> Additionally this results in one CPU being ignored.
>>
>> Fix that by calling the BSP detection logic when registering the boot
>> CPU's APIC, too.
>>
>> Fixes: 5c5682b9f87a ("x86/cpu: Detect real BSP on crash kernels")
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/kernel/cpu/topology.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
>> index aaca8d235dc2..23c3db5e6396 100644
>> --- a/arch/x86/kernel/cpu/topology.c
>> +++ b/arch/x86/kernel/cpu/topology.c
>> @@ -255,7 +255,7 @@ void __init topology_register_boot_apic(u32 apic_id)
>>   	WARN_ON_ONCE(topo_info.boot_cpu_apic_id != BAD_APICID);
>>   
>>   	topo_info.boot_cpu_apic_id = apic_id;
>> -	topo_register_apic(apic_id, CPU_ACPIID_INVALID, true);
>> +	topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);
> 
> No. This does not fix anything at all. It just papers over the
> underlying problem.
> 
> Thanks,
> 
>          tglx
> ---
> diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
> index 27d1a5b7f571..ac41d83b38d3 100644
> --- a/arch/x86/xen/smp_pv.c
> +++ b/arch/x86/xen/smp_pv.c
> @@ -154,9 +154,9 @@ static void __init xen_pv_smp_config(void)
>   	u32 apicid = 0;
>   	int i;
>   
> -	topology_register_boot_apic(apicid++);
> +	topology_register_boot_apic(apicid);
>   
> -	for (i = 1; i < nr_cpu_ids; i++)
> +	for (i = 0; i < nr_cpu_ids; i++)
>   		topology_register_apic(apicid++, CPU_ACPIID_INVALID, true);
>   
>   	/* Pretend to be a proper enumerated system */
> 
>          
> 

Thanks, works great.

Do you want it to send as your patch, or should I add your Signed-off-by or
your Suggested-by?


Juergen

[PATCH] x86/xen/smp_pv: Register the boot CPU APIC properly

Posted by Thomas Gleixner 1 year, 5 months ago

The topology core expects the boot APIC to be registered from earhy APIC
detection first and then again when the firmware tables are evaluated. This
is used for detecting the real BSP CPU on a kexec kernel.

The recent conversion of XEN/PV to register fake APIC IDs failed to
register the boot CPU APIC correctly as it only registers it once. This
causes the BSP detection mechanism to trigger wrongly:

   CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1

Additionally this results in one CPU being ignored.

Register the boot CPU APIC twice so that the XEN/PV fake enumeration
behaves like real firmware.

Reported-by: Juergen Gross <jgross@suse.com>
Fixes: e75307023466 ("x86/xen/smp_pv: Register fake APICs")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/smp_pv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -154,9 +154,9 @@ static void __init xen_pv_smp_config(voi
 	u32 apicid = 0;
 	int i;
 
-	topology_register_boot_apic(apicid++);
+	topology_register_boot_apic(apicid);
 
-	for (i = 1; i < nr_cpu_ids; i++)
+	for (i = 0; i < nr_cpu_ids; i++)
 		topology_register_apic(apicid++, CPU_ACPIID_INVALID, true);
 
 	/* Pretend to be a proper enumerated system */

Re: [PATCH] x86/xen/smp_pv: Register the boot CPU APIC properly

Posted by Jürgen Groß 1 year, 5 months ago

On 02.05.24 16:39, Thomas Gleixner wrote:
> The topology core expects the boot APIC to be registered from earhy APIC
> detection first and then again when the firmware tables are evaluated. This
> is used for detecting the real BSP CPU on a kexec kernel.
> 
> The recent conversion of XEN/PV to register fake APIC IDs failed to
> register the boot CPU APIC correctly as it only registers it once. This
> causes the BSP detection mechanism to trigger wrongly:
> 
>     CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1
> 
> Additionally this results in one CPU being ignored.
> 
> Register the boot CPU APIC twice so that the XEN/PV fake enumeration
> behaves like real firmware.
> 
> Reported-by: Juergen Gross <jgross@suse.com>
> Fixes: e75307023466 ("x86/xen/smp_pv: Register fake APICs")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Juergen Gross <jgross@suse.com>

Thanks for the patch, I'll take it via the Xen tree.


Juergen

> ---
>   arch/x86/xen/smp_pv.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/xen/smp_pv.c
> +++ b/arch/x86/xen/smp_pv.c
> @@ -154,9 +154,9 @@ static void __init xen_pv_smp_config(voi
>   	u32 apicid = 0;
>   	int i;
>   
> -	topology_register_boot_apic(apicid++);
> +	topology_register_boot_apic(apicid);
>   
> -	for (i = 1; i < nr_cpu_ids; i++)
> +	for (i = 0; i < nr_cpu_ids; i++)
>   		topology_register_apic(apicid++, CPU_ACPIID_INVALID, true);
>   
>   	/* Pretend to be a proper enumerated system */

Re: [PATCH 1/2] x86/cpu: fix BSP detection when running as Xen PV guest

Posted by Juergen Gross 1 year, 6 months ago

Ping?

On 05.04.24 14:34, Juergen Gross wrote:
> When booting as a Xen PV guest the boot processor isn't detected
> correctly and the following message is shown:
> 
>    CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1
> 
> Additionally this results in one CPU being ignored.
> 
> Fix that by calling the BSP detection logic when registering the boot
> CPU's APIC, too.
> 
> Fixes: 5c5682b9f87a ("x86/cpu: Detect real BSP on crash kernels")
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>   arch/x86/kernel/cpu/topology.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
> index aaca8d235dc2..23c3db5e6396 100644
> --- a/arch/x86/kernel/cpu/topology.c
> +++ b/arch/x86/kernel/cpu/topology.c
> @@ -255,7 +255,7 @@ void __init topology_register_boot_apic(u32 apic_id)
>   	WARN_ON_ONCE(topo_info.boot_cpu_apic_id != BAD_APICID);
>   
>   	topo_info.boot_cpu_apic_id = apic_id;
> -	topo_register_apic(apic_id, CPU_ACPIID_INVALID, true);
> +	topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);
>   }
>   
>   /**

Re: [PATCH 1/2] x86/cpu: fix BSP detection when running as Xen PV guest

Posted by Juergen Gross 1 year, 5 months ago

PING!!!!

This patch fixes a regression introduced in 6.9-rc1.

Please consider taking the patch in the 6.9 cycle!

#regzbot ^introduced: 5c5682b9f87a


Juergen

On 19.04.24 13:52, Juergen Gross wrote:
> Ping?
> 
> On 05.04.24 14:34, Juergen Gross wrote:
>> When booting as a Xen PV guest the boot processor isn't detected
>> correctly and the following message is shown:
>>
>>    CPU topo: Boot CPU APIC ID not the first enumerated APIC ID: 0 > 1
>>
>> Additionally this results in one CPU being ignored.
>>
>> Fix that by calling the BSP detection logic when registering the boot
>> CPU's APIC, too.
>>
>> Fixes: 5c5682b9f87a ("x86/cpu: Detect real BSP on crash kernels")
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/kernel/cpu/topology.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
>> index aaca8d235dc2..23c3db5e6396 100644
>> --- a/arch/x86/kernel/cpu/topology.c
>> +++ b/arch/x86/kernel/cpu/topology.c
>> @@ -255,7 +255,7 @@ void __init topology_register_boot_apic(u32 apic_id)
>>       WARN_ON_ONCE(topo_info.boot_cpu_apic_id != BAD_APICID);
>>       topo_info.boot_cpu_apic_id = apic_id;
>> -    topo_register_apic(apic_id, CPU_ACPIID_INVALID, true);
>> +    topology_register_apic(apic_id, CPU_ACPIID_INVALID, true);
>>   }
>>   /**
>