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

On 7/27/2026 2:37 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] System suspend/resume bypasses `cpu_hotplug_disable()`, causing secondary CPUs to come online without the `SnpEn` bit set.
> --
> 
> --- Patch [2]: [PATCH v11 2/6] x86/sev: Disable CPU hotplug while SNP is active ---
> commit 2ca360619d15fed203ba00f43d20453130503df3
> Author: Ashish Kalra <[email protected]>
> 
> x86/sev: Disable CPU hotplug while SNP is active
> 
> This commit replaces cpus_read_lock() with cpu_hotplug_disable() in
> snp_prepare() to prevent CPU hotplug while SEV-SNP is being enabled. It
> additionally keeps hotplug disabled as long as SNP is active to ensure OS CPU
> hotplug does not diverge from firmware expectations.
> 
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index cff285d8ad8e3..e2f69fba09389 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
> 
> [ ... ]
> 
>> @@ -526,14 +525,21 @@ int snp_prepare(void)
>>  
>>  	clear_rmp();
>>  
>> -	cpus_read_lock();
>> +	/*
>> +	 * Disable CPU hotplug before enabling SNP: no CPU may come online
>> +	 * without SnpEn while SNP is active, and none may go offline during
>> +	 * enable.  This keeps cpu_online_mask stable for the check and the
>> +	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
>> +	 * re-enabled in snp_shutdown() once the firmware disables SNP.
>> +	 */
>> +	cpu_hotplug_disable();
> 
> [Severity: High]
> This is a pre-existing issue, but does cpu_hotplug_disable() reliably
> prevent secondary CPUs from coming online without the SnpEn bit set?
> 
> System suspend and resume transitions bypass the sysfs hotplug locks. During a
> suspend and resume cycle, freeze_secondary_cpus() and thaw_secondary_cpus()
> directly invoke internal routines that offline and online CPUs, respectively.
> 
> Because snp_enable() is not registered in a CPU hotplug callback, if a
> secondary CPU is thawed, it appears it would wake up and come online without
> the MSR_AMD64_SYSCFG_SNP_EN bit set. Would this leave the system vulnerable
> by allowing arbitrary host access to SEV-SNP guest memory, since no RMP
> checks would be performed by those CPUs?
> 

Pre-existing base SNP-host behaviour, not introduced here (as tagged). An SNP host doesn't support suspend/resume: 
SnpEn is set on all CPUs once at boot, immediately followed by SNP_INIT, and that state can't be rebuilt on resume without a full re-init.
So a CPU coming back online without SnpEn can't occur in a working config.

Thanks,
Ashish