Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 4 Aug 2023 20:12:43 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>, x86@kernel.org,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Andrew Cooper <andrew.cooper3@citrix.com>,
        Arjan van de Ven <arjan@linux.intel.com>,
        Huang Rui <ray.huang@amd.com>, Juergen Gross <jgross@suse.com>,
        Dimitri Sivanich <dimitri.sivanich@hpe.com>,
        Michael Kelley <mikelley@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>
Subject: Re: [patch V3 02/40] x86/apic: Fake primary thread mask for XEN/PV
Message-ID: <20230804181243.GLZM0/m0xXb/wMpUcD@fat_crate.local>
References: <20230802101635.459108805@linutronix.de>
 <20230802101932.819070039@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230802101932.819070039@linutronix.de>
Precedence: bulk

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?

-- 
Regards/Gruss,
    Boris.

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