Re: [PATCH v14 4/5] x86/sev: Perform RMP optimizations asynchronously

[Borislav Petkov]

On Thu, Sep 10, 2026 at 10:00:08PM +0000, Ashish Kalra wrote:
>  void snp_setup_rmpopt(void)
>  {
>  	u64 rmpopt_base;
> @@ -591,6 +648,30 @@ void snp_setup_rmpopt(void)
>  	if (!rmpopt_capable())
>  		return;
>  
> +	guard(mutex)(&rmpopt_wq_mutex);
> +
> +	/*
> +	 * Set up once: the workqueue and RMPOPT_BASE MSRs are left in place on
> +	 * shutdown, so a later re-initialization just re-queues the optimization
> +	 * pass rather than redoing the setup.
> +	 */
> +	if (rmpopt_wq) {
> +		queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
> +		return;
> +	}

No, this is not how this is done. This is a *setup* function but you also use
it to start the workqueue if it has been allocated already. So it should
either setup or start but not both.

So what you do is, you try to allocate the workqueue. If it fails, you clear
X86_FEATURE_RMPOPT so that rmpopt_capable() is false and that can be your
start_workqueue function.

This way you get rid of all that

	if (rmpopt_wq) 

sprinkles everywhere.

> +
> +	/*
> +	 * Use a dedicated per-CPU workqueue so the potentially lengthy warm-up
> +	 * scan does not tie up a shared workqueue worker.
> +	 */
> +	rmpopt_wq = alloc_workqueue("rmpopt_wq", WQ_PERCPU, 1);
> +	if (!rmpopt_wq) {
> +		pr_err("Failed to allocate RMPOPT workqueue\n");
> +		return;
> +	}
> +
> +	INIT_DELAYED_WORK(&rmpopt_delayed_work, do_rmpopt_work);
> +
>  	rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
>  	rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
>  
> @@ -600,6 +681,15 @@ void snp_setup_rmpopt(void)
>  	 */
>  	for_each_cpu(cpu, cpu_primary_thread_mask)
>  		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> +
> +	rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
> +
> +	if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T)
> +		rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
> +
> +	queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
> +
> +	pr_info("RMPOPT optimizations enabled\n");
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");

There is no ccp driver patch calling this so this export needs to happen when
you're actually adding the ccp code.

Same thing for the snp_rmpopt_all_physmem() export to kvm-amd.

Looking at this more, I would like to get rid of the snp_setup_rmpopt() export
and have this function do the necessary setup stuff from an initcall in this
file. This way you set up the stuff at kernel init time and have everything
ready to go.

Then the ccp will *only* call a function which is called snp_enable_rmpopt()
after it has enabled SNP. That function simply enables the workqueue.

And then kvm-amd can call that function too so we end up with one export.

Oh, and you can zap those comments while at it:

diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index ca99617142be..c8ba71431a5e 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -619,7 +619,6 @@ static void rmpopt(u64 pa)
 		     : "memory", "cc");
 }
 
-/* on_each_cpu() callback: optimize the whole RMPOPT range on this CPU. */
 static void rmpopt_scan_range(void *arg)
 {
 	u64 pa;
@@ -632,7 +631,7 @@ static void do_rmpopt_work(struct work_struct *work)
 {
 	/*
 	 * Warm up the RMPOPT cache on this pinned per-CPU worker with interrupts
-	 * on, so the IRQ-disabled fan-out below only issues cache-hit RMPOPTs.
+	 * enabled, so the IRQ-disabled fan-out below only issues cache-hit RMPOPTs.
 	 */
 	rmpopt_scan_range(NULL);
 
@@ -649,11 +648,6 @@ void snp_setup_rmpopt(void)
 
 	guard(mutex)(&rmpopt_wq_mutex);
 
-	/*
-	 * Set up once: the workqueue and RMPOPT_BASE MSRs are left in place on
-	 * shutdown, so a later re-initialization just re-queues the optimization
-	 * pass rather than redoing the setup.
-	 */
 	if (rmpopt_wq) {
 		queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
 		return;

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette