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

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

On 9/2/2026 4:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] PM Suspend (S3) bypasses CPU hotplug disable, leading to security breakdown
> - [High] CPU hotplug remains enabled on kexec boot if setup_rmptable() fails
> --
> 
> commit c91487bb659a8ce73e581c086c8aa1aa19f4b9aa
> Author: Ashish Kalra <ashish.kalra@amd.com>
> x86/sev: Disable CPU hotplug while SNP is active
> 
> This patch disables CPU hotplug while SNP is active to ensure no CPU comes
> online without the SnpEn bit set. It ties hotplug to the SNP-enable bit by
> disabling it in snp_prepare() and re-enabling it in snp_shutdown().
> 
>> 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: Critical]
> Does this sequence prevent CPUs from being onlined without SnpEn set during
> power management transitions?
> 
> While cpu_hotplug_disable() blocks sysfs-initiated hotplug operations, it
> appears that system suspend (S3) and hibernation (S4) bypass this mechanism.
> During suspend, the PM core uses freeze_secondary_cpus() and
> thaw_secondary_cpus(), which explicitly call _cpu_down() and _cpu_up()
> regardless of the cpu_hotplug_disabled counter.
> 
> Additionally, MSR_AMD64_SYSCFG is not included in the list of MSRs saved and
> restored during PM transitions in arch/x86/power/cpu.c.
> 
> If a system with SNP active goes to sleep and resumes, will the secondary
> CPUs wake up without the SnpEn bit set, potentially breaking the SEV-SNP
> security model by failing to enforce RMP checks?

SEV-SNP hosts do not support suspend-to-RAM (S3) or hibernation (S4). The
SNP security state that would have to survive a PM transition — the per-CPU
SnpEn bit, the RMP table, and the SNP-locked AMD IOMMU — is not saved and
restored across suspend/resume, so a resumed SNP host would be in an
inconsistent state independently of CPU hotplug. (That is also why
MSR_AMD64_SYSCFG is not in the PM save/restore set.)

This patch does not change any of that. It replaces the existing
cpus_read_lock() in snp_prepare() with cpu_hotplug_disable(); neither
touches the PM suspend/resume path (freeze_secondary_cpus() /
thaw_secondary_cpus()). The patch's purpose is narrower: to stop a runtime,
sysfs-initiated CPU online from bringing a CPU up without SnpEn while SNP is
active. The S3/S4 vector is pre-existing.

> 
> [ ... ]
> 
>> @@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>>   */
>>  int __init snp_rmptable_init(void)
>>  {
>> +	u64 val;
>> +
>>  	if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
>>  		return -ENOSYS;
>>  
>> @@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
>>  	if (!setup_rmptable())
>>  		return -ENOSYS;
> 
> [Severity: High]
> If setup_rmptable() fails and returns -ENOSYS early, do we leave CPU hotplug
> enabled even though the hardware might still have SNP enabled from a previous
> kernel?
> 
> When booting via kexec from an SNP-enabled kernel, the CPUs retain the SnpEn
> bit in MSR_AMD64_SYSCFG. If setup_rmptable() fails here (for example, due to
> a memremap failure), the function exits before reaching the new check that
> disables CPU hotplug.
> 
> Because the function failed, the kernel clears CC_ATTR_HOST_SEV_SNP. If a CPU
> is subsequently hotplugged via sysfs, the new kernel will not set SnpEn on
> it, resulting in a state where boot CPUs have SnpEn set but the hotplugged
> CPU does not.
> 
> Could this early return be moved after the MSR_AMD64_SYSCFG_SNP_EN check to
> ensure hotplug is disabled whenever the hardware indicates SNP is active?
> 

On any platform that supports RMPOPT (which requires a segmented RMP), a
kexec performs a full SNP shutdown: the CCP driver's .shutdown handler
clears SnpEn (SNP_SHUTDOWN via __sev_snp_shutdown_locked). So on a kexec
boot of such a platform SnpEn is already clear — snp_rmptable_init() reads
it as clear and the machine takes the normal path, where snp_prepare()
disables CPU hotplug. The "SnpEn still set from the previous kernel" case
does not arise here.

That case only happens with legacy firmware that leaves SnpEn set across a
kexec shutdown, and those platforms do not support RMPOPT. On such a
platform, if setup_rmptable() additionally fails, SNP is not brought up at
all — snp_rmptable_init() returns -ENOSYS and CC_ATTR_HOST_SEV_SNP is
cleared — so no SNP guest runs. The SnpEn-set / hotplug-enabled state there
is pre-existing SEV-SNP behaviour (snp_rmptable_init() did not touch CPU
hotplug before this series), not a regression introduced here.

Thanks,
Ashish

>> +
>> +	/*
>> +	 * On a kexec boot SNP may already be enabled (legacy firmware leaves
>> +	 * SnpEn set across shutdown), in which case snp_prepare() bails without
>> +	 * disabling CPU hotplug, so disable it here.
>> +	 */
>> +	rdmsrq(MSR_AMD64_SYSCFG, val);
>> +	if (val & MSR_AMD64_SYSCFG_SNP_EN)
>> +		cpu_hotplug_disable();
>> +
>>  	/*
>>  	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
>>  	 * notifier is invoked to do SNP IOMMU shutdown before kdump.
>