Re: [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs

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

On 9/2/2026 4:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] CPU hotplug lock is leaked if SEV-SNP initialization fails.
> --
> 
> commit 16bd9580b377dc645541bd4f478c471c109e65ae
> Author: Ashish Kalra <ashish.kalra@amd.com>
> 
> x86/sev: Initialize RMPOPT configuration MSRs
> 
> This commit introduces the RMPOPT configuration MSRs and adds support to
> set up and enable RMPOPT once SNP is initialized. The RMPOPT table base
> is initialized to enable RMP optimization for up to 2 TB of system RAM.
> 
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index f833cb7e4da36..287a8345854ba 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>  
>>  	sev_es_tmr_size = SNP_TMR_SIZE;
>>  
>> +	snp_setup_rmpopt();
>> +
> 
> [Severity: High]
> This is a pre-existing issue, but does __sev_snp_init_locked() permanently
> leak the CPU hotplug lock if SEV-SNP initialization fails?
> 
> If snp_prepare() succeeds, it disables CPU hotplug. However, if an error
> occurs later in the function (for example, if walk_iomem_res_desc() fails),
> the function returns the error code without re-enabling CPU hotplug:
> 
> drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
>     ...
>     rc = snp_prepare();
>     if (rc)
>         return rc;
>     ...
>     rc = walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_MEM, 0, ~0,
>                              snp_range_list, snp_filter_reserved_mem_regions);
>     if (rc) {
>         dev_err(sev->dev,
>                 "SEV: SNP_INIT_EX walk_iomem_res_desc failed rc = %d\n", rc);
>         return rc;
>     }
>     ...
> }
> 
> This leaves cpu_hotplug_disabled strictly greater than zero, which would
> permanently prevent CPUs from being onlined or offlined. Does this function
> need an error handling path that calls cpu_hotplug_enable() before returning
> on these failure paths?
> 

Thanks — but this is intentional, not a leak. CPU hotplug is tied to
SnpEn, not to the success of __sev_snp_init_locked().

snp_prepare() disables hotplug and then sets SnpEn on all CPUs via
on_each_cpu(snp_enable, ...) before it returns 0. So by the time
walk_iomem_res_desc() (and the other post-snp_prepare() steps) run,
SnpEn is already set on every online CPU and the machine is in SNP mode.

While SnpEn is set, a CPU must not come online without it — that is
exactly what disabling hotplug guards against. Re-enabling hotplug on
the walk_iomem_res_desc() failure path, with SnpEn still set, would
reopen that unsafe window, so hotplug is deliberately left disabled
there. It is re-enabled only once SnpEn is actually cleared, on the
x86_snp_shutdown path: snp_shutdown() verifies SnpEn is clear and then
calls cpu_hotplug_enable().

The one snp_prepare() failure that occurs before SnpEn is set — the
"not all CPUs online" check — does re-enable hotplug itself. There is no
failure path between cpu_hotplug_disable() and snp_enable() that can
return with hotplug disabled and SnpEn clear (the intervening steps are
void). So hotplug tracks SnpEn on every path.

Thanks,
Ashish

>>  	return 0;
>>  }
>