Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active

On Tue, Jun 30, 2026 at 06:11:03PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <[email protected]>
> 
> While SNP is active, every memory write is checked against the RMP to
> protect SEV-SNP guest memory.  A core performs these RMP checks only once
> ...

Use this commit message for your next revision. The idea is to split it into
smaller paragraphs for easier parsing, do simple formulations and not talk
about future patches because git history is not always linear:

"While SNP is active, every memory write is checked against the RMP to
protect SEV-SNP guest memory.  A core performs these RMP checks only once SNP
has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is set on
that core; the firmware requires the SNP-enable bit to be set on every present
CPU before SNP initialization.

A core that is not SNP-enabled and not SNP-initialized performs no RMP checks
at all, so there is no valid configuration with SNP active and any CPU exempt
from RMP checks.

The firmware determines which CPUs are present from the processor and the
BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates them at
SNP init; it is not aware of the OS bringing CPUs online or offline
afterwards.

SNP_INIT fails unless SnpEn is set on all CPUs, so a CPU that is offline when
SNP_INIT is issued, does not have SnpEn set, SNP_INIT fails, and there can be
no SNP guest memory.  OS CPU hotplug can thus diverge from the firmware's
expectations and break SNP.

Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
disabled SNP.

If snp_prepare() fails before enabling SNP it re-enables hotplug itself; once
SNP is enabled hotplug stays disabled, including across a failed SNP_INIT and
across the legacy SNP_SHUTDOWN_EX path, both of which leave SNP enabled.

A kexec target that boots with SNP already enabled, disables hotplug once in
snp_rmptable_init(), since snp_prepare() bails when SNP is already enabled."

> 
> Suggested-by: Thomas Lendacky <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> ---
>  arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index dab6e1c290bc..04a58ac4339c 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -535,6 +535,15 @@ int snp_prepare(void)
>  
>  	clear_rmp();
>  
> +	/*
> +	 * Disable CPU hotplug before enabling SNP, so no CPU can come online
> +	 * without SnpEn while SNP is enabled; it is re-enabled in snp_shutdown()
> +	 * once SNP is disabled.  Must be before cpus_read_lock():
> +	 * cpu_hotplug_disable() takes cpu_add_remove_lock, which nests above
> +	 * cpu_hotplug_lock.
> +	 */
> +	cpu_hotplug_disable();
> +
>  	cpus_read_lock();

What's the point of grabbing the hotplug lock if you just disabled hotplug?

>  	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
> @@ -560,6 +569,10 @@ int snp_prepare(void)
>  unlock:
>  	cpus_read_unlock();

ditto.

>  
> +	/* Re-enable CPU hotplug; SnpEn was never set. */


^ Superfluous comment.

> +	if (ret)
> +		cpu_hotplug_enable();
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
> @@ -587,6 +600,13 @@ void snp_shutdown(void)
>  
>  	rmpopt_cleanup();
>  
> +	/*
> +	 * Re-enable CPU hotplug now that the firmware has disabled SNP; CPU
> +	 * hotplug is not re-enabled for a legacy SNP shutdown.  After
> +	 * rmpopt_cleanup() so RMPOPT_BASE is cleared with hotplug still disabled.
> +	 */

I can't parse that comment.

> +	cpu_hotplug_enable();
> +
>  	clear_rmp();
>  	on_each_cpu(mfd_reconfigure, NULL, 1);
>  }

-- 
Regards/Gruss,
    Boris.

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