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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] System suspend (PM transitions) bypasses `cpu_hotplug_disable()`, stripping `SnpEn` from secondary CPUs on resume.
--

commit 29b8362e9ad72efa8892f6bb3124dbbcc29ee822
Author: Ashish Kalra <ashish.kalra@amd.com>

x86/sev: Disable CPU hotplug while SNP is active

This commit disables CPU hotplug in snp_prepare() before SNP is enabled and
re-enables it in snp_shutdown(). This aims to prevent CPUs from coming online
without the SnpEn bit set while SNP is active, as the firmware cannot track
OS CPU hotplug events.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index dab6e1c290bca..04a58ac4339cb 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();

[Severity: Critical]
Does this actually prevent all CPUs from coming online without SnpEn?

Looking at system suspend (PM transitions), freeze_secondary_cpus() calls
_cpu_down() directly, which bypasses the cpu_hotplug_disabled check. Upon
resume, thaw_secondary_cpus() brings them back online via _cpu_up().

Since snp_enable() is called via a one-off on_each_cpu() in snp_prepare()
rather than being registered as a CPU hotplug callback, wouldn't secondary
CPUs resume with the SnpEn MSR bit cleared? 

This could allow a suspend/resume cycle to create an asymmetrical state where
hardware RMP checks are bypassed.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782841284.git.ashish.kalra@amd.com?part=3