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

[Kalra, Ashish]

Hello Boris,

On 9/16/2026 9:19 PM, Borislav Petkov wrote:
> On Wed, Sep 16, 2026 at 10:36:30PM +0000, sashiko-bot@xxxxxxxxxx wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] x86/sev: CPU hotplug disablement bypassed on early kexec errors
>> --
>>
>> commit ac084f2d7188c520f2ee3ced2ab76e4c068df91f
>> Author: Ashish Kalra <ashish.kalra@xxxxxxx>
>>
>> x86/sev: Disable CPU hotplug while SNP is active
>>
>> This commit disables CPU hotplug when SNP is enabled to prevent SNP guest
>> memory from being broken by CPUs coming online without the SnpEn bit set.
>> It ties CPU hotplug to the SNP-enable bit, disabling it in snp_prepare()
>> and re-enabling it in snp_shutdown() when SNP is disabled. It also
>> explicitly disables CPU hotplug during kexec if SNP is already enabled.
>>
>>> 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
>>
>> [ ... ]
>>
>>> @@ -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;
>>>  
>>> +	/*
>>> +	 * 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();
>>
>> [Severity: High]
>> If a kexec kernel boots into a system where the previous kernel left
>> MSR_AMD64_SYSCFG_SNP_EN active, and an early initialization step fails
>> (such as passing amd_iommu=off
> 
> amd_iommu=off means no SNP.
> 
>> on the kernel command line resulting in !amd_iommu_snp_en, or
>> setup_rmptable() failing),
> 
> The same happens in that case:
> 
>         if (snp_rmptable_init()) {
>                 pr_warn("SNP: RMP initialization failed, SNP cannot be supported.\n");
>                 goto disable_snp;
> 		     ^^^^^^^^^^^
> 
> Does Sashiko not see that or somehow it ate a lot of credits so it cannot use
> enough now to "swap in" the whole context... if that is the AI that is going
> to destroy the world, then I'm not too worried. :)
> 
> Or is my AI failing me right now?
> 
> :-P
> 
> Leaving in the rest.

Right, agreed — 

As you have pointed out, on any snp_rmptable_init() failure — setup_rmptable() failing,
!amd_iommu_snp_en (amd_iommu=off), etc. — the caller does goto disable_snp and SNP is
disabled entirely. 

Thanks,
Ashish

> 
>> snp_rmptable_init() will
>> return -ENOSYS before checking the physical MSR hardware state and
>> disabling CPU hotplug.
>>
>> Because hotplug remains enabled and the CCP driver skips initialization on
>> failure (bypassing snp_prepare()), could a subsequently hotplugged CPU come
>> online without the SNP_EN bit set?
> 
> Yes, but the above disabled all our intention of running SNP in the first
> place...
>  
>> If this results in a mixed hardware state, does it violate firmware
>> constraints and cause fatal RMP faults when the new CPU accesses memory?
>>
>> Should the hardware state be checked and hotplug disabled unconditionally
>> before the software configuration checks?
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789594774.git.ashish.kalra@xxxxxxx?part=2
>