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

On 7/31/2026 12:44 AM, Borislav Petkov wrote:
> On Mon, Jul 27, 2026 at 07:05:29PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <[email protected]>
>>
>> When SEV-SNP is enabled, all writes to memory are checked to ensure
>> integrity of SNP guest memory. This imposes performance overhead on the
> 
> s/SNP//
> 
> The checks are done not only on SNP guest memory but on *all* memory, as your
> next paragraph suggests.

Yes, the checks are done on *all* memory but for ensuring the integrity of SNP 
guest memory, so that is what the above paragraph is mentioning.

> 
>> whole system.
>>
>> RMPOPT is a new instruction that minimizes the performance overhead of
>> RMP checks on the hypervisor and on non-SNP guests by allowing RMP
>> checks to be skipped for 1GB regions of memory that are known not to
>> contain any SEV-SNP guest memory.
> 
> Let's tone down the abbreviations. "SNP guest memory" is enough and let's
> stick to that.
> 
>> Add support for performing RMP optimizations asynchronously using a
>> dedicated workqueue.
>>
>> Enable RMPOPT optimizations for up to 2TB of system RAM starting from
>> the lowest physical memory address aligned down to a 1GB boundary at
>> RMP initialization time. RMP checks can initially be skipped for 1GB
> 
> Why "initially"? What are you trying to say here?

"initially" meant the init-time state — before any SNP guests exist, all eligible 1 GB ranges are optimized — and the
last sentence covers how that changes as guests launch.

The other way i can put it is: "RMP checks are skipped for 1-GB ranges that don't contain SNP guest memory and 
as SNP guests are launched, RMPUPDATE disables the corresponding optimizations".

> 
>> memory ranges that do not contain SEV-SNP guest memory (excluding
>> preassigned pages such as the RMP table and firmware pages). As SNP
>> guests are launched, RMPUPDATE will disable the corresponding RMPOPT
>> optimizations.
> 
> Because it will add pages to the RMP table?
> 
> This paragraph needs clarification.

Not by adding pages — the RMP table already covers all memory. When RMPUPDATE assigns a page to an SNP guest
(guest-owned state) inside an optimized 1 GB region, the hardware clears that region's RMPOPT optimization, so RMP
checks resume there to protect the guest memory. I'll reword the paragraph to say that explicitly.

> 
>> Suggested-by: Thomas Lendacky <[email protected]>
>> Suggested-by: Dave Hansen <[email protected]>
>> Suggested-by: K Prateek Nayak <[email protected]>
>> Reviewed-by: Ackerley Tng <[email protected]>
>> Signed-off-by: Ashish Kalra <[email protected]>
>> ---
>>  arch/x86/virt/svm/sev.c | 160 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 158 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index 8bfd80284836..04b19e64f832 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/iommu.h>
>>  #include <linux/amd-iommu.h>
>>  #include <linux/nospec.h>
>> +#include <linux/workqueue.h>
>>  
>>  #include <asm/sev.h>
>>  #include <asm/processor.h>
>> @@ -125,7 +126,18 @@ static void *rmp_bookkeeping __ro_after_init;
>>  static u64 probed_rmp_base, probed_rmp_size;
>>  
>>  static cpumask_var_t rmpopt_cpumask;
>> -static phys_addr_t rmpopt_pa_start;
>> +static phys_addr_t rmpopt_pa_start, rmpopt_pa_end;
>> +
>> +enum rmpopt_function {
> 
> 	rmpopt_op_type
> 
>> +	RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS,
>> +	RMPOPT_FUNC_REPORT_STATUS
> 
> RMPOPT_OP_VERIFY...
> 
>> +};
>> +
> 
> /*
>  * This timeout was selected this way because...
>  */
> 

The value is a heuristic that came out of review feedback on the series, i can add a comment here
documenting what the timeout is for (coalescing SNP guest teardowns into one re-optimization pass and
letting guest pages convert back to shared before the scan).

>> +#define RMPOPT_WORK_TIMEOUT	10000
>> +
>> +static struct workqueue_struct *rmpopt_wq;
>> +static struct delayed_work rmpopt_delayed_work;
>> +static DEFINE_MUTEX(rmpopt_wq_mutex);
>>  
>>  static LIST_HEAD(snp_leaked_pages_list);
>>  static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
>> @@ -565,11 +577,20 @@ static void snp_cleanup_rmpopt(void)
>>  {
>>  	int cpu;
>>  
>> +	guard(mutex)(&rmpopt_wq_mutex);
>> +
>> +	if (!rmpopt_wq)
>> +		return;
> 
> If there's no workqueue, you skip all the rest, including undoing things which
> are not workqueue-related?
> 
> That workqueue pointer must be magical and special. Yet, I don't see anything
> explaining that.

I will add a comment spelling that out.

> 
>> +
>> +	cancel_delayed_work_sync(&rmpopt_delayed_work);
>> +	destroy_workqueue(rmpopt_wq);
>> +
>>  	for_each_cpu(cpu, rmpopt_cpumask)
>>  		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
>>  
>>  	free_cpumask_var(rmpopt_cpumask);
>> -	rmpopt_pa_start = 0;
>> +	rmpopt_pa_start = rmpopt_pa_end = 0;
>> +	rmpopt_wq = NULL;
>>  }
>>  
>>  void snp_shutdown(void)
>> @@ -599,6 +620,96 @@ static bool rmpopt_capable(void)
>>  	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>>  }
>>  
>> +/*
>> + * RMPOPT: F2 0F 01 FC
>> + *   Input:  RAX = system physical address (1GB aligned)
>> + *           RCX = operation type
>> + *   Output: CF set if the range was optimized
>> + */
>> +static inline bool __rmpopt(u64 pa_start, u64 op_type)
>> +{
>> +	bool optimized;
>> +
> 
> 	/*
> 	 * needs a comment here which says which binutils version
> 	 * supports the RMPOPT mnemonic.
> 	 */
>> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
>> +		     : "=@ccc" (optimized)
>> +		     : "a" (pa_start), "c" (op_type)
>> +		     : "memory", "cc");
>> +
>> +	return optimized;
>> +}
>> +
>> +static void rmpopt(u64 pa)
>> +{
>> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
>> +	u64 op_type = RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS;
> 
> 	enum rmpopt_op_type op = ...
> 
>> +
>> +	__rmpopt(pa_start, op_type);
> 
> Looks like the __rmpopt() carve out is not really necessary and you can merge
> it back into rmpopt().
> 
>> +}
>> +
>> +/*
>> + * 'val' is a system physical address.
>> + */
>> +static void rmpopt_smp(void *val)
> 
> You don't need that one - you can use rmpopt(). But keep on reading...
> 
>> +{
>> +	rmpopt((u64)val);
>> +}
>> +
>> +/*
>> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this
>> + * range of memory does not contain any SNP guest memory.
>> + */
> 
> Put that comment above rmpopt().

Will merge __rmpopt() into rmpopt(), drop rmpopt_smp(), and switch the op type to enum rmpopt_op_type. Will move the
descriptive comment above rmpopt(), and add a note that binutils doesn't support the RMPOPT mnemonic yet, hence the
.byte encoding. The CF result is unused on this path, so will drop the output operand.

> 
>> +static void rmpopt_work_handler(struct work_struct *work)
>> +{
>> +	cpumask_var_t follower_mask;
>> +	phys_addr_t pa;
> 
> So either phys_addr_t or u64 but not both for a pa.

Ok.

> 
>> +	int this_cpu;
>> +
>> +	pr_info("Attempt RMP optimizations on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
>> +		rmpopt_pa_start, rmpopt_pa_end);
> 
> This is going to spam dmesg every time the workqueue runs?
> 
> Nope, zap it.

Ok.

> 
>> +	if (!alloc_cpumask_var(&follower_mask, GFP_KERNEL)) {
>> +		pr_warn("RMP optimization pass skipped: cpumask allocation failed\n");
>> +		return;
>> +	}
> 
> Why? Why isn't the follower mask allocated once at init time?
> 

Will move the follower mask to a one-time allocation at setup (alongside rmpopt_cpumask and freed in
snp_cleanup_rmpopt()) and just recompute it per pass. The work handler no longer allocates/frees it, which also drops
the per-run allocation-failure path.

>> +
>> +	/*
>> +	 * 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.
>> +	 *
>> +	 * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
>> +	 * on every other primary thread. Followers are "designed to"
>> +	 * skip the scan if they see the "cached" scan results.
>> +	 *
>> +	 * Pin the worker to the current CPU for the leader loop so that
> 
> Isn't worker == leader here?

Right — the workqueue worker's CPU is the leader. Will reword the comment to use "leader"/"followers" consistently and drop
the redundant "worker" term.

>> +	 * this_cpu remains valid and the RMPOPT instruction executes on
>> +	 * the correct CPU. 
> 
>> Use migrate_disable() rather than get_cpu() to
>> +	 * prevent migration while still allowing preemption.
> 
> No need to explain that.
> 

Ok.

>> +	 */
>> +	migrate_disable();
>> +	this_cpu = smp_processor_id();
>> +
>> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
>> +		       topology_sibling_cpumask(this_cpu));
>> +
>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>> +		rmpopt(pa);
>> +
>> +	migrate_enable();
>> +
>> +	/*
>> +	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
>> +	 * intentionally not held here: CPU hotplug is disabled for the entire
>> +	 * time SNP is active (see snp_prepare()), and this work only runs while
>> +	 * SNP is active, so the follower set stays valid across the whole scan.
>> +	 */
>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>> +		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);
> 
> An IPI per 1G pa?!?!? On each CPU?!
> 
> Instead of IPIing each CPU and inside the handler, doing the loop?
> 
> Nope.
> 

You're right — an IPI per 1 GB is far too many. Will restructure to a single IPI per follower core: a new on_each_cpu()
callback can loop over the whole range on the CPU it runs on. The leader will call it directly (migrate-disabled) to populate
the RMP scan cache, then one on_each_cpu_mask() will run it on the remaining cores.

This will also fold nicely with the earlier cleanup:  __rmpopt() getting merged into rmpopt().

One important tradeoff to be aware of: each follower IPI handler will now run a 2048-iteration loop with IRQs disabled — 
but followers are RMP-scan cache hits (the leader populated the cache), so each rmpopt() there is cheap, and this only runs
at setup and guest-teardown re-optimization time.

Thanks,
Ashish

>> +
>> +	free_cpumask_var(follower_mask);
>> +}
> 
> Ok, enough for this part. Part II coming up later.
> 
> Thx.
>