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

[Borislav Petkov]

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@xxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
>  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?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette