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

[sashiko-bot@kernel.org]

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@amd.com>

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 on the kernel command line resulting in
!amd_iommu_snp_en, or setup_rmptable() failing), 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?

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@amd.com?part=2