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

["Kalra, Ashish" <ashish.kalra@amd.com>]

Hello Boris,

On 8/29/2026 3:52 PM, Borislav Petkov wrote:
> 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.

Agreed.

rmpopt_cpumask is indeed just cpu_primary_thread_mask: all primary threads are online whenever SNP is acrtive (snp_prepare() check),
so my online-filtered set is identical.

And yes - re-running RMPOPT on the leader in the fan-out is just a cache hit, so excluding it isn't worth the andnot + separate follower mask.

The one piece to keep (which your diff below *does* already): the single warm-up scan before the fan out - 

migrate_disable();
rmopt_scan_range(NULL);
migrate_enable();
on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);

That first scan is the whole reason for a "leader": it populates the shared scan cache in preemptible context, so every RMPOPT the IPI issues
is a cache hit and the IRQ‑off window per core stays short. 

And we drop the leader/follower bookkeeping, but the warm-up stays.

Minor nit with the patch below : with the andnot gone, this_cpu is now unused, so please drop it.

Thanks,
Ashish

> 
> 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);
>