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

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

On 9/10/2026 5:25 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] System suspend/resume bypasses cpu_hotplug_disable() and brings CPUs online without SnpEn set, breaking the SNP invariant.
> --
> 
> commit 0243c46e8f299e3de42fbcddbc4c7bd93fb68c65
> Author: Ashish Kalra <ashish.kalra@amd.com>
> 
> x86/sev: Disable CPU hotplug while SNP is active
> 
> This commit disables CPU hotplug while SEV-SNP is active. This ensures no
> CPU can be brought online without the SnpEn bit set in MSR_AMD64_SYSCFG,
> which is required for hardware RMP checks to function correctly.
> 
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index cff285d8ad8e3..558f7924a3f85 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -526,14 +525,18 @@ int snp_prepare(void)
>>  
>>  	clear_rmp();
>>  
>> -	cpus_read_lock();
>> +	/*
>> +	 * No CPU may come online without SnpEn while SNP is active; disable
>> +	 * hotplug here and re-enable it in snp_shutdown().
>> +	 */
>> +	cpu_hotplug_disable();
> 
> [Severity: High]
> Does system suspend/resume bypass cpu_hotplug_disable() and bring CPUs online
> without SnpEn set, breaking the SNP invariant?
> 
> If the host initiates an ACPI system suspend (e.g., S3), freeze_secondary_cpus()
> offlines all secondary CPUs via _cpu_down(), which bypasses the
> cpu_hotplug_disabled counter incremented by cpu_hotplug_disable().
> 
> Upon resume, thaw_secondary_cpus() brings the CPUs back online via _cpu_up().
> Because CPU MSR state is lost during power-off in suspend and snp_enable() is
> not registered as a CPU hotplug callback, the secondary CPUs wake up with SnpEn
> cleared. This would allow them to bypass hardware RMP checks.
> 

You're right that the suspend path bypasses cpu_hotplug_disable() — freeze_secondary_cpus() uses _cpu_down() directly and ignores the cpu_hotplug_disabled counter. But this is neither introduced nor affected by this patch, and does not apply to SNP hosts:

1. This patch does not touch the suspend/resume path. cpu_hotplug_disable() holds the online CPU set stable against the regular (sysfs) hotplug path while SNP is active. The cpus_read_lock() this patch replaces did not gate the freeze/thaw path either, so suspend/resume behaviour is unchanged from base SNP.
2. SNP hosts do not support system suspend/resume. It is not a supported configuration for an SNP‑enabled host, and there is no path today to re‑establish the per‑CPU SNP state (SnpEn) on resume — snp_enable() has never been registered as a CPU‑hotplug/resume callback. That is a pre‑existing property of base SNP, independent of this series.

Thanks,
Ashish