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

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

Hello Boris,

On 9/4/2026 8:29 PM, Borislav Petkov wrote:
> On Wed, Sep 02, 2026 at 09:28:57PM +0000, Ashish Kalra wrote:
>> Subject: Re: [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously
> 
> s/Add support to perform/Perform/
> 
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> When SNP is enabled, all writes to memory are checked to ensure memory
>> integrity. This imposes performance overhead on the 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 SNP guest memory.
>>
>> Add support for performing RMP optimizations asynchronously using a
>> dedicated workqueue.
>>
>> At RMP initialization time, run an optimization pass over all physical
>> memory (up to 2TB of system RAM, starting from the lowest physical
>> memory address aligned down to a 1GB boundary), skipping RMP checks for
>> 1GB regions that do not contain SNP guest memory (excluding preassigned
>> pages such as the RMP table and firmware pages).
>>
>> As SNP guests are launched, RMPUPDATE assigns their private pages to
>> guest-owned state; when such a page falls within an optimized 1GB
>> region, the hardware clears that region's RMPOPT optimization and RMP
>> checks resume there to protect the guest memory.
>>
>> Since launching SNP guests clears these optimizations, perform them
>> again asynchronously using the dedicated workqueue.
>>
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
>> Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
>> Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
>> Reviewed-by: Ackerley Tng <ackerleytng@google.com>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> 
> R-by's need to get dropped when a patch changes in more or less significant
> way.

Tom and Dave gave their R-by's on v12/v13 series, so probably i will keep their
R-b's.

> 
>> @@ -561,10 +571,26 @@ static void rmpopt_disable(void)
>>  {
>>  	int cpu;
>>  
>> +	guard(mutex)(&rmpopt_wq_mutex);
>> +
>> +	/*
>> +	 * rmpopt_wq is non-NULL only after RMPOPT has been fully set up: the
>> +	 * workqueue is allocated and the RMPOPT_BASE MSRs are programmed.
>> +	 * snp_setup_rmpopt() resets it to NULL if any of those steps fail, so a
>> +	 * NULL rmpopt_wq means nothing was set up and there is nothing to tear
>> +	 * down.
>> +	 */
>> +	if (!rmpopt_wq)
> 
> Now take your patch and rip all that gunk which destroys the setup work done
> by snp_setup_rmpopt(). Instead, you init things once and do not touch them
> even if RMP optimizations are disabled.
> 
> In case they get enabled again later, you simply reactivate them instead of
> doing useless setup work all over again.
> 

So, you want me to set up once, never tear down, on re-enable just reactivate. 

So rmpopt_disable() should stop destroying the workqueue, clearing the RMPOPT_BASE MSRs, and resetting pa_start/pa_end.

One thing i need to handle here: The current rmpopt_disable() clears the MSRs for a reason: it runs only on a full SNP
shutdown (SnpEn cleared), right before cpu_hotplug_enable(). Once hotplug is back on, a primary thread can offline -> online
and lose its RMPOPT_BASE (reset to 0, no cpuhp callback - which is only safe because hotplug is disabled while SNP is active).
If we simply stop clearing MSRs but keep the current re-init fast-path that skips MSR programming, a re-init after such a cycle 
could run rmpopt() on a core with RMPOPT_BASE=0.

So the fix is: keep the setup allocated once, but make setup always (re)program RMPOPT_BASE — then we never need to clear it on
disable, and the cycle case is covered on re-init. Proposed fix: 

  /* full shutdown: just stop a pending pass; leave the setup intact */
  static void rmpopt_disable(void)
  {
        guard(mutex)(&rmpopt_wq_mutex);
        cancel_delayed_work_sync(&rmpopt_delayed_work);
  }

  void snp_setup_rmpopt(void)
  {
        ...
        if (!rmpopt_capable())
                return;
        guard(mutex)(&rmpopt_wq_mutex);

        /* allocate the workqueue exactly once, keep it for the kernel's life */
        if (!rmpopt_wq) {
                rmpopt_wq = alloc_workqueue(...);   /* or use system_unbound_wq, see below */
                if (!rmpopt_wq) { pr_err(...); return; }
                INIT_DELAYED_WORK(&rmpopt_delayed_work, do_rmpopt_work);
        }

        /* always (re)program MSRs + range, so a hotplug cycle across a full
           shutdown can't leave a core with a stale RMPOPT_BASE */
        rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
        rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
        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");
  }


>> +		return;
>> +
>> +	cancel_delayed_work_sync(&rmpopt_delayed_work);
>> +	destroy_workqueue(rmpopt_wq);
>> +
>>  	for_each_cpu(cpu, cpu_primary_thread_mask)
>>  		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
>>  
>> -	rmpopt_pa_start = 0;
>> +	rmpopt_pa_start = rmpopt_pa_end = 0;
>> +	rmpopt_wq = NULL;
>>  }
>>  
>>  void snp_shutdown(void)
>> @@ -595,6 +621,44 @@ static bool rmpopt_capable(void)
>>  	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>>  }
>>  
>> +/*
>> + * 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)
>> +{
>> +	enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
>> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
>> +
> 
> 	/* Supported by binutils 2.48+ */
> 
>> +	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 do_rmpopt_work(struct work_struct *work)
>> +{
>> +	/*
>> +	 * RMPOPT caches the results of a RMP table scan in reserved processor
>> +	 * memory, allowing future invocations to skip such costly operations.
>> +	 */
> 
> We know already. Drop this comment.
> 
>> +	migrate_disable();
>> +	rmpopt_scan_range(NULL);
>> +	migrate_enable();
>> +
>> +	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
>> +}
>> +
>>  void snp_setup_rmpopt(void)
>>  {
>>  	u64 rmpopt_base;
>> @@ -603,6 +667,34 @@ void snp_setup_rmpopt(void)
>>  	if (!rmpopt_capable())
>>  		return;
>>  
>> +	guard(mutex)(&rmpopt_wq_mutex);
>> +
>> +	/*
>> +	 * On re-initialization after a legacy SNP shutdown (SNP_SHUTDOWN_EX
>> +	 * with x86_snp_shutdown=0), snp_shutdown() and thus rmpopt_disable() are
>> +	 * skipped, so the workqueue, delayed work and per-CPU RMPOPT_BASE MSRs
>> +	 * are still set up and valid (SnpEn stayed set and CPU hotplug stayed
>> +	 * disabled).  Rather than re-doing the setup, which would leak the
>> +	 * existing state, just re-queue the optimization pass to re-optimize any
>> +	 * memory the previous SNP session de-optimized.
>> +	 */
>> +	if (rmpopt_wq) {
>> +		queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * Create an RMPOPT-specific workqueue to avoid scheduling
>> +	 * RMPOPT workitem on the global system workqueue.
>> +	 */
> 
> Why?
> 

A full‑physmem RMPOPT pass is a warm‑up scan plus an IPI fan‑out over up to 2TB. The default system_wq is per‑CPU and concurrency‑managed,
a long‑running item there runs on the queueing CPU's worker pool and can stall (or be stalled by) other work on that pool. So we wanted it
off system_wq and use a dedicated RMPOPT specific workqueue.

Another thing i looked at is for long-running unbound work, probably the standard shared queue is system_unbound_wq, which is probably built
for this use case and won't clog the per-CPU system_wq.

If we switch to it, there's no workqueue to allocate or free — the setup/teardown "gunk" disappears entirely (static
DECLARE_DELAYED_WORK, queue/mod_delayed_work(system_unbound_wq, …)), which satisfies your "init once, don't touch." 

Additionally, with system_unbound_wq and a static delayed_work there's no pointer to guard, and queue/mod/cancel are already
synchronized by the workqueue core, so rmpopt_wq_mutex can also be dropped.

Thanks,
Ashish

>> +	rmpopt_wq = alloc_workqueue("rmpopt_wq", WQ_UNBOUND, 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;
>>  
>> @@ -612,6 +704,23 @@ 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);
>> +
>> +	/* Limit memory scanning to 2TB of RAM */
> 
> No need for that comment - we know.
> 
>> +	if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
>> +		pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
> 
> No need for that print - nothing we can do about it anyway.
> 
>> +			rmpopt_pa_start + SZ_2T);
>> +		rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
>> +	}
>> +
>> +	/*
>> +	 * Once all per-CPU RMPOPT tables have been configured, enable RMPOPT
>> +	 * optimizations on all physical memory.
>> +	 */
> 
> Drop this comment too.
> 
>> +	queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
>> +
>> +	pr_info("RMPOPT optimizations enabled\n");
>>  }
>>  EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
>>  
>> -- 
>> 2.43.0
>>
>