diff mbox series

[V3,02/40] x86/apic: Fake primary thread mask for XEN/PV

Message ID 20230802101932.819070039@linutronix.de
State New
Headers
Series x86/cpu: Rework the topology evaluation |

Commit Message

Thomas Gleixner Aug. 2, 2023, 10:21 a.m. UTC 
  The SMT control mechanism got added as speculation attack vector
mitigation. The implemented logic relies on the primary thread mask to
be set up properly.

This turns out to be an issue with XEN/PV guests because their CPU hotplug
mechanics do not enumerate APICs and therefore the mask is never correctly
populated.

This went unnoticed so far because by chance XEN/PV ends up with
smp_num_siblings == 2. So smt_hot-plug_control stays at its default value
CPU_SMT_ENABLED and the primary thread mask is never evaluated in the
context of CPU hotplug.

This stopped "working" with the upcoming overhaul of the topology
evaluation which legitimately provides a fake topology for XEN/PV. That
sets smp_num_siblings to 1, which causes the core CPU hot-plug core to
refuse to bring up the APs.

This happens because smt_hotplug_control is set to CPU_SMT_NOT_SUPPORTED
which causes cpu_smt_allowed() to evaluate the unpopulated primary thread
mask with the conclusion that all non-boot CPUs are not valid to be
plugged.

The core code has already been made more robust against this kind of fail,
but the primary thread mask really wants to be populated to avoid other
issues all over the place.

Just fake the mask by pretending that all XEN/PV vCPUs are primary threads,
which is consistent because all of XEN/PVs topology is fake or non-existent.

Fixes: 6a4d2657e048 ("x86/smp: Provide topology_is_primary_thread()")
Fixes: f54d4434c281 ("x86/apic: Provide cpu_primary_thread mask")
Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/apic/apic.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Borislav Petkov Aug. 4, 2023, 6:12 p.m. UTC | #1 
On Wed, Aug 02, 2023 at 12:21:01PM +0200, Thomas Gleixner wrote:
> The SMT control mechanism got added as speculation attack vector
> mitigation. The implemented logic relies on the primary thread mask to
> be set up properly.
> 
> This turns out to be an issue with XEN/PV guests because their CPU hotplug
> mechanics do not enumerate APICs and therefore the mask is never correctly
> populated.
> 
> This went unnoticed so far because by chance XEN/PV ends up with
> smp_num_siblings == 2. So smt_hot-plug_control stays at its default value

I think this means to say "cpu_smt_control". Committer, pls fix. :-)

> CPU_SMT_ENABLED and the primary thread mask is never evaluated in the
> context of CPU hotplug.
> 
> This stopped "working" with the upcoming overhaul of the topology
> evaluation which legitimately provides a fake topology for XEN/PV. That
> sets smp_num_siblings to 1, which causes the core CPU hot-plug core to
> refuse to bring up the APs.
> 
> This happens because smt_hotplug_control is set to CPU_SMT_NOT_SUPPORTED

Ditto.

> which causes cpu_smt_allowed() to evaluate the unpopulated primary thread
> mask with the conclusion that all non-boot CPUs are not valid to be
> plugged.
> 
> The core code has already been made more robust against this kind of fail,
> but the primary thread mask really wants to be populated to avoid other
> issues all over the place.
> 
> Just fake the mask by pretending that all XEN/PV vCPUs are primary threads,
> which is consistent because all of XEN/PVs topology is fake or non-existent.
> 
> Fixes: 6a4d2657e048 ("x86/smp: Provide topology_is_primary_thread()")
> Fixes: f54d4434c281 ("x86/apic: Provide cpu_primary_thread mask")
> Reported-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/kernel/apic/apic.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -36,6 +36,8 @@
>  #include <linux/smp.h>
>  #include <linux/mm.h>
>  
> +#include <xen/xen.h>
> +
>  #include <asm/trace/irq_vectors.h>
>  #include <asm/irq_remapping.h>
>  #include <asm/pc-conf-reg.h>
> @@ -2344,6 +2346,15 @@ static int __init smp_init_primary_threa
>  {
>  	unsigned int cpu;
>  
> +	/*
> +	 * XEN/PV provides either none or useless topology information.
> +	 * Pretend that all vCPUs are primary threads.
> +	 */
> +	if (xen_pv_domain()) {
> +		cpumask_copy(&__cpu_primary_thread_mask, cpu_possible_mask);
> +		return 0;
> +	}

Can this be somewhere in the Xen init code instead?
Thomas Gleixner Aug. 4, 2023, 8:02 p.m. UTC | #2 
On Fri, Aug 04 2023 at 20:12, Borislav Petkov wrote:
>> @@ -2344,6 +2346,15 @@ static int __init smp_init_primary_threa
>>  {
>>  	unsigned int cpu;
>>  
>> +	/*
>> +	 * XEN/PV provides either none or useless topology information.
>> +	 * Pretend that all vCPUs are primary threads.
>> +	 */
>> +	if (xen_pv_domain()) {
>> +		cpumask_copy(&__cpu_primary_thread_mask, cpu_possible_mask);
>> +		return 0;
>> +	}
>
> Can this be somewhere in the Xen init code instead?

Not for now. That's all going away with the 3rd installment. But right
now it's the right place to be.
diff mbox series

Patch

--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -36,6 +36,8 @@ 
 #include <linux/smp.h>
 #include <linux/mm.h>
 
+#include <xen/xen.h>
+
 #include <asm/trace/irq_vectors.h>
 #include <asm/irq_remapping.h>
 #include <asm/pc-conf-reg.h>
@@ -2344,6 +2346,15 @@  static int __init smp_init_primary_threa
 {
 	unsigned int cpu;
 
+	/*
+	 * XEN/PV provides either none or useless topology information.
+	 * Pretend that all vCPUs are primary threads.
+	 */
+	if (xen_pv_domain()) {
+		cpumask_copy(&__cpu_primary_thread_mask, cpu_possible_mask);
+		return 0;
+	}
+
 	for (cpu = 0; cpu < nr_logical_cpuids; cpu++)
 		cpu_mark_primary_thread(cpu, cpuid_to_apicid[cpu]);
 	return 0;