Re: [PATCH v7 5/6] KVM: SEV: Perform RMP optimizations on SNP guest shutdown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The RMPOPT batching logic fails to optimize memory for guests that terminate shortly before an already-pending workqueue timer expires.
--

commit 855d23e84feebfc7b3cebafc11cb0b8453330a11
Author: Ashish Kalra <ashish.kalra@amd.com>

KVM: SEV: Perform RMP optimizations on SNP guest shutdown

This commit triggers an RMP optimization pass after an SNP guest
terminates to take advantage of pages being converted back to shared.
It batches the work so that a single optimization pass can cover
multiple guests terminating within a short period.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 4272293478766..0e94f81907ec2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2992,6 +2992,8 @@ void sev_vm_destroy(struct kvm *kvm)
>  		 */
>  		if (snp_decommission_context(kvm))
>  			return;
> +
> +		snp_rmpopt_all_physmem();

[Severity: Medium]
Does calling snp_rmpopt_all_physmem() here trigger the optimization pass
too early in the KVM teardown sequence?

sev_vm_destroy() runs during kvm_arch_destroy_vm(), which executes before
kvm_free_memslots() where the actual conversion of guest memory from private
to shared occurs.

Additionally, looking at snp_rmpopt_all_physmem() in arch/x86/virt/svm/sev.c,
it appears to use queue_delayed_work() rather than mod_delayed_work():

    queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
                       msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));

Because queue_delayed_work() does not reset the timer of an already-queued
work item, the timer is not extended when subsequent guests terminate.

If a second guest terminates just before the pending RMPOPT work item executes,
the optimization pass will run concurrently with or prior to the second guest's
kvm_free_memslots(). Since the second guest's memory is still private at that
point, the RMPOPT pass would skip it, leaving the memory unoptimized and
causing a performance regression for RMP checks.

Would it be safer to use mod_delayed_work() to ensure the timer is properly
extended, and perhaps trigger the pass after the memslots have actually been
freed?

>  	} else {
>  		sev_unbind_asid(kvm, sev->handle);
>  	}

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