Re: [PATCH v14 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Race condition between KVM VM destruction and CCP unbind allows RMPOPT work to be queued after SNP shutdown, leading to a potential kernel panic.
- [High] System lockup due to broadcasting a cold-cache memory scan to all CPUs with interrupts disabled.
--

commit 5610134a51dd4a088684e2eb5968c3e869f5727c
Author: Ashish Kalra <ashish.kalra@amd.com>

x86/sev: Re-enable RMP optimizations on SNP guest shutdown

This commit re-enables RMP optimizations on SNP guest shutdown by scheduling
a delayed work pass. It introduces snp_rmpopt_all_physmem() to queue the scan
10 seconds after a guest termination. The delayed work batches multiple guest
teardowns into a single optimization pass.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 5705723f1f412..d8e6b8a08b79f 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -3032,6 +3032,8 @@ void sev_vm_destroy(struct kvm *kvm)
>  		 */
>  		if (snp_decommission_context(kvm))
>  			return;
> +
> +		snp_rmpopt_all_physmem();

[Severity: High]
Does this create a race condition with CCP unbind that can lead to a host panic?

If the CCP device is unbound or shut down, snp_shutdown() is invoked, which
disables RMPOPT by cancelling pending work via rmpopt_disable(). However,
rmpopt_disable() does not set rmpopt_wq = NULL:

arch/x86/virt/svm/sev.c:rmpopt_disable() {
    guard(mutex)(&rmpopt_wq_mutex);

    if (rmpopt_wq)
        cancel_delayed_work_sync(&rmpopt_delayed_work);
}

If KVM concurrently destroys a VM, it calls snp_rmpopt_all_physmem() here.
Because rmpopt_wq is still valid, the background scan is blindly re-queued.

When the delayed work executes, it will issue the RMPOPT instruction while
SNP is disabled in hardware (SYSCFG_SNP_EN == 0), causing an Invalid Opcode
(#UD) exception and panicking the host system.

>  	} else {
>  		sev_unbind_asid(kvm, sev->handle);
>  	}
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 35678b1f535d4..c16f826423903 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -640,6 +640,37 @@ static void do_rmpopt_work(struct work_struct *work)
>  	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);

[Severity: High]
Can this cause a system lockup by broadcasting a cold-cache memory scan to all
CPUs with interrupts disabled?

When an SNP guest shuts down, it queues rmpopt_delayed_work. The worker
function do_rmpopt_work() attempts to warm up the cache first:

arch/x86/virt/svm/sev.c:do_rmpopt_work() {
    rmpopt_scan_range(NULL);
    on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
}

Because the RMPOPT hardware cache is per-CPU, running rmpopt_scan_range()
locally only warms up the cache for the worker's CPU. The subsequent
on_each_cpu_mask() forces all other CPUs to run the scan with interrupts
disabled.

Since all other CPUs have a cold cache, they perform a massive memory scan
(up to 2TB) while IRQs are disabled, which can cause immense latency and
lead to host lockup (exceeding NMI watchdog or RCU stall timeouts).

>  }
>  
> +/*
> + * Delay, in milliseconds, before the RMP re-optimization pass runs after an SNP
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788984053.git.ashish.kalra@amd.com?part=5