Re: [PATCH v12 4/5] x86/sev: Add support to perform RMP optimizations asynchronously

[Borislav Petkov <bp@alien8.de>]

On Mon, Aug 10, 2026 at 07:22:10PM +0000, Ashish Kalra wrote:
> +/*
> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
> + * memory does not contain any SNP guest memory.
> + *
> + * @pa is a system physical address; RMPOPT operates on the containing 1GB.
> + */
> +static void rmpopt(u64 pa)
> +{
> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
> +	enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
> +
> +	/*
> +	 * RMPOPT (F2 0F 01 FC): RAX = 1GB-aligned SPA, RCX = op type, CF set if
> +	 * the range was optimized (result unused on this path).
> +	 *
> +	 * Binutils does not support the RMPOPT mnemonic yet, so the instruction
> +	 * is encoded with .byte.
> +	 */

I don't know whether you've checked the binutils sources but this should have
the version of binutils which supports it and I think there is no such version
yet - I've pinged binutils team to see what their plans are.

> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     :: "a" (pa_start), "c" (op)
> +		     : "memory", "cc");
> +}
> +
> +/* on_each_cpu() callback: optimize the whole RMPOPT range on this CPU. */
> +static void rmpopt_scan_range(void *arg)
> +{
> +	u64 pa;
> +
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		rmpopt(pa);
> +}
> +
> +static void rmpopt_work_handler(struct work_struct *work)
> +{
> +	int this_cpu;
> +
> +	/*
> +	 * RMPOPT scans the RMP table, stores the result of the scan in the
> +	 * reserved processor memory. The RMP scan is the most expensive
> +	 * part. If a second RMPOPT occurs, it can skip the expensive scan
> +	 * if they can see a cached result in the reserved processor memory.
> +	 *
> +	 * Run RMPOPT on one CPU first (the leader), then on every other primary
> +	 * thread (the followers).  A follower skips the expensive RMP scan by
> +	 * reusing the cached scan results the leader produced.
> +	 *
> +	 * migrate_disable() pins this work to the current CPU so it stays the
> +	 * leader for the whole leader loop: this_cpu remains valid and the
> +	 * RMPOPT instruction runs on it.
> +	 */
> +	migrate_disable();
> +	this_cpu = smp_processor_id();
> +
> +	cpumask_andnot(rmpopt_follower_mask, rmpopt_cpumask,
> +		       topology_sibling_cpumask(this_cpu));
> +
> +	rmpopt_scan_range(NULL);
> +
> +	migrate_enable();
> +
> +	/*
> +	 * Followers: one IPI per remaining core, each optimizing the whole
> +	 * range.  Each runs with interrupts disabled, but only issues cache-hit
> +	 * RMPOPTs (the leader populated the scan cache above), so the window is
> +	 * short.  cpus_read_lock() is intentionally not held: CPU hotplug is
> +	 * disabled the entire time SNP is active (see snp_prepare()), and this
> +	 * work only runs while SNP is active, so the follower set stays valid.
> +	 */
> +	on_each_cpu_mask(rmpopt_follower_mask, rmpopt_scan_range, NULL, true);

If the leader ran once and the results are cached, then does it matter if
I run RMPOPT on it again? IOW, what's stopping me from doing

	on_each_cpu_mask(rmpopt_cpumask, rmpopt_scan_range, NULL, true);

and don't care about leaders and followers at all?

Also, rmpopt_mask is cpu_primary_thread_mask and you don't need any of those
gymnastics.

IOW, this is how my cleanup ontop looks like so far:

diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 55a6e2319899..a1f6f3111517 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -125,7 +125,6 @@ static void *rmp_bookkeeping __ro_after_init;
 
 static u64 probed_rmp_base, probed_rmp_size;
 
-static cpumask_var_t rmpopt_cpumask, rmpopt_follower_mask;
 static u64 rmpopt_pa_start, rmpopt_pa_end;
 
 enum rmpopt_op_type {
@@ -587,11 +586,9 @@ static void rmpopt_disable(void)
 	cancel_delayed_work_sync(&rmpopt_delayed_work);
 	destroy_workqueue(rmpopt_wq);
 
-	for_each_cpu(cpu, rmpopt_cpumask)
+	for_each_cpu(cpu, cpu_primary_thread_mask)
 		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
 
-	free_cpumask_var(rmpopt_cpumask);
-	free_cpumask_var(rmpopt_follower_mask);
 	rmpopt_pa_start = rmpopt_pa_end = 0;
 	rmpopt_wq = NULL;
 }
@@ -632,16 +629,9 @@ static bool rmpopt_capable(void)
  */
 static void rmpopt(u64 pa)
 {
-	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
 	enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
+	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
 
-	/*
-	 * RMPOPT (F2 0F 01 FC): RAX = 1GB-aligned SPA, RCX = op type, CF set if
-	 * the range was optimized (result unused on this path).
-	 *
-	 * Binutils does not support the RMPOPT mnemonic yet, so the instruction
-	 * is encoded with .byte.
-	 */
 	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
 		     :: "a" (pa_start), "c" (op)
 		     : "memory", "cc");
@@ -656,43 +646,22 @@ static void rmpopt_scan_range(void *arg)
 		rmpopt(pa);
 }
 
-static void rmpopt_work_handler(struct work_struct *work)
+static void do_rmpopt_work(struct work_struct *work)
 {
 	int this_cpu;
 
 	/*
-	 * RMPOPT scans the RMP table, stores the result of the scan in the
-	 * reserved processor memory. The RMP scan is the most expensive
-	 * part. If a second RMPOPT occurs, it can skip the expensive scan
-	 * if they can see a cached result in the reserved processor memory.
-	 *
-	 * Run RMPOPT on one CPU first (the leader), then on every other primary
-	 * thread (the followers).  A follower skips the expensive RMP scan by
-	 * reusing the cached scan results the leader produced.
-	 *
-	 * migrate_disable() pins this work to the current CPU so it stays the
-	 * leader for the whole leader loop: this_cpu remains valid and the
-	 * RMPOPT instruction runs on it.
+	 * RMPOPT caches the results of a RMP table scan in reserved processor
+	 * memory, allowing future invocations to skip such costly operations.
 	 */
 	migrate_disable();
 	this_cpu = smp_processor_id();
 
-	cpumask_andnot(rmpopt_follower_mask, rmpopt_cpumask,
-		       topology_sibling_cpumask(this_cpu));
-
 	rmpopt_scan_range(NULL);
 
 	migrate_enable();
 
-	/*
-	 * Followers: one IPI per remaining core, each optimizing the whole
-	 * range.  Each runs with interrupts disabled, but only issues cache-hit
-	 * RMPOPTs (the leader populated the scan cache above), so the window is
-	 * short.  cpus_read_lock() is intentionally not held: CPU hotplug is
-	 * disabled the entire time SNP is active (see snp_prepare()), and this
-	 * work only runs while SNP is active, so the follower set stays valid.
-	 */
-	on_each_cpu_mask(rmpopt_follower_mask, rmpopt_scan_range, NULL, true);
+	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
 }
 
 void snp_setup_rmpopt(void)
@@ -729,30 +698,7 @@ void snp_setup_rmpopt(void)
 		return;
 	}
 
-	INIT_DELAYED_WORK(&rmpopt_delayed_work, rmpopt_work_handler);
-
-	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
-		pr_err("Failed to allocate RMPOPT cpumask\n");
-		destroy_workqueue(rmpopt_wq);
-		rmpopt_wq = NULL;
-		return;
-	}
-
-	if (!zalloc_cpumask_var(&rmpopt_follower_mask, GFP_KERNEL)) {
-		pr_err("Failed to allocate RMPOPT follower cpumask\n");
-		free_cpumask_var(rmpopt_cpumask);
-		destroy_workqueue(rmpopt_wq);
-		rmpopt_wq = NULL;
-		return;
-	}
-
-	/*
-	 * The RMPOPT_BASE MSR has core scope. All primary threads are online,
-	 * otherwise SNP would not have been enabled.
-	 */
-	for_each_online_cpu(cpu)
-		if (topology_is_primary_thread(cpu))
-			cpumask_set_cpu(cpu, rmpopt_cpumask);
+	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;
@@ -761,7 +707,7 @@ void snp_setup_rmpopt(void)
 	 * Per-CPU RMPOPT tables cover at most 2 TB.  Program each core's
 	 * RMPOPT_BASE with the start of RAM to optimize up to 2 TB.
 	 */
-	for_each_cpu(cpu, rmpopt_cpumask)
+	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);

-- 
Regards/Gruss,
    Boris.

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