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

["Kalra, Ashish" <ashish.kalra@amd.com>]

On 6/30/2026 1:32 PM, sashiko-bot@kernel.org wrote:
> 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.
> 
> [ ... ]
> 

You're right about the mechanism — cpu_hotplug_disable() doesn't cover the suspend freeze/thaw path, and snp_enable() has no resume hook. But an SNP
host doesn't support suspend/resume: SNP is set up once at boot (SnpEn on all CPUs followed by SNP_INIT), and that state can't be rebuilt on resume
without a full re-init. So this can't arise in practice, and it's pre-existing base-SNP behavior, not something this patch introduces.

Thanks,
Ashish