Skip to contentSkip to conversation

Add RMPOPT support.

PATCH Superseded Ashish Kalra 4 participants 21 messages Sep 2 – Sep 9

Subsystems (6)
x86 KVM Crypto API KVM x86 +2 more subsystems

v13 of 16 Superseded Latest is v16 →

lore
Share & tools
lore

Ashish Kalra

author maintainer
#0 lore raw

v13 cover Add RMPOPT support.

From: Ashish Kalra <[email protected]>

In the SEV-SNP architecture, hypervisor and non-SNP guests are subject to RMP checks on writes to provide integrity of SEV-SNP guest memory.

The RMPOPT architecture enables optimizations whereby the RMP checks can be skipped if 1GB regions of memory are known to not contain any SNP guest memory.

RMPOPT is a new instruction designed to minimize the performance overhead of RMP checks for the hypervisor and non-SNP guests.

RMPOPT instruction currently supports two functions. In case of the verify and report status function the CPU will read the RMP contents, verify the entire 1GB region starting at the provided SPA is HV-owned. For the entire 1GB region it checks that all RMP entries in this region are HV-owned (i.e, not in assigned state) and then accordingly updates the RMPOPT table to indicate if optimization has been enabled and provide indication to software if the optimization was successful.

In case of report status function, the CPU returns the optimization status for the 1GB region.

The RMPOPT table is managed by a combination of software and hardware. Software uses the RMPOPT instruction to set bits in the table, indicating that regions of memory are entirely HV-owned. Hardware automatically clears bits in the RMPOPT table when RMP contents are changed during RMPUPDATE instruction.

For more information on the RMPOPT instruction, see the AMD64 RMPOPT technical documentation.

As SNP is enabled by default the hypervisor and non-SNP guests are subject to RMP write checks to provide integrity of SNP guest memory.

This patch-series adds support to enable RMP optimizations for up to 2TB of system RAM across the system and allow RMPUPDATE to disable those optimizations as SNP guests are launched.

Support for RAM larger than 2 TB will be added in follow-on series.

This series also adds support to disable CPU hotplug while SNP is active, as the SEV firmware enumerates CPUs at SNP initialization and is not aware of the OS bringing CPUs online or offline afterwards. This also keeps the set of CPUs stable for the asynchronous RMPOPT scan, so the per-core RMPOPT_BASE MSRs programmed during setup remain valid.

This series also introduces support to re-enable RMP optimizations during SNP guest termination, after guest pages have been converted back to shared.

RMP optimizations are performed asynchronously by queuing work on a dedicated workqueue after a 10 second delay.

Delaying work allows batching of multiple SNP guest terminations.

Once 1GB hugetlb guest_memfd support is merged, support for re-enabling RMPOPT optimizations during 1GB page cleanup will be added in follow-on series.

v13:
- 3/5 (Initialize RMPOPT MSRs): use cpu_primary_thread_mask directly instead of building a local rmpopt_cpumask -- all primary threads are online while SNP is active, so the two are equivalent. Rename snp_cleanup_rmpopt() to rmpopt_disable().
- 4/5 (async RMPOPT): drop the leader/follower bookkeeping. Warm the RMP scan cache once, then fan the RMPOPT out to cpu_primary_thread_mask; re-running RMPOPT on the warm-up CPU is a cache hit, so the separate follower cpumask and the cpumask_andnot() are no longer needed. Rename rmpopt_work_handler() to do_rmpopt_work() and drop the now-unused this_cpu.
- Add Reviewed-by: Tom Lendacky <[email protected]> to the series.

Review feedback from Borislav Petkov.

v12:
- Merged the former "Add interface to re-enable RMP optimizations" and "KVM: SEV: Perform RMP optimizations on SNP guest shutdown" patches into a single patch (now 5/5), since the interface has no user without the KVM caller. The series is now 5 patches.
- 2/5 (Disable CPU hotplug): drop a stray reference to later patches from the commit message and trim the snp_prepare() comment. Move cpu_hotplug_enable() to the end of snp_shutdown(), after clear_rmp() and the mfd_reconfigure() IPI, so no all-CPU operation runs after hotplug is re-enabled.
- 3/5 (Initialize RMPOPT MSRs): restructure snp_probe_rmptable_info() so the contiguous probe (which clears X86_FEATURE_RMPOPT) is the common fall-through, also covering X86_FEATURE_SEGMENTED_RMP advertised but disabled by firmware. Move snp_setup_rmpopt() to the end of __sev_snp_init_locked(). Give snp_cleanup_rmpopt() its own short comment in snp_shutdown().
- 4/5 (async RMPOPT): document the optimization trigger points in the commit message. Rename enum rmpopt_function to rmpopt_op_type. Merge __rmpopt() into rmpopt() and replace rmpopt_smp() with a new rmpopt_scan_range() that loops as the on_each_cpu() callback, so the follower scan issues one IPI per core rather than one per 1GB. Use u64 for physical addresses. Allocate the follower cpumask once at setup. Document rmpopt_wq as the setup sentinel. Move the RMPOPT_WORK_TIMEOUT define to the guest-shutdown patch (its only user) as (10 * MSEC_PER_SEC). Remove a spammy pr_info() and reword comments.
- 5/5 (Re-enable RMP optimizations on SNP guest shutdown): document why re-optimization is driven by guest teardown (the only event that returns guest memory to hypervisor ownership); reword the commit message (s/clear/disable/ for optimizations, drop "Conversely").

Review feedback from Borislav Petkov.

v11:
- Reordered so "Disable CPU hotplug while SNP is active" (2/6) precedes "Initialize RMPOPT configuration MSRs" (3/6): the RMPOPT setup/cleanup code is then introduced with CPU hotplug already disabled and never takes cpus_read_lock().
- 1/6 (cpufeatures): drop the tools/arch/x86/include/asm/cpufeatures.h change and adopt the commit message as applied by Boris.
- 2/6 (Disable CPU hotplug): reword the commit message. Drop the redundant cpus_read_lock()/cpus_read_unlock() in snp_prepare() in this same patch -- with hotplug disabled the online CPU mask is stable, so the read lock is not needed and hotplug handling has no hole when bisecting.
- 3/6 (Initialize RMPOPT MSRs): replace the rmpopt_capable bool with a small helper local to arch/x86/virt/svm/sev.c -- cpu_feature_enabled(X86_FEATURE_RMPOPT) && cc_platform_has(CC_ATTR_HOST_SEV_SNP) -- clearing X86_FEATURE_RMPOPT for a contiguous (non-segmented) RMP in snp_probe_rmptable_info() (runs at BSP init, before alternatives). Rename rmpopt_cleanup() to snp_cleanup_rmpopt() to match snp_setup_rmpopt(). Both are introduced without cpus_read_lock() (hotplug is already disabled by 2/6). Simplify the RMPOPT_BASE comments. Drop the now-unused <asm/sev.h> include from core.c.
- 4/6 (async RMPOPT): the follower scan is likewise introduced without cpus_read_lock(). Drop the cond_resched() calls (nops on x86). On re-initialization after a legacy SNP shutdown, re-queue the optimization pass instead of skipping it. pr_warn() on cpumask allocation failure.

Review feedback from Borislav Petkov and K Prateek Nayak.

v10:
- Rework the CPU-hotplug patch (3/6): disable CPU hotplug in snp_prepare(), before SnpEn is set, instead of late in __sev_snp_init_locked(), so no CPU can come online without SnpEn during SNP initialization (per upstream review). Tie hotplug to SnpEn: it stays disabled while SnpEn is set -- including across a failed SNP_INIT and across the legacy SNP_SHUTDOWN_EX path -- and is re-enabled only once the firmware clears SnpEn on the x86_snp_shutdown path. Drop the separate idempotent flag: snp_prepare() re-enables hotplug on its own early failure, and a kexec target that boots with SnpEn already set disables hotplug once in snp_rmptable_init(). Reword the commit log and comments accordingly.
- Emit a pr_warn() in rmpopt_work_handler() (4/6) when the follower cpumask allocation fails, instead of silently skipping the optimization pass.

Sashiko AI upstream review identified several of the above issues.

v9:
- Rename rmpopt_configured to rmpopt_capable.
- Make rmpopt_cpumask a cpumask_var_t (allocated/freed at setup/cleanup) instead of a static cpumask_t.
- Drop the v8 WARN_ON_ONCE() on the RMPOPT_BASE writes; use a plain wrmsrq_on_cpu(), matching the SNP MSR-write convention in this file.
- Disable CPU hotplug with cpu_hotplug_disable()/cpu_hotplug_enable() (per tglx); re-enable only on the full x86_snp_shutdown path.
- Simplify rmpopt_work_handler() to a single leader-then-followers path:
with CPU hotplug disabled while SNP is active and snp_prepare() requiring all CPUs online when RMPOPT_BASE is programmed, every core is always programmed, so the explicit-leader fallback is now unreachable. Drop it along with the v8 work_on_cpu()/rmpopt_leader_fn() helper.
- Drop the debugfs interface (was patch 7/7) and its report-only plumbing; observability will be revisited after this series is merged.
- Restrict snp_rmpopt_all_physmem()'s export to the kvm-amd module.
- Use scoped_guard(cpus_read_lock) for the per-CPU MSR and follower loops.

Sashiko AI upstream review identified several of the above issues.

v8:
- Add a new patch to disable CPU hotplug while SNP is active, keeping the CPU set stable for the RMPOPT work handler.
- Drop the setup_clear_cpu_cap(X86_FEATURE_RMPOPT) calls; the rmpopt_configured bool is the runtime guard.
- WARN_ON_ONCE() on the RMPOPT_BASE MSR writes that previously ignored their return value.
- Simplify rmpopt_work_handler() by removing the explicit-leader
fallback: with CPU hotplug disabled while SNP is active and snp_prepare() requiring all CPUs online when RMPOPT_BASE is programmed, every core is always programmed, so the running CPU can always be the leader. This drops the smp_call_function_single() fallback (and with it the AB-BA deadlock and IRQ-latency concerns) and collapses the leader selection into a single leader-then-followers path.
- Use mod_delayed_work() in snp_rmpopt_all_physmem() so the batching delay tracks the last SNP guest termination.

Sashiko AI code review identified several of the above issues.

v7:
- Sync tools/arch/x86/include/asm/cpufeatures.h to mirror the kernel header for X86_FEATURE_RMPOPT.
- Fix commit title to use X86_FEATURE_RMPOPT to match the code (was X86_FEATURE_AMD_RMPOPT).
- Add static bool rmpopt_configured, set only when segmented RMP setup succeeds in setup_rmptable(). Check rmpopt_configured alongside cpu_feature_enabled(X86_FEATURE_RMPOPT) in snp_setup_rmpopt() and snp_rmpopt_all_physmem(), because setup_clear_cpu_cap() is unreliable after alternatives are patched. Add snp_clear_rmpopt_configured() called from amd_cc_platform_clear() when CC_ATTR_HOST_SEV_SNP is cleared. Do not use __ro_after_init on rmpopt_configured since the writer snp_clear_rmpopt_configured() is not __init.
- Add cond_resched() to all three leader loops in rmpopt_work_handler() to prevent soft lockups on systems with up to 2TB of RAM.
- Add comment above __rmpopt() documenting the RMPOPT instruction encoding (F2 0F 01 FC) and register interface (RAX = system physical address input, RCX = operation type input, RFLAGS.CF = output).
Note: RMPOPT does not modify RAX unlike PVALIDATE/RMPUPDATE, so the existing "a" (input-only) constraint is correct.

Sashiko AI code review identified several of the above issues.

v6:
- Drop wrmsrq_on_cpus() helper; use for_each_cpu() with wrmsrq_on_cpu() instead, as RMPOPT_BASE MSR programming is not performance-critical.
- Rewrite rmpopt_work_handler() leader selection to use a local follower_mask copy instead of modifying the global rmpopt_cpumask. This eliminates the current_cpu_cleared tracking and the restore at the end, and removes the need for synchronization comments about transient cpumask inconsistency.
- Add three-way leader selection in rmpopt_work_handler():
1. Current CPU is a primary thread in cpumask: run leader locally.
2. Current CPU is a sibling thread whose primary is in cpumask:
run leader locally (RMPOPT_BASE MSR is per-core), remove the primary from followers via cpumask_andnot(topology_sibling_cpumask).
3. Current CPU's core has no RMPOPT_BASE MSR programmed: pick an explicit leader via cpumask_first() + smp_call_function_single() to avoid #UD, with cpus_read_lock() around the IPI loop.
- Add WARN_ON_ONCE guard for empty cpumask in the explicit leader fallback path, with migrate_enable() before goto out.
- Add .llseek = seq_lseek to rmpopt_table_fops for consistency with other seq_file-based debugfs files and to support tools like "less".
- Change debugfs file permissions from 0444 to 0400 to restrict access to root only.
- Add comment in rmpopt_table_seq_show() explaining why cpu_online_mask is safe: RMPOPT_BASE MSR is per-core and snp_prepare() ensures all CPUs are online when the MSR is programmed.

Sashiko AI code review identified several of the above issues.

v5:
- Introduce rmpopt_cleanup() to tear down workqueue, debugfs, cpumask, and MSR state, called from snp_shutdown().
- Introduce rmpopt_wq_mutex to serialize snp_setup_rmpopt(), snp_rmpopt_all_physmem(), and rmpopt_cleanup().
- Introduce rmpopt_show_mutex to serialize debugfs reporting of rmpopt_report_cpumask.
- Move snp_rmpopt_all_physmem() call after SNP DECOMMISSION during guest shutdown.
- Use migrate_disable()/migrate_enable() for CPU pinning in the rmpopt_work_handler() leader loop to maintain CPU affinity without disabling preemption for the entire RMPOPT scan.
- Add cpus_read_lock()/cpus_read_unlock() around the follower on_each_cpu_mask() loop in rmpopt_work_handler().
- Guard snp_setup_rmpopt() against re-initialization when SNP_SHUTDOWN_EX with x86_snp_shutdown=0 skips rmpopt_cleanup() but clears snp_initialized, preventing workqueue and resource leaks on repeated init/shutdown cycles.
- Replace setup_clear_cpu_cap() with pr_err() on alloc_workqueue() failure in snp_setup_rmpopt(), as setup_clear_cpu_cap() cannot be used after alternatives are patched; callers check rmpopt_wq != NULL as the runtime guard instead.
- Add pr_info() when RMPOPT coverage is capped at 2TB.
- Add comments noting CPU hotplug is not supported with SNP enabled and only online primary threads are covered by rmpopt_cpumask.
- Add comment in setup_rmptable() noting Segmented RMP must be enabled to enable RMPOPT.
- Simplify cpumask setup loop to set if primary thread rather than skip if not primary.
- Improve grammar and clarity in snp_setup_rmpopt() comments.
- Added Reviewed-by's.

Sashiko AI code review identified several of the above issues.

v4:
- Add new wrmsrq_on_cpus() helper to write same u64 value to a per-CPU MSR across a cpumask without per-cpu struct allocation overhead.
- Rename configure_and_enable_rmpopt() to snp_setup_rmpopt().
- Use wrmsrq_on_cpus() instead of wrmsrq_on_cpu() loop for programming RMPOPT_BASE MSRs.
- Add setup_clear_cpu_cap(X86_FEATURE_RMPOPT) if segmented RMP setup fails or workqueue allocation fails.
- Add X86_FEATURE_RMPOPT feature clear logic in amd_cc_platform_clear() for CC_ATTR_HOST_SEV_SNP.
- All of the above allow checking for only X86_FEATURE_RMPOPT for both RMPOPT setup/enable and RMP re-optimizations.
- Rename snp_perform_rmp_optimization() to snp_rmpopt_all_physmem().
- Split rmpopt() into rmpopt() and rmpopt_smp() for SMP callback use.
- Introduce separate rmpopt_report_cpumask for debugfs reporting, distinct from rmpopt_cpumask used for primary thread tracking.
- Remove snp_perform_rmp_optimization() call from __sev_snp_init_locked() and instead setup and enable RMPOPT after SNP is enabled and initialized.

v3:
- Drop all RMPOPT kthread support and introduce adding custom and dedicated workqueue to schedule delayed and asynchronous RMPOPT work.
- Drop the guest_memfd inode cleanup interface and add support to re-enable RMP optimizations during guest shutdown using the asynchronous and delayed workqueue interface.
- Introduce new __rmpopt() helper and rmpopt() and rmpopt_report_status() wrappers on top which use rax and rcx parameters to closely match RMPOPT specs.
- Use new optimized RMPOPT loop to issue RMPOPT instructions on all system RAM upto 2TB and all CPUs, by optimizing each range on one CPU first, then let other CPUs execute RMPOPT in parallel so they can skip most work as the range has already been optimized.
- Also add support for running the optimized RMPOPT loop only on one thread per core.
- Replace all PUD_SIZE references with SZ_1G to conform to 1GB regions as specified by RMPOPT specifications and not be dependent on PUD_SIZE which makes the RMPOPT patch-set independent of x86 page table sizes.
- Use wrmsrq_on_cpu() to program the RMPOPT_BASE MSR registers on all CPUs that removes all ugly casting to use on_each_cpu_mask().
- Fix inline commits and patch commit messages

v2:
- Drop all NUMA and Socket configuration and enablement support and enable RMPOPT support for up to 2TB of system RAM.
- Drop get_cpumask_of_primary_threads() and enable per-core RMPOPT base MSRs and issue RMPOPT instruction on all CPUs.
- Drop the configfs interface to manually re-enable RMP optimizations.
- Add new guest_memfd cleanup interface to automatically re-enable RMP optimizations during guest shutdown.
- Include references to the public RMPOPT documentation.
- Move debugfs directory for RMPOPT under architecuture specific parent directory.

Ashish Kalra (5):
x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag x86/sev: Disable CPU hotplug while SNP is active x86/sev: Initialize RMPOPT configuration MSRs x86/sev: Add support to perform RMP optimizations asynchronously x86/sev: Re-enable RMP optimizations on SNP guest shutdown

 arch/x86/include/asm/cpufeatures.h |   2 +-
 arch/x86/include/asm/msr-index.h   |   3 +
 arch/x86/include/asm/sev.h         |   4 +
 arch/x86/kernel/cpu/scattered.c    |   1 +
 arch/x86/kvm/svm/sev.c             |  10 ++
 arch/x86/virt/svm/sev.c            | 228 +++++++++++++++++++++++++++--
 drivers/crypto/ccp/sev-dev.c       |   2 +
 7 files changed, 235 insertions(+), 15 deletions(-)
Signature · 1 line
2.43.0
author maintainer
#1 lore raw

PATCH v13 1/5 x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag

From: Ashish Kalra <[email protected]>

Add a flag indicating whether RMPOPT instruction is supported.

RMPOPT is a new instruction that reduces the performance overhead of RMP checks for the hypervisor and non-SNP guests by allowing those checks to be skipped when 1-GB memory regions are known to contain no SEV-SNP guest memory.

For more information on the RMPOPT instruction, see the AMD64 RMPOPT technical documentation.

[ bp: Zap respective tools/ change. ]

---
 arch/x86/include/asm/cpufeatures.h | 2 +-
 arch/x86/kernel/cpu/scattered.c    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
arch/x86/include/asm/cpufeatures.h +1 -1
index f70ee74b5f92..3b5b32d3391b 100644
@@ -76,7 +76,7 @@
 #define X86_FEATURE_K8			( 3*32+ 4) /* Opteron, Athlon64 */
 #define X86_FEATURE_ZEN5		( 3*32+ 5) /* CPU based on Zen5 microarchitecture */
 #define X86_FEATURE_ZEN6		( 3*32+ 6) /* CPU based on Zen6 microarchitecture */
-/* Free                                 ( 3*32+ 7) */
+#define X86_FEATURE_RMPOPT		( 3*32+ 7) /* Support for AMD RMPOPT instruction */
 #define X86_FEATURE_CONSTANT_TSC	( 3*32+ 8) /* "constant_tsc" TSC ticks at a constant rate */
 /* free: was #define X86_FEATURE_UP	( 3*32+ 9) * "up" SMP kernel running on UP */
 #define X86_FEATURE_ART			( 3*32+10) /* "art" Always running timer (ART) */
arch/x86/kernel/cpu/scattered.c +1 -0
index 8665a6474806..d1795ce219da 100644
@@ -67,6 +67,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_PERFMON_V2,		CPUID_EAX,  0, 0x80000022, 0 },
 	{ X86_FEATURE_AMD_LBR_V2,		CPUID_EAX,  1, 0x80000022, 0 },
 	{ X86_FEATURE_AMD_LBR_PMC_FREEZE,	CPUID_EAX,  2, 0x80000022, 0 },
+	{ X86_FEATURE_RMPOPT,			CPUID_EDX,  0, 0x80000025, 0 },
 	{ X86_FEATURE_AMD_HTR_CORES,		CPUID_EAX, 30, 0x80000026, 0 },
 	{ 0, 0, 0, 0, 0 }
 };
Signature · 1 line
2.43.0
author maintainer
#2 lore raw

PATCH v13 2/5 x86/sev: Disable CPU hotplug while SNP is active

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

While SNP is active, every memory write is checked against the RMP to protect SNP guest memory. A core performs these RMP checks only once SNP has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is set on that core; the firmware requires the SNP-enable bit to be set on every present CPU before SNP initialization.

A core that is not SNP-enabled and not SNP-initialized performs no RMP checks at all, so there is no valid configuration with SNP active and any CPU exempt from RMP checks.

The firmware determines which CPUs are present from the processor and the BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates them at SNP init; it is not aware of the OS bringing CPUs online or offline afterwards.

SNP_INIT fails unless SnpEn is set on all CPUs, so a CPU that is offline when SNP_INIT is issued, does not have SnpEn set, SNP_INIT fails, and there can be no SNP guest memory. OS CPU hotplug can thus diverge from the firmware's expectations and break SNP.

Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before SNP is enabled, and re-enable it in snp_shutdown() once the firmware has disabled SNP.

If snp_prepare() fails before enabling SNP it re-enables hotplug itself; once SNP is enabled hotplug stays disabled, including across a failed SNP_INIT and across the legacy SNP_SHUTDOWN_EX path, both of which leave SNP enabled.

A kexec target that boots with SNP already enabled, disables hotplug once in snp_rmptable_init(), since snp_prepare() bails when SNP is already enabled.

With CPU hotplug now disabled while SNP is active, the online CPU mask is stable, so the cpus_read_lock() previously taken in snp_prepare() to iterate it is redundant. Drop cpus_read_lock()/cpus_read_unlock() here.

---
 arch/x86/virt/svm/sev.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)
arch/x86/virt/svm/sev.c +26 -10
index cff285d8ad8e..558f7924a3f8 100644
@@ -513,7 +513,6 @@ static void clear_hsave_pa(void *arg)
 
 int snp_prepare(void)
 {
-	int ret;
 	u64 val;
 
 	/*
@@ -526,14 +525,18 @@ int snp_prepare(void)
 
 	clear_rmp();
 
-	cpus_read_lock();
+	/*
+	 * No CPU may come online without SnpEn while SNP is active; disable
+	 * hotplug here and re-enable it in snp_shutdown().
+	 */
+	cpu_hotplug_disable();
 
 	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
-		ret = -EOPNOTSUPP;
+		cpu_hotplug_enable();
 		pr_warn("SNP init failed: not all CPUs online. (%*pbl online <-> %*pbl present masks).\n",
 			cpumask_pr_args(cpu_online_mask),
 			cpumask_pr_args(cpu_present_mask));
-		goto unlock;
+		return -EOPNOTSUPP;
 	}
 
 	wbinvd_on_all_cpus();
@@ -548,12 +551,7 @@ int snp_prepare(void)
 	/* SNP_INIT requires MSR_VM_HSAVE_PA to be cleared on all CPUs. */
 	on_each_cpu(clear_hsave_pa, NULL, 1);
 
-	ret = 0;
-
-unlock:
-	cpus_read_unlock();
-
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
 
@@ -567,6 +565,13 @@ void snp_shutdown(void)
 
 	clear_rmp();
 	on_each_cpu(mfd_reconfigure, NULL, 1);
+
+	/*
+	 * The firmware has disabled SNP (SnpEn is clear), so re-enable CPU
+	 * hotplug.  A legacy SNP shutdown returns above with SnpEn still set and
+	 * leaves hotplug disabled.
+	 */
+	cpu_hotplug_enable();
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
 
@@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
  */
 int __init snp_rmptable_init(void)
 {
+	u64 val;
+
 	if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
 		return -ENOSYS;
 
@@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
 	if (!setup_rmptable())
 		return -ENOSYS;
 
+	/*
+	 * On a kexec boot SNP may already be enabled (legacy firmware leaves
+	 * SnpEn set across shutdown), in which case snp_prepare() bails without
+	 * disabling CPU hotplug, so disable it here.
+	 */
+	rdmsrq(MSR_AMD64_SYSCFG, val);
+	if (val & MSR_AMD64_SYSCFG_SNP_EN)
+		cpu_hotplug_disable();
+
 	/*
 	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
 	 * notifier is invoked to do SNP IOMMU shutdown before kdump.
Signature · 1 line
2.43.0
#3 lore raw

Re: [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] PM Suspend (S3) bypasses CPU hotplug disable, leading to security breakdown - [High] CPU…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] PM Suspend (S3) bypasses CPU hotplug disable, leading to security breakdown
- [High] CPU hotplug remains enabled on kexec boot if setup_rmptable() fails --

commit c91487bb659a8ce73e581c086c8aa1aa19f4b9aa
Author: Ashish Kalra <[email protected]>
x86/sev: Disable CPU hotplug while SNP is active

This patch disables CPU hotplug while SNP is active to ensure no CPU comes online without the SnpEn bit set. It ties hotplug to the SNP-enable bit by disabling it in snp_prepare() and re-enabling it in snp_shutdown().

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
index cff285d8ad8e3..558f7924a3f85 100644

[ ... ]

arch/x86/virt/svm/sev.c int snp_prepare(void) · line 532

Quoted 10 lines
arch/x86/virt/svm/sev.c +5 -1
@@ -526,14 +525,18 @@ int snp_prepare(void)
 
 	clear_rmp();
 
-	cpus_read_lock();
+	/*
+	 * No CPU may come online without SnpEn while SNP is active; disable
+	 * hotplug here and re-enable it in snp_shutdown().
+	 */
+	cpu_hotplug_disable();

[Severity: Critical]
Does this sequence prevent CPUs from being onlined without SnpEn set during power management transitions?

While cpu_hotplug_disable() blocks sysfs-initiated hotplug operations, it appears that system suspend (S3) and hibernation (S4) bypass this mechanism. During suspend, the PM core uses freeze_secondary_cpus() and thaw_secondary_cpus(), which explicitly call _cpu_down() and _cpu_up() regardless of the cpu_hotplug_disabled counter.

Additionally, MSR_AMD64_SYSCFG is not included in the list of MSRs saved and restored during PM transitions in arch/x86/power/cpu.c.

If a system with SNP active goes to sleep and resumes, will the secondary CPUs wake up without the SnpEn bit set, potentially breaking the SEV-SNP security model by failing to enforce RMP checks?

[ ... ]

arch/x86/virt/svm/sev.c int __init snp_rmptable_init(void) · line 594

Quoted 12 lines
arch/x86/virt/svm/sev.c +2 -0
@@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
  */
 int __init snp_rmptable_init(void)
 {
+	u64 val;
+
 	if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
 		return -ENOSYS;
 
@@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
 	if (!setup_rmptable())
 		return -ENOSYS;

[Severity: High]
If setup_rmptable() fails and returns -ENOSYS early, do we leave CPU hotplug enabled even though the hardware might still have SNP enabled from a previous kernel?

When booting via kexec from an SNP-enabled kernel, the CPUs retain the SnpEn bit in MSR_AMD64_SYSCFG. If setup_rmptable() fails here (for example, due to a memremap failure), the function exits before reaching the new check that disables CPU hotplug.

Because the function failed, the kernel clears CC_ATTR_HOST_SEV_SNP. If a CPU is subsequently hotplugged via sysfs, the new kernel will not set SnpEn on it, resulting in a state where boot CPUs have SnpEn set but the hotplugged CPU does not.

Could this early return be moved after the MSR_AMD64_SYSCFG_SNP_EN check to ensure hotplug is disabled whenever the hardware indicates SNP is active?

Quoted 13 lines
arch/x86/virt/svm/sev.c +10 -0
+
+	/*
+	 * On a kexec boot SNP may already be enabled (legacy firmware leaves
+	 * SnpEn set across shutdown), in which case snp_prepare() bails without
+	 * disabling CPU hotplug, so disable it here.
+	 */
+	rdmsrq(MSR_AMD64_SYSCFG, val);
+	if (val & MSR_AMD64_SYSCFG_SNP_EN)
+		cpu_hotplug_disable();
+
 	/*
 	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
 	 * notifier is invoked to do SNP IOMMU shutdown before kdump.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author maintainer
#4 lore raw

Re: [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active

Quoted 41 lines · On 9/2/2026 4:51 PM, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] PM Suspend (S3) bypasses CPU hotplug disable, leading to security breakdown
- [High] CPU hotplug remains enabled on kexec boot if setup_rmptable() fails --

commit c91487bb659a8ce73e581c086c8aa1aa19f4b9aa
Author: Ashish Kalra <[email protected]>
x86/sev: Disable CPU hotplug while SNP is active

This patch disables CPU hotplug while SNP is active to ensure no CPU comes online without the SnpEn bit set. It ties hotplug to the SNP-enable bit by disabling it in snp_prepare() and re-enabling it in snp_shutdown().

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
index cff285d8ad8e3..558f7924a3f85 100644

[ ... ]

arch/x86/virt/svm/sev.c int snp_prepare(void) · line 532

Quoted 10 lines
arch/x86/virt/svm/sev.c +5 -1
@@ -526,14 +525,18 @@ int snp_prepare(void)
 
 	clear_rmp();
 
-	cpus_read_lock();
+	/*
+	 * No CPU may come online without SnpEn while SNP is active; disable
+	 * hotplug here and re-enable it in snp_shutdown().
+	 */
+	cpu_hotplug_disable();

[Severity: Critical]
Does this sequence prevent CPUs from being onlined without SnpEn set during power management transitions?

While cpu_hotplug_disable() blocks sysfs-initiated hotplug operations, it appears that system suspend (S3) and hibernation (S4) bypass this mechanism. During suspend, the PM core uses freeze_secondary_cpus() and thaw_secondary_cpus(), which explicitly call _cpu_down() and _cpu_up() regardless of the cpu_hotplug_disabled counter.

Quoted 6 lines

Additionally, MSR_AMD64_SYSCFG is not included in the list of MSRs saved and restored during PM transitions in arch/x86/power/cpu.c.

If a system with SNP active goes to sleep and resumes, will the secondary CPUs wake up without the SnpEn bit set, potentially breaking the SEV-SNP security model by failing to enforce RMP checks?

SEV-SNP hosts do not support suspend-to-RAM (S3) or hibernation (S4). The SNP security state that would have to survive a PM transition — the per-CPU SnpEn bit, the RMP table, and the SNP-locked AMD IOMMU — is not saved and restored across suspend/resume, so a resumed SNP host would be in an inconsistent state independently of CPU hotplug. (That is also why MSR_AMD64_SYSCFG is not in the PM save/restore set.)

This patch does not change any of that. It replaces the existing cpus_read_lock() in snp_prepare() with cpu_hotplug_disable(); neither touches the PM suspend/resume path (freeze_secondary_cpus() / thaw_secondary_cpus()). The patch's purpose is narrower: to stop a runtime, sysfs-initiated CPU online from bringing a CPU up without SnpEn while SNP is active. The S3/S4 vector is pre-existing.

Quoted 26 lines

[ ... ]

arch/x86/virt/svm/sev.c int __init snp_rmptable_init(void) · line 594

Quoted 12 lines
arch/x86/virt/svm/sev.c +2 -0
@@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
  */
 int __init snp_rmptable_init(void)
 {
+	u64 val;
+
 	if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
 		return -ENOSYS;
 
@@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
 	if (!setup_rmptable())
 		return -ENOSYS;

[Severity: High]
If setup_rmptable() fails and returns -ENOSYS early, do we leave CPU hotplug enabled even though the hardware might still have SNP enabled from a previous kernel?

When booting via kexec from an SNP-enabled kernel, the CPUs retain the SnpEn bit in MSR_AMD64_SYSCFG. If setup_rmptable() fails here (for example, due to a memremap failure), the function exits before reaching the new check that disables CPU hotplug.

Quoted 8 lines

Because the function failed, the kernel clears CC_ATTR_HOST_SEV_SNP. If a CPU is subsequently hotplugged via sysfs, the new kernel will not set SnpEn on it, resulting in a state where boot CPUs have SnpEn set but the hotplugged CPU does not.

Could this early return be moved after the MSR_AMD64_SYSCFG_SNP_EN check to ensure hotplug is disabled whenever the hardware indicates SNP is active?

On any platform that supports RMPOPT (which requires a segmented RMP), a kexec performs a full SNP shutdown: the CCP driver's .shutdown handler clears SnpEn (SNP_SHUTDOWN via __sev_snp_shutdown_locked). So on a kexec boot of such a platform SnpEn is already clear — snp_rmptable_init() reads it as clear and the machine takes the normal path, where snp_prepare() disables CPU hotplug. The "SnpEn still set from the previous kernel" case does not arise here.

That case only happens with legacy firmware that leaves SnpEn set across a kexec shutdown, and those platforms do not support RMPOPT. On such a platform, if setup_rmptable() additionally fails, SNP is not brought up at all — snp_rmptable_init() returns -ENOSYS and CC_ATTR_HOST_SEV_SNP is cleared — so no SNP guest runs. The SnpEn-set / hotplug-enabled state there is pre-existing SEV-SNP behaviour (snp_rmptable_init() did not touch CPU hotplug before this series), not a regression introduced here.

Thanks,
Ashish

Quoted 14 lines
Quoted 13 lines
arch/x86/virt/svm/sev.c +10 -0
+
+	/*
+	 * On a kexec boot SNP may already be enabled (legacy firmware leaves
+	 * SnpEn set across shutdown), in which case snp_prepare() bails without
+	 * disabling CPU hotplug, so disable it here.
+	 */
+	rdmsrq(MSR_AMD64_SYSCFG, val);
+	if (val & MSR_AMD64_SYSCFG_SNP_EN)
+		cpu_hotplug_disable();
+
 	/*
 	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
 	 * notifier is invoked to do SNP IOMMU shutdown before kdump.
author maintainer
#5 lore raw

PATCH v13 3/5 x86/sev: Initialize RMPOPT configuration MSRs

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

The new RMPOPT instruction helps manage per-CPU RMP optimization structures inside the CPU. It takes a 1GB-aligned physical address and either returns the status of the optimizations or tries to enable the optimizations.

Per-CPU RMPOPT tables support at most 2 TB of addressable memory for RMP optimizations.

Initialize the per-CPU RMPOPT table base to the starting physical address. This enables RMP optimization for up to 2 TB of system RAM on all CPUs.

Additionally, add support to setup and enable RMPOPT once SNP is enabled and initialized.

---
 arch/x86/include/asm/msr-index.h |  3 ++
 arch/x86/include/asm/sev.h       |  2 ++
 arch/x86/virt/svm/sev.c          | 58 +++++++++++++++++++++++++++++---
 drivers/crypto/ccp/sev-dev.c     |  2 ++
 4 files changed, 60 insertions(+), 5 deletions(-)
arch/x86/include/asm/msr-index.h +3 -0
index 3a8e51a0c9e8..1635e2e1c576 100644
@@ -761,6 +761,9 @@
 #define MSR_AMD64_SEG_RMP_ENABLED_BIT	0
 #define MSR_AMD64_SEG_RMP_ENABLED	BIT_ULL(MSR_AMD64_SEG_RMP_ENABLED_BIT)
 #define MSR_AMD64_RMP_SEGMENT_SHIFT(x)	(((x) & GENMASK_ULL(13, 8)) >> 8)
+#define MSR_AMD64_RMPOPT_BASE		0xc0010139
+#define MSR_AMD64_RMPOPT_ENABLE_BIT	0
+#define MSR_AMD64_RMPOPT_ENABLE		BIT_ULL(MSR_AMD64_RMPOPT_ENABLE_BIT)
 
 #define MSR_SVSM_CAA			0xc001f000
 
arch/x86/include/asm/sev.h +2 -0
index 9e7a077c445d..5638d09b5132 100644
@@ -662,6 +662,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
 	__snp_leak_pages(pfn, pages, true);
 }
 int snp_prepare(void);
+void snp_setup_rmpopt(void);
 void snp_shutdown(void);
 #else
 static inline bool snp_probe_rmptable_info(void) { return false; }
@@ -680,6 +681,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
 static inline void kdump_sev_callback(void) { }
 static inline void snp_fixup_e820_tables(void) {}
 static inline int snp_prepare(void) { return -ENODEV; }
+static inline void snp_setup_rmpopt(void) {}
 static inline void snp_shutdown(void) {}
 #endif
 
arch/x86/virt/svm/sev.c +53 -5

90 lines not on this page · raw message

drivers/crypto/ccp/sev-dev.c +2 -0
index f833cb7e4da3..287a8345854b 100644
@@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 
 	sev_es_tmr_size = SNP_TMR_SIZE;
 
+	snp_setup_rmpopt();
+
 	return 0;
 }
 
Signature · 1 line
2.43.0
#6 lore raw

Re: [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] CPU hotplug lock is leaked if SEV-SNP initialization fails. --

commit 16bd9580b377dc645541bd4f478c471c109e65ae
Author: Ashish Kalra <[email protected]>

x86/sev: Initialize RMPOPT configuration MSRs

This commit introduces the RMPOPT configuration MSRs and adds support to set up and enable RMPOPT once SNP is initialized. The RMPOPT table base is initialized to enable RMP optimization for up to 2 TB of system RAM.

drivers/crypto/ccp/sev-dev.c static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid) · line 1667

Quoted 10 lines
drivers/crypto/ccp/sev-dev.c +2 -0
index f833cb7e4da36..287a8345854ba 100644
@@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 
 	sev_es_tmr_size = SNP_TMR_SIZE;
 
+	snp_setup_rmpopt();
+

[Severity: High]
This is a pre-existing issue, but does __sev_snp_init_locked() permanently leak the CPU hotplug lock if SEV-SNP initialization fails?

If snp_prepare() succeeds, it disables CPU hotplug. However, if an error occurs later in the function (for example, if walk_iomem_res_desc() fails), the function returns the error code without re-enabling CPU hotplug:

drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
    ...
    rc = snp_prepare();
    if (rc)
        return rc;
    ...
    rc = walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_MEM, 0, ~0,
                             snp_range_list, snp_filter_reserved_mem_regions);
    if (rc) {
        dev_err(sev->dev,
                "SEV: SNP_INIT_EX walk_iomem_res_desc failed rc = %d\n", rc);
        return rc;
    }
    ...
}

This leaves cpu_hotplug_disabled strictly greater than zero, which would permanently prevent CPUs from being onlined or offlined. Does this function need an error handling path that calls cpu_hotplug_enable() before returning on these failure paths?

Quoted 2 lines
drivers/crypto/ccp/sev-dev.c +0 -0
 	return 0;
 }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
author maintainer
#7 lore raw

Re: [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs

Quoted 50 lines · On 9/2/2026 4:44 PM, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] CPU hotplug lock is leaked if SEV-SNP initialization fails. --

commit 16bd9580b377dc645541bd4f478c471c109e65ae
Author: Ashish Kalra <[email protected]>

x86/sev: Initialize RMPOPT configuration MSRs

This commit introduces the RMPOPT configuration MSRs and adds support to set up and enable RMPOPT once SNP is initialized. The RMPOPT table base is initialized to enable RMP optimization for up to 2 TB of system RAM.

drivers/crypto/ccp/sev-dev.c static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid) · line 1667

Quoted 10 lines
drivers/crypto/ccp/sev-dev.c +2 -0
index f833cb7e4da36..287a8345854ba 100644
@@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 
 	sev_es_tmr_size = SNP_TMR_SIZE;
 
+	snp_setup_rmpopt();
+

[Severity: High]
This is a pre-existing issue, but does __sev_snp_init_locked() permanently leak the CPU hotplug lock if SEV-SNP initialization fails?

If snp_prepare() succeeds, it disables CPU hotplug. However, if an error occurs later in the function (for example, if walk_iomem_res_desc() fails), the function returns the error code without re-enabling CPU hotplug:

drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
    ...
    rc = snp_prepare();
    if (rc)
        return rc;
    ...
    rc = walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_MEM, 0, ~0,
                             snp_range_list, snp_filter_reserved_mem_regions);
    if (rc) {
        dev_err(sev->dev,
                "SEV: SNP_INIT_EX walk_iomem_res_desc failed rc = %d\n", rc);
        return rc;
    }
    ...
}
Quoted 5 lines

This leaves cpu_hotplug_disabled strictly greater than zero, which would permanently prevent CPUs from being onlined or offlined. Does this function need an error handling path that calls cpu_hotplug_enable() before returning on these failure paths?

Thanks — but this is intentional, not a leak. CPU hotplug is tied to SnpEn, not to the success of __sev_snp_init_locked().

snp_prepare() disables hotplug and then sets SnpEn on all CPUs via on_each_cpu(snp_enable, ...) before it returns 0. So by the time walk_iomem_res_desc() (and the other post-snp_prepare() steps) run, SnpEn is already set on every online CPU and the machine is in SNP mode.

While SnpEn is set, a CPU must not come online without it — that is exactly what disabling hotplug guards against. Re-enabling hotplug on the walk_iomem_res_desc() failure path, with SnpEn still set, would reopen that unsafe window, so hotplug is deliberately left disabled there. It is re-enabled only once SnpEn is actually cleared, on the x86_snp_shutdown path: snp_shutdown() verifies SnpEn is clear and then calls cpu_hotplug_enable().

The one snp_prepare() failure that occurs before SnpEn is set — the "not all CPUs online" check — does re-enable hotplug itself. There is no failure path between cpu_hotplug_disable() and snp_enable() that can return with hotplug disabled and SnpEn clear (the intervening steps are void). So hotplug tracks SnpEn on every path.

Thanks,
Ashish

Quoted 3 lines
Quoted 2 lines
drivers/crypto/ccp/sev-dev.c +0 -0
 	return 0;
 }
author maintainer
#8 lore raw

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

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

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.

---
 arch/x86/virt/svm/sev.c | 113 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 111 insertions(+), 2 deletions(-)
arch/x86/virt/svm/sev.c +111 -2
index 07f178eb76c7..a4c0fe49b9ec 100644
@@ -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>
@@ -124,7 +125,16 @@ static void *rmp_bookkeeping __ro_after_init;
 
 static u64 probed_rmp_base, probed_rmp_size;
 
-static phys_addr_t rmpopt_pa_start;
+static u64 rmpopt_pa_start, rmpopt_pa_end;
+
+enum rmpopt_op_type {
+	RMPOPT_OP_VERIFY_AND_REPORT_STATUS,
+	RMPOPT_OP_REPORT_STATUS
+};
+
+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);
@@ -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)
+		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);
+
+	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.
+	 */
+	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.
+	 */
+	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 */
+	if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
+		pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
+			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.
+	 */
+	queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
+
+	pr_info("RMPOPT optimizations enabled\n");
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
 
Signature · 1 line
2.43.0
Reviewed-by on v13 4/5
#9 lore raw

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

Quoted 2 lines · Ashish Kalra

Add support for performing RMP optimizations asynchronously using a dedicated workqueue.

This is looking fine to me at this point:

#10 lore raw

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

In reply to Ashish Kalra
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Critical] Declare %rax as clobbered or output in rmpopt() to prevent infinite loops. - [High] Schedule…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Declare %rax as clobbered or output in rmpopt() to prevent infinite loops.
- [High] Schedule work in process context; executing on_each_cpu_mask triggers hard lockups.
- [High] Use ALIGN_DOWN for rmpopt_pa_end to prevent out-of-bounds RMP table reads. --

commit 0121e9d5bd127536db9548e1a9f202c9576ef5ff
Author: Ashish Kalra <[email protected]>

x86/sev: Add support to perform RMP optimizations asynchronously

This commit introduces support for asynchronous RMP checks on 1GB physical memory regions to optimize performance on hardware supporting RMPOPT. A dedicated workqueue triggers the optimization process during SNP initialization and after legacy shutdowns.

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
index 07f178eb76c7e..a4c0fe49b9ec7 100644

[ ... ]

Quoted 8 lines
arch/x86/virt/svm/sev.c +4 -0
@@ -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.
+ *

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 637

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+ * @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);
+
+	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
+		     :: "a" (pa_start), "c" (op)
+		     : "memory", "cc");

[Severity: Critical]
Does this inline assembly need to declare %rax as an output or clobber?

Since the RMPOPT hardware instruction returns a status code in %rax, could GCC assume %rax is unmodified because it is only provided as an input? If rmpopt() is inlined, GCC might allocate %rax for the induction variable pa in the calling loop. This would cause the loop step to compute incorrectly, potentially resulting in an infinite loop during boot.

Quoted 12 lines
arch/x86/virt/svm/sev.c +12 -0
+}
+
+/* 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)

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 659

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+{
+	/*
+	 * RMPOPT caches the results of a RMP table scan in reserved processor
+	 * memory, allowing future invocations to skip such costly operations.
+	 */
+	migrate_disable();
+	rmpopt_scan_range(NULL);
+	migrate_enable();
+
+	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);

[Severity: High]
Could executing on_each_cpu_mask() here with wait=true trigger hard lockups?

This call runs the rmpopt_scan_range() loop synchronously on all primary threads in an IPI context with hardirqs disabled. Since the loop can iterate up to 2048 times invoking the slow RMPOPT hardware instruction, is it possible for remote CPUs with cold caches (like on multi-socket systems) to stall for seconds inside the IPI handler and trigger an NMI watchdog panic?

Quoted 1 line
arch/x86/virt/svm/sev.c +1 -0
+}

[ ... ]

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 708

Quoted 6 lines
arch/x86/virt/svm/sev.c +2 -0
@@ -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);

[Severity: High]
Should rmpopt_pa_end be calculated using ALIGN_DOWN() instead of ALIGN()?

If max_pfn is not 1GB aligned, aligning upwards creates a hardware scanning range that extends into unbacked physical memory. When rmpopt_scan_range() processes this final 1GB block, could the hardware instruction attempt to read RMP entries beyond the allocated RMP table and cause a Machine Check Exception or memory corruption?

Quoted 7 lines
arch/x86/virt/svm/sev.c +7 -0
+
+	/* Limit memory scanning to 2TB of RAM */
+	if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
+		pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
+			rmpopt_pa_start + SZ_2T);
+		rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
+	}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
author maintainer
#11 lore raw

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

Quoted 45 lines · On 9/2/2026 4:57 PM, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Declare %rax as clobbered or output in rmpopt() to prevent infinite loops.
- [High] Schedule work in process context; executing on_each_cpu_mask triggers hard lockups.
- [High] Use ALIGN_DOWN for rmpopt_pa_end to prevent out-of-bounds RMP table reads. --

commit 0121e9d5bd127536db9548e1a9f202c9576ef5ff
Author: Ashish Kalra <[email protected]>

x86/sev: Add support to perform RMP optimizations asynchronously

This commit introduces support for asynchronous RMP checks on 1GB physical memory regions to optimize performance on hardware supporting RMPOPT. A dedicated workqueue triggers the optimization process during SNP initialization and after legacy shutdowns.

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
index 07f178eb76c7e..a4c0fe49b9ec7 100644

[ ... ]

Quoted 8 lines
arch/x86/virt/svm/sev.c +4 -0
@@ -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.
+ *

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 637

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+ * @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);
+
+	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
+		     :: "a" (pa_start), "c" (op)
+		     : "memory", "cc");

[Severity: Critical]
Does this inline assembly need to declare %rax as an output or clobber?

Quoted 6 lines

Since the RMPOPT hardware instruction returns a status code in %rax, could GCC assume %rax is unmodified because it is only provided as an input? If rmpopt() is inlined, GCC might allocate %rax for the induction variable pa in the calling loop. This would cause the loop step to compute incorrectly, potentially resulting in an infinite loop during boot.

RMPOPT does not modify RAX — its only output is the carry flag (RFLAGS.CF), which the asm already covers with the "cc" clobber. RAX (the 1GB-aligned SPA) and RCX (the op type) are inputs only and are left unchanged, unlike RMPREAD/PSMASH/RMPUPDATE which do return a status in RAX. So the input-only "a"(pa_start) constraint describes RMPOPT correctly; GCC may keep the loop's pa in RAX and the loop step is correct. (rmpopt() intentionally discards CF on this path; only the debug/report path reads it, via a separate asm with a "=@ccc" output.)

Quoted 26 lines
Quoted 12 lines
arch/x86/virt/svm/sev.c +12 -0
+}
+
+/* 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)

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 659

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+{
+	/*
+	 * RMPOPT caches the results of a RMP table scan in reserved processor
+	 * memory, allowing future invocations to skip such costly operations.
+	 */
+	migrate_disable();
+	rmpopt_scan_range(NULL);
+	migrate_enable();
+
+	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);

[Severity: High]
Could executing on_each_cpu_mask() here with wait=true trigger hard lockups?

Quoted 7 lines

This call runs the rmpopt_scan_range() loop synchronously on all primary threads in an IPI context with hardirqs disabled. Since the loop can iterate up to 2048 times invoking the slow RMPOPT hardware instruction, is it possible for remote CPUs with cold caches (like on multi-socket systems) to stall for seconds inside the IPI handler and trigger an NMI watchdog panic?

The fan-out is reached only after the single warm-up scan that precedes it:
do_rmpopt_work() runs rmpopt_scan_range() once on the current CPU under migrate_disable() (interrupts on, preemptible), which does the expensive RMP scan and stores the result in the RMPOPT table. All primary threads are programmed with the same RMPOPT_BASE, so that table is shared; the subsequent on_each_cpu_mask() therefore issues only cache-hit RMPOPTs — reads of the already-computed result, including on remote sockets — not re-scans.

Measured over a 2 TB window: the cache-hit fan-out is ~0.32 s per core — ~30x under the 10 s NMI watchdog even with ~2048 iterations and cold data caches. The IRQ-disabled window per follower is bounded by the cache-hit cost, not the scan cost.

Quoted 14 lines
Quoted 1 line
arch/x86/virt/svm/sev.c +1 -0
+}

[ ... ]

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 708

Quoted 6 lines
arch/x86/virt/svm/sev.c +2 -0
@@ -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);

[Severity: High]
Should rmpopt_pa_end be calculated using ALIGN_DOWN() instead of ALIGN()?

Quoted 5 lines

If max_pfn is not 1GB aligned, aligning upwards creates a hardware scanning range that extends into unbacked physical memory. When rmpopt_scan_range() processes this final 1GB block, could the hardware instruction attempt to read RMP entries beyond the allocated RMP table and cause a Machine Check Exception or memory corruption?

RMPOPT does not read outside the RMP table in a way that can fault. It only reads RMP table entries to determine whether the 1 GB region is entirely HV-owned; for any address in the region that lies outside the RMP table's range, the instruction simply treats that address as HV-owned. It does not raise a #MC and does not perform an out-of-bounds access.

Rounding rmpopt_pa_end up with ALIGN() is therefore intentional and safe: it lets the final, partially-populated 1 GB of RAM be optimized — the backed part is checked normally and any tail beyond the RMP table is treated as HV-owned — with no risk of reading past the table. Using ALIGN_DOWN() would instead leave that last partial-1 GB region un-optimized for no safety benefit, so ALIGN() is deliberate here.

Thanks,
Ashish

Quoted 9 lines
Quoted 8 lines
arch/x86/virt/svm/sev.c +7 -0
+
+	/* Limit memory scanning to 2TB of RAM */
+	if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
+		pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
+			rmpopt_pa_start + SZ_2T);
+		rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
+	}
maintainer
#12 lore raw

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

In reply to Ashish Kalra
Quoted 1 line · Ashish Kalra

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

s/Add support to perform/Perform/

Quoted 27 lines

From: Ashish Kalra <[email protected]>

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.

Quoted 6 lines

R-by's need to get dropped when a patch changes in more or less significant way.

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
@@ -561,10 +571,26 @@ static void rmpopt_disable(void)
 {
 	int cpu;
 

arch/x86/virt/svm/sev.c static void rmpopt_disable(void) · line 583

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+	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.

Quoted 19 lines
arch/x86/virt/svm/sev.c +8 -1
+		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);
 }
 
+/*

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 634

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+ * 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+ */
Quoted 13 lines
arch/x86/virt/svm/sev.c +10 -0
+	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;
+

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 654

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+	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.

Quoted 24 lines
arch/x86/virt/svm/sev.c +7 -0
+	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)
{

arch/x86/virt/svm/sev.c +10 -0
 	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.

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 689

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+	 */
+	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?

Quoted 12 lines
arch/x86/virt/svm/sev.c +8 -0
+	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;
 

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 710

Quoted 7 lines
arch/x86/virt/svm/sev.c +4 -0
@@ -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.

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 712

Quoted 2 lines
arch/x86/virt/svm/sev.c +2 -0
+	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.

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 720

Quoted 8 lines
arch/x86/virt/svm/sev.c +8 -0
+			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.

Quoted 9 lines
arch/x86/virt/svm/sev.c +3 -0
+	queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
+
+	pr_info("RMPOPT optimizations enabled\n");
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
 
Signature · 1 line
2.43.0
Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#13 lore raw

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

Hello Boris,

Quoted 39 lines · Borislav Petkov
Quoted 1 line · Ashish Kalra

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

s/Add support to perform/Perform/

Quoted 27 lines

From: Ashish Kalra <[email protected]>

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 20 lines
Quoted 5 lines
arch/x86/virt/svm/sev.c +0 -0
@@ -561,10 +571,26 @@ static void rmpopt_disable(void)
 {
 	int cpu;
 

arch/x86/virt/svm/sev.c static void rmpopt_disable(void) · line 583

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+	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.

Quoted 3 lines

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

}

Quoted 86 lines
Quoted 19 lines
arch/x86/virt/svm/sev.c +8 -1
+		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);
 }
 
+/*

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 634

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+ * 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+ */
Quoted 13 lines
arch/x86/virt/svm/sev.c +10 -0
+	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;
+

arch/x86/virt/svm/sev.c static bool rmpopt_capable(void) · line 654

Quoted 10 lines
arch/x86/virt/svm/sev.c +10 -0
+	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.

Quoted 28 lines
arch/x86/virt/svm/sev.c +7 -0
+	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)
{

arch/x86/virt/svm/sev.c +14 -0
 	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;
Quoted 9 lines

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 689

Quoted 6 lines
arch/x86/virt/svm/sev.c +6 -0
+	}
+
+	/*
+	 * 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

Quoted 48 lines
Quoted 12 lines
arch/x86/virt/svm/sev.c +8 -0
+	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;
 

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 710

Quoted 7 lines
arch/x86/virt/svm/sev.c +4 -0
@@ -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.

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 712

Quoted 2 lines
arch/x86/virt/svm/sev.c +2 -0
+	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.

arch/x86/virt/svm/sev.c void snp_setup_rmpopt(void) · line 720

Quoted 8 lines
arch/x86/virt/svm/sev.c +8 -0
+			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.

Quoted 9 lines
arch/x86/virt/svm/sev.c +3 -0
+	queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
+
+	pr_info("RMPOPT optimizations enabled\n");
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
 
Signature · 1 line
2.43.0
maintainer
#14 lore raw

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

Quoted 7 lines · Ashish
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

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.

Tom gave you a R-by on v12 before I asked you to axe off a bunch of stuff from that patch. Dave gave you a R-by to this revision which is still under discussion and you wanna keep it regardless.

There's a R-by Ackerley which you carry at least since v5:

https://lore.kernel.org/r/6f1ec3d8ebcf3aaceccc099c07d0deb545dd4ab9.1779133590.git.ashish.kalra@amd.com

to which Ackerley is STILL GIVING YOU review feedback ON THAT SAME THREAD:

https://lore.kernel.org/r/CAEvNRgGfyb7zvZ1u1j7YLomD%[email protected]

and yet you're still debating.

From where I'm standing, it looks like you don't understand how those tags should be used.

Do you need to go refresh up on the docs:

"Both Tested-by and Reviewed-by tags, once received on mailing list from tester or reviewer, should be added by author to the applicable patches when sending next versions. However if the patch has changed substantially in following version, these tags might not be applicable anymore and thus should be removed. Usually removal of someone's Acked-by, Tested-by or Reviewed-by tags should be mentioned in the patch changelog with an explanation (after the '---' separator)."

?

You're using Suggested-by tags also willy-nilly:

"A Suggested-by: tag indicates that the patch idea is suggested by the person named and ensures credit to the person for the idea: if we diligently credit our idea reporters, they will, hopefully, be inspired to help us again in the future. Note, this is one of only three tags you might be able to use without explicit permission of the person named (see 'Tagging people requires permission' below for details)."

So all 4 people have suggested this patch?

No, ofc not. You have simply received review comments from them which you've decided to integrate into your patch. This doesn't need a Suggested-by tag. This is normal patch review process. You should try it sometimes.

And this is damn well documented but you're still debating.

Well, you can debate all you want - those patches are not going anywhere until you do them right. This is solely your call.

Quoted 3 lines

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:

Yes, you basically do the *minimal* work that is absolutely necessary and leave everything else untouched because it is unnecessary complication to all the code and if one is going to toggle SNP and hotplug, then one has bigger problems than some leftover facilities.

Quoted 6 lines

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.

You can't:

system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND | __WQ_DEPRECATED, WQ_MAX_ACTIVE);
								   ^^^^^^^^^^^^^^^

	__WQ_DEPRECATED         = 1 << 19, /* internal: workqueue is deprecated */

So if you do an unbound workqueue and then block migration around it, you're basically doing a WQ_PERCPU one. So why don't you do one of those and drop the migration toggles around it?

All this talking is to get you to get the hint that *whatever* you do, it needs to have a good comment above it explaining why it has been chosen this way. Or put that info in the commit message.

So that people who look at that code in the future, can change it after knowing why.

And you should not do excessive commenting - it suffices if you put a couple of key comments which explain non-trivial things only. The rest people can figure out by simply reading the code.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#15 lore raw

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

Quoted 54 lines · Borislav Petkov
Quoted 7 lines · Ashish
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

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.

Tom gave you a R-by on v12 before I asked you to axe off a bunch of stuff from that patch. Dave gave you a R-by to this revision which is still under discussion and you wanna keep it regardless.

There's a R-by Ackerley which you carry at least since v5:

https://lore.kernel.org/r/6f1ec3d8ebcf3aaceccc099c07d0deb545dd4ab9.1779133590.git.ashish.kalra@amd.com

to which Ackerley is STILL GIVING YOU review feedback ON THAT SAME THREAD:

https://lore.kernel.org/r/CAEvNRgGfyb7zvZ1u1j7YLomD%[email protected]

and yet you're still debating.

From where I'm standing, it looks like you don't understand how those tags should be used.

Do you need to go refresh up on the docs:

"Both Tested-by and Reviewed-by tags, once received on mailing list from tester or reviewer, should be added by author to the applicable patches when sending next versions. However if the patch has changed substantially in following version, these tags might not be applicable anymore and thus should be removed. Usually removal of someone's Acked-by, Tested-by or Reviewed-by tags should be mentioned in the patch changelog with an explanation (after the '---' separator)."

?

You're using Suggested-by tags also willy-nilly:

"A Suggested-by: tag indicates that the patch idea is suggested by the person named and ensures credit to the person for the idea: if we diligently credit our idea reporters, they will, hopefully, be inspired to help us again in the future. Note, this is one of only three tags you might be able to use without explicit permission of the person named (see 'Tagging people requires permission' below for details)."

So all 4 people have suggested this patch?

Quoted 8 lines

No, ofc not. You have simply received review comments from them which you've decided to integrate into your patch. This doesn't need a Suggested-by tag. This is normal patch review process. You should try it sometimes.

And this is damn well documented but you're still debating.

Well, you can debate all you want - those patches are not going anywhere until you do them right. This is solely your call.

Thanks, Boris for the explanation and for quoting the documentation, I'll get these right in v14.

Tags: you're right, I was crediting reviewers as if they'd suggested the approach, when they were just reviewing.

I'll drop the Reviewed-by tags that no longer apply to this patch -Tom's (from v12, before the rework you asked for) and Ackerley's and note the removals in the changelog under the '---'.

On Suggested-by, I'll keep it only where the approach was actually proposed and drop the ones that were really just review comments, including my mis-tagging of your own feedback.

I'll apply the same cleanup across the rest of the series.

Quoted 9 lines
Quoted 4 lines

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:

Yes, you basically do the *minimal* work that is absolutely necessary and leave everything else untouched because it is unnecessary complication to all the code and if one is going to toggle SNP and hotplug, then one has bigger problems than some leftover facilities.

Re-init / hotplug: agreed - I'll do only the minimal work that's necessary and leave the rest untouched. Setup programs RMPOPT_BASE once and simply re-arms the pass on re-init; the disable path only cancels the pending work - no tearing down the queue or clearing MSRs/state. And I'll drop the re-init reprogramming I was defending: as you say, if SNP and hotplug are being toggled underneath this, that's a bigger problem than some leftover facilities.

Quoted 23 lines
Quoted 7 lines

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.

You can't:

system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND | __WQ_DEPRECATED, WQ_MAX_ACTIVE);
								   ^^^^^^^^^^^^^^^

	__WQ_DEPRECATED         = 1 << 19, /* internal: workqueue is deprecated */

So if you do an unbound workqueue and then block migration around it, you're basically doing a WQ_PERCPU one. So why don't you do one of those and drop the migration toggles around it?

All this talking is to get you to get the hint that *whatever* you do, it needs to have a good comment above it explaining why it has been chosen this way. Or put that info in the commit message.

Quoted 7 lines

So that people who look at that code in the future, can change it after knowing why.

And you should not do excessive commenting - it suffices if you put a couple of key comments which explain non-trivial things only. The rest people can figure out by simply reading the code.

Workqueue: As you indicated system_unbound_wq is __WQ_DEPRECATED, so I

won't use it. And you're right that migrate_disable() around the local scan makes an unbound queue pointless, so I'll switch to a WQ_PERCPU workqueue and drop the migration toggles. I'll add a short comment on why it's per-CPU (the warm-up scan runs pinned and primes the shared RMPOPT table so the on_each_cpu_mask() fan-out is only cache hits), and trim the redundant comments rather than over-commenting.

Will respin with all of this.

Thanks,
Ashish

author maintainer
#16 lore raw

PATCH v13 5/5 x86/sev: Re-enable RMP optimizations on SNP guest shutdown

In reply to Ashish Kalra

From: Ashish Kalra <[email protected]>

The RMPOPT table is a per-CPU table which indicates whether 1GB regions of physical memory are entirely hypervisor-owned.

When performing host memory accesses in hypervisor mode as well as non-SNP guest mode, the processor may consult the RMPOPT table to potentially skip an RMP access and improve performance.

Normal guest events disable RMP optimizations: pages are converted from shared to private as SNP guests are launched, and large pages are split and collapsed during guest operation -- both disable the RMPOPT optimizations for the affected 1GB regions.

When guests are torn down, their pages are converted back to shared, so those regions may become eligible for RMPOPT optimization again. Without some intervention, all RMP optimizations would eventually be lost, so re-optimize all of physical memory on SNP guest teardown.

Perform the re-optimization after a delay, using mod_delayed_work() so that the delay timer is reset on each call. This batches multiple guest terminations into a single pass: the re-optimization runs 10 seconds after the *last* termination rather than after the first. mod_delayed_work() also re-queues work that is already in-flight, so a re-scan request during an active scan is not silently dropped.

Guest teardown is currently the only event that returns guest memory to hypervisor ownership: SNP guests do not support ballooning or memory hotplug, so pages freed during a guest's lifetime remain guest-owned. It is therefore the only point at which memory becomes eligible for RMP re-optimization, which is why re-optimization is driven by guest teardown rather than by a periodic scan.

---
 arch/x86/include/asm/sev.h |  2 ++
 arch/x86/kvm/svm/sev.c     | 10 ++++++++++
 arch/x86/virt/svm/sev.c    | 27 +++++++++++++++++++++++++++
 3 files changed, 39 insertions(+)
arch/x86/include/asm/sev.h +2 -0
index 5638d09b5132..3235e171647d 100644
@@ -662,6 +662,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
 	__snp_leak_pages(pfn, pages, true);
 }
 int snp_prepare(void);
+void snp_rmpopt_all_physmem(void);
 void snp_setup_rmpopt(void);
 void snp_shutdown(void);
 #else
@@ -681,6 +682,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
 static inline void kdump_sev_callback(void) { }
 static inline void snp_fixup_e820_tables(void) {}
 static inline int snp_prepare(void) { return -ENODEV; }
+static inline void snp_rmpopt_all_physmem(void) {}
 static inline void snp_setup_rmpopt(void) {}
 static inline void snp_shutdown(void) {}
 #endif
arch/x86/kvm/svm/sev.c +10 -0
index 5705723f1f41..0babf96e38c6 100644
@@ -3032,6 +3032,16 @@ void sev_vm_destroy(struct kvm *kvm)
 		 */
 		if (snp_decommission_context(kvm))
 			return;
+
+		/*
+		 * Perform RMP optimizations on memory freed by terminating
+		 * guests.  The scan is deferred, so it normally runs after
+		 * sev_gmem_invalidate() has converted this guest's pages back to
+		 * shared, and picks them up then.  A very large guest whose
+		 * conversion has not finished by then is picked up by a later
+		 * teardown's scan.
+		 */
+		snp_rmpopt_all_physmem();
 	} else {
 		sev_unbind_asid(kvm, sev->handle);
 	}
arch/x86/virt/svm/sev.c +27 -0
index a4c0fe49b9ec..fb66a1aa6fc3 100644
@@ -659,6 +659,33 @@ static void do_rmpopt_work(struct work_struct *work)
 	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
 }
 
+/*
+ * Delay, in milliseconds, before the RMP re-optimization pass runs after an
+ * SNP guest is torn down.  snp_rmpopt_all_physmem() re-arms the delayed work
+ * with mod_delayed_work() on each teardown, so the pass fires this long after
+ * the last teardown.  This coalesces a burst of teardowns into a single scan
+ * and gives each guest's pages time to be converted back to the shared,
+ * hypervisor-owned state before the scan re-optimizes their 1GB regions.  The
+ * 10 second value is a heuristic trading re-optimization latency against
+ * scanning too eagerly.
+ */
+#define RMPOPT_WORK_TIMEOUT	(10 * MSEC_PER_SEC)
+
+void snp_rmpopt_all_physmem(void)
+{
+	if (!rmpopt_capable())
+		return;
+
+	guard(mutex)(&rmpopt_wq_mutex);
+
+	if (!rmpopt_wq)
+		return;
+
+	mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
+			 msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
+}
+EXPORT_SYMBOL_FOR_MODULES(snp_rmpopt_all_physmem, "kvm-amd");
+
 void snp_setup_rmpopt(void)
 {
 	u64 rmpopt_base;
Signature · 1 line
2.43.0
Reviewed-by on v13 5/5
#17 lore raw

Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown

Quoted 6 lines · Ashish Kalra

Perform the re-optimization after a delay, using mod_delayed_work() so that the delay timer is reset on each call. This batches multiple guest terminations into a single pass: the re-optimization runs 10 seconds after the *last* termination rather than after the first. mod_delayed_work() also re-queues work that is already in-flight, so a re-scan request during an active scan is not silently dropped.

Simple enough.

#18 lore raw

Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown

In reply to Ashish Kalra
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Redundant and costly RMP optimization scan in hardirq context causing severe CPU stalls --

commit f95e55ff4f3219c5a1e0b2d2fc34fde539f227f7
Author: Ashish Kalra <[email protected]> x86/sev: Re-enable RMP optimizations on SNP guest shutdown

This commit introduces a mechanism to re-optimize memory on SNP guest shutdown using a delayed work queue to batch multiple terminations.

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
index a4c0fe49b9ec7..fb66a1aa6fc35 100644

[ ... ]

Quoted 7 lines
arch/x86/virt/svm/sev.c +3 -0
@@ -659,6 +659,33 @@ static void do_rmpopt_work(struct work_struct *work)
 	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
 }
 
+/*
+ * Delay, in milliseconds, before the RMP re-optimization pass runs after an
+ * SNP guest is torn down.

[ ... ]

arch/x86/virt/svm/sev.c static void do_rmpopt_work(struct work_struct *work) · line 686

Quoted 13 lines
arch/x86/virt/svm/sev.c +13 -0
+void snp_rmpopt_all_physmem(void)
+{
+	if (!rmpopt_capable())
+		return;
+
+	guard(mutex)(&rmpopt_wq_mutex);
+
+	if (!rmpopt_wq)
+		return;
+
+	mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
+			 msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
+}

[Severity: High]
This is a pre-existing issue, but queueing the do_rmpopt_work task upon guest teardown exposes the host to severe CPU stalls.

Looking at do_rmpopt_work, it broadcasts an IPI to all primary threads:

arch/x86/virt/svm/sev.c:do_rmpopt_work() {
    ...
    migrate_disable();
    rmpopt_scan_range(NULL);
    migrate_enable();

    on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
}

The IPI handler rmpopt_scan_range executes the rmpopt hardware instruction in hardirq context over up to 2TB of memory:

arch/x86/virt/svm/sev.c:rmpopt_scan_range() {
    ...
    for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
        rmpopt(pa);
    ...
}

Since rmpopt is a very slow instruction, does iterating it over massive memory ranges inside an IPI handler lead to soft lockups or host denial of service, especially if frequent VM churn repeatedly tears down SNP guests to trigger this scan?

Additionally, because rmpopt_wq is created as WQ_UNBOUND, the worker thread might run on an SMT sibling. If so, wouldn't the initial local scan execute on a CPU where MSR_AMD64_RMPOPT_BASE was never configured, potentially leading to undefined behavior or faults?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
author maintainer
#19 lore raw

Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown

Quoted 62 lines · On 9/2/2026 5:01 PM, [email protected] wrote:

Pre-existing issues:
- [High] Redundant and costly RMP optimization scan in hardirq context causing severe CPU stalls --

commit f95e55ff4f3219c5a1e0b2d2fc34fde539f227f7
Author: Ashish Kalra <[email protected]> x86/sev: Re-enable RMP optimizations on SNP guest shutdown

This commit introduces a mechanism to re-optimize memory on SNP guest shutdown using a delayed work queue to batch multiple terminations.

Quoted 4 lines
arch/x86/virt/svm/sev.c +0 -0
index a4c0fe49b9ec7..fb66a1aa6fc35 100644

[ ... ]

Quoted 7 lines
arch/x86/virt/svm/sev.c +3 -0
@@ -659,6 +659,33 @@ static void do_rmpopt_work(struct work_struct *work)
 	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
 }
 
+/*
+ * Delay, in milliseconds, before the RMP re-optimization pass runs after an
+ * SNP guest is torn down.

[ ... ]

arch/x86/virt/svm/sev.c static void do_rmpopt_work(struct work_struct *work) · line 686

Quoted 13 lines
arch/x86/virt/svm/sev.c +13 -0
+void snp_rmpopt_all_physmem(void)
+{
+	if (!rmpopt_capable())
+		return;
+
+	guard(mutex)(&rmpopt_wq_mutex);
+
+	if (!rmpopt_wq)
+		return;
+
+	mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
+			 msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
+}

[Severity: High]
This is a pre-existing issue, but queueing the do_rmpopt_work task upon guest teardown exposes the host to severe CPU stalls.

Looking at do_rmpopt_work, it broadcasts an IPI to all primary threads:

arch/x86/virt/svm/sev.c:do_rmpopt_work() {
    ...
    migrate_disable();
    rmpopt_scan_range(NULL);
    migrate_enable();

    on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
}

The IPI handler rmpopt_scan_range executes the rmpopt hardware instruction in hardirq context over up to 2TB of memory:

arch/x86/virt/svm/sev.c:rmpopt_scan_range() {
    ...
    for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
        rmpopt(pa);
    ...
}
Quoted 4 lines

Since rmpopt is a very slow instruction, does iterating it over massive memory ranges inside an IPI handler lead to soft lockups or host denial of service, especially if frequent VM churn repeatedly tears down SNP guests to trigger this scan?

Two things bound this:

1. Coalescing. snp_rmpopt_all_physmem() uses mod_delayed_work() with a 10 s delay that is reset on every call, so a burst of guest teardowns does not queue one scan per teardown — the pass runs once, 10 s after the last teardown. Frequent VM churn is coalesced into a single pass, not amplified.

2. The IPI does not run an uncached scan. do_rmpopt_work() first runs rmpopt_scan_range() once on the worker's own CPU under migrate_disable() (interrupts on, preemptible), which does the expensive part and populates the shared RMPOPT table — all primary threads share the same RMPOPT_BASE. Only then does on_each_cpu_mask() fan out, and those RMPOPTs are cache hits (reads of the already-computed result), so the IRQ-disabled window per CPU is bounded by the cache-hit cost, not the scan cost — measured at ~320 ms over the largest configuration tested, well under the soft-lockup/NMI thresholds.

Quoted 5 lines

Additionally, because rmpopt_wq is created as WQ_UNBOUND, the worker thread might run on an SMT sibling. If so, wouldn't the initial local scan execute on a CPU where MSR_AMD64_RMPOPT_BASE was never configured, potentially leading to undefined behavior or faults?

MSR_AMD64_RMPOPT_BASE has core scope, so programming it on each core's primary thread configures it for both SMT threads of that core. A WQ_UNBOUND worker that happens to run on a secondary thread therefore still executes RMPOPT against a validly-configured RMPOPT_BASE (its core's), and since CPU hotplug is disabled while SNP is active, every core stays configured. There is no CPU on which RMPOPT_BASE is unconfigured, so no fault or undefined behaviour.

Thanks,
Ashish

maintainer
#20 lore raw

Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown

In reply to Ashish Kalra

arch/x86/kvm/svm/sev.c void sev_vm_destroy(struct kvm *kvm) · line 3043

Quoted 8 lines · Ashish Kalra
arch/x86/kvm/svm/sev.c +8 -0
+		/*
+		 * Perform RMP optimizations on memory freed by terminating
+		 * guests.  The scan is deferred, so it normally runs after
+		 * sev_gmem_invalidate() has converted this guest's pages back to
+		 * shared, and picks them up then.  A very large guest whose
+		 * conversion has not finished by then is picked up by a later
+		 * teardown's scan.
+		 */

Move that comment over the function definition.

Quoted 10 lines

+/*
+ * Delay, in milliseconds, before the RMP re-optimization pass runs after an
+ * SNP guest is torn down. snp_rmpopt_all_physmem() re-arms the delayed work
+ * with mod_delayed_work() on each teardown, so the pass fires this long after
+ * the last teardown. This coalesces a burst of teardowns into a single scan
+ * and gives each guest's pages time to be converted back to the shared,
+ * hypervisor-owned state before the scan re-optimizes their 1GB regions. The
+ * 10 second value is a heuristic trading re-optimization latency against
+ * scanning too eagerly.
+ */

Simplify:

"Delay, in milliseconds, before the RMP re-optimization pass runs after an SNP guest is torn down, passed as the delay to mod_delayed_work(). This coalesces a burst of teardowns into a single scan and gives each guest's pages time to be converted back to the shared, hypervisor-owned state. The 10 second value is a heuristic trading re-optimization latency against scanning too eagerly."

Quoted 1 line

+#define RMPOPT_WORK_TIMEOUT (10 * MSEC_PER_SEC)

Signature · 4 lines
Regards/Gruss,
    Boris.

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