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

From: Borislav Petkov
Date: Fri Aug 04 2023 - 14:13:27 EST


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