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

On 7/23/2026 1:53 PM, Borislav Petkov wrote:
> On Tue, Jun 30, 2026 at 06:11:03PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <[email protected]>
>>
>> While SNP is active, every memory write is checked against the RMP to
>> protect SEV-SNP guest memory.  A core performs these RMP checks only once
>> ...
> 
> Use this commit message for your next revision. The idea is to split it into
> smaller paragraphs for easier parsing, do simple formulations and not talk
> about future patches because git history is not always linear:
> 
> "While SNP is active, every memory write is checked against the RMP to
> protect SEV-SNP guest memory.  A core performs these RMP checks only once SNP
> has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is set on
> that core; the firmware requires the SNP-enable bit to be set on every present
> CPU before SNP initialization.
> 
> A core that is not SNP-enabled and not SNP-initialized performs no RMP checks
> at all, so there is no valid configuration with SNP active and any CPU exempt
> from RMP checks.
> 
> The firmware determines which CPUs are present from the processor and the
> BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates them at
> SNP init; it is not aware of the OS bringing CPUs online or offline
> afterwards.
> 
> SNP_INIT fails unless SnpEn is set on all CPUs, so a CPU that is offline when
> SNP_INIT is issued, does not have SnpEn set, SNP_INIT fails, and there can be
> no SNP guest memory.  OS CPU hotplug can thus diverge from the firmware's
> expectations and break SNP.
> 
> Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
> SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
> disabled SNP.
> 
> If snp_prepare() fails before enabling SNP it re-enables hotplug itself; once
> SNP is enabled hotplug stays disabled, including across a failed SNP_INIT and
> across the legacy SNP_SHUTDOWN_EX path, both of which leave SNP enabled.
> 
> A kexec target that boots with SNP already enabled, disables hotplug once in
> snp_rmptable_init(), since snp_prepare() bails when SNP is already enabled."
> 

Sure.

>>
>> Suggested-by: Thomas Lendacky <[email protected]>
>> Signed-off-by: Ashish Kalra <[email protected]>
>> ---
>>  arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index dab6e1c290bc..04a58ac4339c 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();
>> +
>>  	cpus_read_lock();
> 
> What's the point of grabbing the hotplug lock if you just disabled hotplug?
> 
>>  	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
>> @@ -560,6 +569,10 @@ int snp_prepare(void)
>>  unlock:
>>  	cpus_read_unlock();
> 
> ditto.
> 

This is part of the base code, but i will have to modify it now that hotplug is explicitly disabled.

Actually, i will have to remove cpus_read_lock()/unlock() across the whole patch series and also in
the original code.

Thanks,
Ashish