[PATCH v11 0/6] Add RMPOPT support.

[PATCH v11 0/6] Add RMPOPT support.

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

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.

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 (6):
  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: Add interface to re-enable RMP optimizations.
  KVM: SEV: Perform 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            | 269 +++++++++++++++++++++++++++--
 drivers/crypto/ccp/sev-dev.c       |   3 +
 7 files changed, 280 insertions(+), 12 deletions(-)

-- 
2.43.0

[PATCH v11 1/6] x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

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. ]

Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Link: https://patch.msgid.link/39e9ee269a572c516a3f4e937bfe12d00697d5e6.1782841284.git.ashish.kalra@amd.com
---
 arch/x86/include/asm/cpufeatures.h | 2 +-
 arch/x86/kernel/cpu/scattered.c    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 3d0940a3b9f3..dbccde9ee5cd 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -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) */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 937129ce6a96..021c0bf22de2 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -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 }
 };
-- 
2.43.0

[PATCH v11 1/6] x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

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. ]

Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Link: https://patch.msgid.link/39e9ee269a572c516a3f4e937bfe12d00697d5e6.1782841284.git.ashish.kalra@amd.com
---
 arch/x86/include/asm/cpufeatures.h | 2 +-
 arch/x86/kernel/cpu/scattered.c    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 3d0940a3b9f3..dbccde9ee5cd 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -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) */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 937129ce6a96..021c0bf22de2 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -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 }
 };
-- 
2.43.0

[PATCH v11 2/6] x86/sev: Disable CPU hotplug while SNP is active

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

While SNP is active, every memory write is checked against the RMP to
protect SEV-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.
The RMPOPT setup and cleanup added later are introduced after this patch
and never take the lock for the same reason.

Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/virt/svm/sev.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index cff285d8ad8e..e2f69fba0938 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -513,7 +513,6 @@ static void clear_hsave_pa(void *arg)
 
 int snp_prepare(void)
 {
-	int ret;
 	u64 val;
 
 	/*
@@ -526,14 +525,21 @@ int snp_prepare(void)
 
 	clear_rmp();
 
-	cpus_read_lock();
+	/*
+	 * Disable CPU hotplug before enabling SNP: no CPU may come online
+	 * without SnpEn while SNP is active, and none may go offline during
+	 * enable.  This keeps cpu_online_mask stable for the check and the
+	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
+	 * re-enabled in snp_shutdown() once the firmware disables SNP.
+	 */
+	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 +554,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");
 
@@ -565,6 +566,13 @@ void snp_shutdown(void)
 	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
 		return;
 
+	/*
+	 * 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();
+
 	clear_rmp();
 	on_each_cpu(mfd_reconfigure, NULL, 1);
 }
@@ -577,6 +585,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 +596,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.
-- 
2.43.0

[PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

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.

Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/include/asm/msr-index.h |  3 ++
 arch/x86/include/asm/sev.h       |  2 +
 arch/x86/virt/svm/sev.c          | 67 +++++++++++++++++++++++++++++---
 drivers/crypto/ccp/sev-dev.c     |  3 ++
 4 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 18c4be75e927..d2cb0a7cd0a2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -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
 
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 594cfa19cbd4..6fd72a44a51e 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -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
 
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index e2f69fba0938..8bfd80284836 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -124,6 +124,9 @@ static void *rmp_bookkeeping __ro_after_init;
 
 static u64 probed_rmp_base, probed_rmp_size;
 
+static cpumask_var_t rmpopt_cpumask;
+static phys_addr_t rmpopt_pa_start;
+
 static LIST_HEAD(snp_leaked_pages_list);
 static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
 
@@ -558,6 +561,17 @@ int snp_prepare(void)
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
 
+static void snp_cleanup_rmpopt(void)
+{
+	int cpu;
+
+	for_each_cpu(cpu, rmpopt_cpumask)
+		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
+
+	free_cpumask_var(rmpopt_cpumask);
+	rmpopt_pa_start = 0;
+}
+
 void snp_shutdown(void)
 {
 	u64 syscfg;
@@ -567,10 +581,11 @@ void snp_shutdown(void)
 		return;
 
 	/*
-	 * 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.
+	 * Clear the RMPOPT_BASE MSRs while CPU hotplug is still disabled, then
+	 * re-enable hotplug now that the firmware has disabled SNP.  A legacy SNP
+	 * shutdown returns above with SnpEn still set and leaves hotplug disabled.
 	 */
+	snp_cleanup_rmpopt();
 	cpu_hotplug_enable();
 
 	clear_rmp();
@@ -578,6 +593,46 @@ void snp_shutdown(void)
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
 
+static bool rmpopt_capable(void)
+{
+	return cpu_feature_enabled(X86_FEATURE_RMPOPT) &&
+	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
+}
+
+void snp_setup_rmpopt(void)
+{
+	u64 rmpopt_base;
+	int cpu;
+
+	if (!rmpopt_capable())
+		return;
+
+	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
+		pr_err("Failed to allocate RMPOPT cpumask\n");
+		return;
+	}
+
+	/*
+	 * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
+	 * to set up the RMPOPT_BASE MSR.  All primary threads are online,
+	 * otherwise SNP would not have been enabled.
+	 */
+	for_each_online_cpu(cpu)
+		if (topology_is_primary_thread(cpu))
+			cpumask_set_cpu(cpu, rmpopt_cpumask);
+
+	rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
+	rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
+
+	/*
+	 * Per-CPU RMPOPT tables cover at most 2 TB.  Program each core's
+	 * RMPOPT_BASE with the start of RAM to optimize up to 2 TB.
+	 */
+	for_each_cpu(cpu, rmpopt_cpumask)
+		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
+}
+EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
+
 /*
  * Do the necessary preparations which are verified by the firmware as
  * described in the SNP_INIT_EX firmware command description in the SNP
@@ -705,10 +760,12 @@ bool snp_probe_rmptable_info(void)
 	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
 		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
 
-	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
+	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
 		return probe_segmented_rmptable_info();
-	else
+	} else {
+		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
 		return probe_contiguous_rmptable_info();
+	}
 }
 
 /*
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index ca473ca198b8..c002a7ca26a8 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1477,6 +1477,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 	}
 
 	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
+
+	snp_setup_rmpopt();
+
 	sev->snp_initialized = true;
 	dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
 		data.tio_en ? "enabled" : "disabled");
-- 
2.43.0

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

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

When SEV-SNP is enabled, all writes to memory are checked to ensure
integrity of SNP guest memory. 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 SEV-SNP guest memory.

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

Enable RMPOPT optimizations for up to 2TB of system RAM starting from
the lowest physical memory address aligned down to a 1GB boundary at
RMP initialization time. RMP checks can initially be skipped for 1GB
memory ranges that do not contain SEV-SNP guest memory (excluding
preassigned pages such as the RMP table and firmware pages). As SNP
guests are launched, RMPUPDATE will disable the corresponding RMPOPT
optimizations.

Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/virt/svm/sev.c | 160 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 158 insertions(+), 2 deletions(-)

diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 8bfd80284836..04b19e64f832 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -19,6 +19,7 @@
 #include <linux/iommu.h>
 #include <linux/amd-iommu.h>
 #include <linux/nospec.h>
+#include <linux/workqueue.h>
 
 #include <asm/sev.h>
 #include <asm/processor.h>
@@ -125,7 +126,18 @@ static void *rmp_bookkeeping __ro_after_init;
 static u64 probed_rmp_base, probed_rmp_size;
 
 static cpumask_var_t rmpopt_cpumask;
-static phys_addr_t rmpopt_pa_start;
+static phys_addr_t rmpopt_pa_start, rmpopt_pa_end;
+
+enum rmpopt_function {
+	RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS,
+	RMPOPT_FUNC_REPORT_STATUS
+};
+
+#define RMPOPT_WORK_TIMEOUT	10000
+
+static struct workqueue_struct *rmpopt_wq;
+static struct delayed_work rmpopt_delayed_work;
+static DEFINE_MUTEX(rmpopt_wq_mutex);
 
 static LIST_HEAD(snp_leaked_pages_list);
 static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
@@ -565,11 +577,20 @@ static void snp_cleanup_rmpopt(void)
 {
 	int cpu;
 
+	guard(mutex)(&rmpopt_wq_mutex);
+
+	if (!rmpopt_wq)
+		return;
+
+	cancel_delayed_work_sync(&rmpopt_delayed_work);
+	destroy_workqueue(rmpopt_wq);
+
 	for_each_cpu(cpu, rmpopt_cpumask)
 		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
 
 	free_cpumask_var(rmpopt_cpumask);
-	rmpopt_pa_start = 0;
+	rmpopt_pa_start = rmpopt_pa_end = 0;
+	rmpopt_wq = NULL;
 }
 
 void snp_shutdown(void)
@@ -599,6 +620,96 @@ static bool rmpopt_capable(void)
 	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
 }
 
+/*
+ * RMPOPT: F2 0F 01 FC
+ *   Input:  RAX = system physical address (1GB aligned)
+ *           RCX = operation type
+ *   Output: CF set if the range was optimized
+ */
+static inline bool __rmpopt(u64 pa_start, u64 op_type)
+{
+	bool optimized;
+
+	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
+		     : "=@ccc" (optimized)
+		     : "a" (pa_start), "c" (op_type)
+		     : "memory", "cc");
+
+	return optimized;
+}
+
+static void rmpopt(u64 pa)
+{
+	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
+	u64 op_type = RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS;
+
+	__rmpopt(pa_start, op_type);
+}
+
+/*
+ * 'val' is a system physical address.
+ */
+static void rmpopt_smp(void *val)
+{
+	rmpopt((u64)val);
+}
+
+/*
+ * RMPOPT optimizations skip RMP checks at 1GB granularity if this
+ * range of memory does not contain any SNP guest memory.
+ */
+static void rmpopt_work_handler(struct work_struct *work)
+{
+	cpumask_var_t follower_mask;
+	phys_addr_t pa;
+	int this_cpu;
+
+	pr_info("Attempt RMP optimizations on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
+		rmpopt_pa_start, rmpopt_pa_end);
+
+	if (!alloc_cpumask_var(&follower_mask, GFP_KERNEL)) {
+		pr_warn("RMP optimization pass skipped: cpumask allocation failed\n");
+		return;
+	}
+
+	/*
+	 * RMPOPT scans the RMP table, stores the result of the scan in the
+	 * reserved processor memory. The RMP scan is the most expensive
+	 * part. If a second RMPOPT occurs, it can skip the expensive scan
+	 * if they can see a cached result in the reserved processor memory.
+	 *
+	 * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
+	 * on every other primary thread. Followers are "designed to"
+	 * skip the scan if they see the "cached" scan results.
+	 *
+	 * Pin the worker to the current CPU for the leader loop so that
+	 * this_cpu remains valid and the RMPOPT instruction executes on
+	 * the correct CPU.  Use migrate_disable() rather than get_cpu() to
+	 * prevent migration while still allowing preemption.
+	 */
+	migrate_disable();
+	this_cpu = smp_processor_id();
+
+	cpumask_andnot(follower_mask, rmpopt_cpumask,
+		       topology_sibling_cpumask(this_cpu));
+
+	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
+		rmpopt(pa);
+
+	migrate_enable();
+
+	/*
+	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
+	 * intentionally not held here: CPU hotplug is disabled for the entire
+	 * time SNP is active (see snp_prepare()), and this work only runs while
+	 * SNP is active, so the follower set stays valid across the whole scan.
+	 */
+	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
+		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);
+
+	free_cpumask_var(follower_mask);
+}
+
 void snp_setup_rmpopt(void)
 {
 	u64 rmpopt_base;
@@ -607,8 +718,38 @@ 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 snp_cleanup_rmpopt()
+	 * are skipped, so the workqueue, delayed work, cpumask 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, rmpopt_work_handler);
+
 	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
 		pr_err("Failed to allocate RMPOPT cpumask\n");
+		destroy_workqueue(rmpopt_wq);
+		rmpopt_wq = NULL;
 		return;
 	}
 
@@ -630,6 +771,21 @@ void snp_setup_rmpopt(void)
 	 */
 	for_each_cpu(cpu, rmpopt_cpumask)
 		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);
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
 
-- 
2.43.0

[PATCH v11 5/6] x86/sev: Add interface to re-enable RMP optimizations.

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

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

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 clear 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 clear the RMPOPT
optimizations for the affected 1GB regions.  Conversely, guest pages are
converted back to shared during SNP guest termination, so those regions
may become eligible for RMPOPT optimization again.

Without some intervention, all RMP optimizations would eventually be
lost.  Add an interface to re-optimize all of physical memory.

The interface uses mod_delayed_work() instead of queue_delayed_work()
so that the delay timer is reset on each call. This provides proper
batching semantics: re-optimization runs 10 seconds after the *last*
VM 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.

Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/include/asm/sev.h |  2 ++
 arch/x86/virt/svm/sev.c    | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 6fd72a44a51e..09b1c5d33790 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -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
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 04b19e64f832..291a360291eb 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -710,6 +710,21 @@ static void rmpopt_work_handler(struct work_struct *work)
 	free_cpumask_var(follower_mask);
 }
 
+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;
-- 
2.43.0

[PATCH v11 6/6] KVM: SEV: Perform RMP optimizations on SNP guest shutdown

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

Pages are converted from shared to private as SNP guests are launched.
This destroys exisiting RMPOPT optimizations in the regions where
pages are converted.

Conversely, guest pages are converted back to shared during SNP guest
termination and their region may become eligible for RMPOPT
optimization.

To take advantage of this, perform RMPOPT after guest termination.
Do it after a delay so that a single RMPOPT pass can be done if
multiple guests terminate in a short period of time.

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/kvm/svm/sev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 427229347876..c574849b1587 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2992,6 +2992,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);
 	}
-- 
2.43.0

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] `snp_cleanup_rmpopt()` unconditionally dereferences `rmpopt_cpumask`, causing a deterministic kernel panic on shutdown if the mask was never allocated (e.g., on CPUs without RMPOPT support or if allocation fails).
- [High] If SEV-SNP firmware initialization fails, the error path returns directly without cleaning up the state set by `snp_prepare()`, leaving CPU hotplug permanently disabled.
- [Medium] Re-initializing SEV-SNP after a legacy shutdown leaks the `rmpopt_cpumask` allocation because the old pointer is overwritten without being freed.
--

--- Patch [3]: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs ---
commit e4623d49ab6201adc5b609f5a6043ed6dfb7f55c
Author: Ashish Kalra <ashish.kalra@amd.com>
Subject: x86/sev: Initialize RMPOPT configuration MSRs

The new RMPOPT instruction helps manage per-CPU RMP optimization
structures inside the CPU. This commit initializes the per-CPU RMPOPT
table base to enable RMP optimization for up to 2 TB of system RAM.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index e2f69fba09389..8bfd802848361 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
[ ... ]
> @@ -558,6 +561,17 @@ int snp_prepare(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>  
> +static void snp_cleanup_rmpopt(void)
> +{
> +	int cpu;
> +
> +	for_each_cpu(cpu, rmpopt_cpumask)

[Severity: High]
If rmpopt_cpumask was never allocated, does this macro dereference a NULL
pointer?

In snp_setup_rmpopt(), if rmpopt_capable() is false, the mask remains
unallocated. When snp_shutdown() subsequently calls snp_cleanup_rmpopt(),
it iterates over the mask without checking if it is available.

Can this cause a kernel panic on system shutdown or module unload for
systems without RMPOPT support?

> +		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
> +
> +	free_cpumask_var(rmpopt_cpumask);
> +	rmpopt_pa_start = 0;
> +}
[ ... ]
> @@ -578,6 +593,46 @@ void snp_shutdown(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>  
> +static bool rmpopt_capable(void)
> +{
> +	return cpu_feature_enabled(X86_FEATURE_RMPOPT) &&
> +	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
> +}
> +
> +void snp_setup_rmpopt(void)
> +{
> +	u64 rmpopt_base;
> +	int cpu;
> +
> +	if (!rmpopt_capable())
> +		return;
> +
> +	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {

[Severity: Medium]
Does this unconditional allocation leak the previously allocated cpumask
when SEV-SNP is re-initialized?

During a legacy shutdown, snp_shutdown() returns early when the SnpEn
firmware bit is still set, skipping snp_cleanup_rmpopt():

arch/x86/virt/svm/sev.c:snp_shutdown() {
...
	rdmsrq(MSR_AMD64_SYSCFG, syscfg);
	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
		return;
...
}

A subsequent call to __sev_snp_init_locked() will invoke snp_setup_rmpopt()
again and overwrite the old pointer without freeing it.

> +		pr_err("Failed to allocate RMPOPT cpumask\n");
> +		return;
> +	}
[ ... ]
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index f833cb7e4da36..0b40d23a26b45 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1630,6 +1630,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>  	}
>  
>  	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
> +
> +	snp_setup_rmpopt();
> +

[Severity: High]
If the SEV-SNP firmware initialization fails, does this leave CPU hotplug
permanently disabled?

A global cpu_hotplug_disable() call is made inside snp_prepare() right before
we reach this code. If the subsequent firmware command fails in
__sev_snp_init_locked(), it returns the error directly without cleaning up:

drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
...
	rc = __sev_do_cmd_locked(cmd, arg, error);
	if (rc) {
		dev_err(sev->dev, "SEV-SNP: %s failed rc %d, error %#x\n",
			cmd == SEV_CMD_SNP_INIT_EX ? "SNP_INIT_EX" : "SNP_INIT",
			rc, *error);
		return rc;
	}
...
}

Since sev->snp_initialized remains false on this error path, snp_shutdown()
is never executed on teardown, meaning cpu_hotplug_enable() is never invoked
to release the lock.

>  	sev->snp_initialized = true;
>  	dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
>  		data.tio_en ? "enabled" : "disabled");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784844080.git.ashish.kalra@amd.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Inline assembly for __rmpopt fails to specify EAX/RAX as a clobbered or output register, risking an infinite loop.
- [High] Missing CPU yield in long-running RMP optimization loops leads to soft lockups on non-preemptible kernels.
--

--- Patch [4]: [PATCH v11 4/6] x86/sev: Add support to perform RMP optimizations asynchronously ---
commit 9b1e0ac7d870b3ba02472132010185afc997e02d
Author: Ashish Kalra <ashish.kalra@amd.com>

x86/sev: Add support to perform RMP optimizations asynchronously

This commit adds support for performing RMP optimizations asynchronously using
a dedicated workqueue. It introduces a new RMPOPT instruction to skip RMP checks
for 1GB regions not containing SEV-SNP guest memory. Optimizations are enabled
for up to 2TB of system RAM starting at RMP initialization time.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 8bfd802848361..04b19e64f8329 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c

[ ... ]

> +static inline bool __rmpopt(u64 pa_start, u64 op_type)
> +{
> +	bool optimized;
> +
> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     : "=@ccc" (optimized)
> +		     : "a" (pa_start), "c" (op_type)
> +		     : "memory", "cc");

[Severity: High]
Does this inline assembly need to specify EAX/RAX as a clobbered or output
register?

If the RMPOPT instruction modifies RAX (as AMD SEV-SNP instructions like
RMPUPDATE typically return an execution status code in EAX/RAX), the current
specification of RAX as input-only ("a" (pa_start)) could allow the compiler
to optimize the loop counter into RAX and silently overwrite it.

This could lead to an infinite loop during rmpopt_work_handler() execution.

> +
> +	return optimized;
> +}

[ ... ]

> +static void rmpopt_work_handler(struct work_struct *work)
> +{

[ ... ]

> +	migrate_disable();
> +	this_cpu = smp_processor_id();
> +
> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
> +		       topology_sibling_cpumask(this_cpu));
> +
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		rmpopt(pa);

[Severity: High]
Does this loop need a cond_resched() to yield the CPU?

This loops up to 2048 times doing an expensive hardware scan inside a
migrate_disable() block. On kernels with CONFIG_PREEMPT_NONE or
PREEMPT_VOLUNTARY, preemption is not implicit.

Without explicitly yielding the CPU, this could cause severe scheduling latency
spikes or trigger a soft lockup watchdog panic.

> +
> +	migrate_enable();
> +
> +	/*
> +	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
> +	 * intentionally not held here: CPU hotplug is disabled for the entire
> +	 * time SNP is active (see snp_prepare()), and this work only runs while
> +	 * SNP is active, so the follower set stays valid across the whole scan.
> +	 */
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);

[Severity: High]
Could this secondary follower loop also need a cond_resched() to prevent
CPU monopolization?

This loop iterates up to 2048 times broadcasting IPIs via on_each_cpu_mask().
Similar to the first loop, without a yield, this could lead to soft lockups on
non-preemptible kernels.

> +
> +	free_cpumask_var(follower_mask);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784844080.git.ashish.kalra@amd.com?part=4

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

[sashiko-bot]

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

Pre-existing issues:
- [High] System suspend/resume bypasses `cpu_hotplug_disable()`, causing secondary CPUs to come online without the `SnpEn` bit set.
--

--- Patch [2]: [PATCH v11 2/6] x86/sev: Disable CPU hotplug while SNP is active ---
commit 2ca360619d15fed203ba00f43d20453130503df3
Author: Ashish Kalra <ashish.kalra@amd.com>

x86/sev: Disable CPU hotplug while SNP is active

This commit replaces cpus_read_lock() with cpu_hotplug_disable() in
snp_prepare() to prevent CPU hotplug while SEV-SNP is being enabled. It
additionally keeps hotplug disabled as long as SNP is active to ensure OS CPU
hotplug does not diverge from firmware expectations.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index cff285d8ad8e3..e2f69fba09389 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c

[ ... ]

> @@ -526,14 +525,21 @@ int snp_prepare(void)
>  
>  	clear_rmp();
>  
> -	cpus_read_lock();
> +	/*
> +	 * Disable CPU hotplug before enabling SNP: no CPU may come online
> +	 * without SnpEn while SNP is active, and none may go offline during
> +	 * enable.  This keeps cpu_online_mask stable for the check and the
> +	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
> +	 * re-enabled in snp_shutdown() once the firmware disables SNP.
> +	 */
> +	cpu_hotplug_disable();

[Severity: High]
This is a pre-existing issue, but does cpu_hotplug_disable() reliably
prevent secondary CPUs from coming online without the SnpEn bit set?

System suspend and resume transitions bypass the sysfs hotplug locks. During a
suspend and resume cycle, freeze_secondary_cpus() and thaw_secondary_cpus()
directly invoke internal routines that offline and online CPUs, respectively.

Because snp_enable() is not registered in a CPU hotplug callback, if a
secondary CPU is thawed, it appears it would wake up and come online without
the MSR_AMD64_SYSCFG_SNP_EN bit set. Would this leave the system vulnerable
by allowing arbitrary host access to SEV-SNP guest memory, since no RMP
checks would be performed by those CPUs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784844080.git.ashish.kalra@amd.com?part=2

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

[Kalra, Ashish]

On 7/27/2026 2:37 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] System suspend/resume bypasses `cpu_hotplug_disable()`, causing secondary CPUs to come online without the `SnpEn` bit set.
> --
> 
> --- Patch [2]: [PATCH v11 2/6] x86/sev: Disable CPU hotplug while SNP is active ---
> commit 2ca360619d15fed203ba00f43d20453130503df3
> Author: Ashish Kalra <ashish.kalra@amd.com>
> 
> x86/sev: Disable CPU hotplug while SNP is active
> 
> This commit replaces cpus_read_lock() with cpu_hotplug_disable() in
> snp_prepare() to prevent CPU hotplug while SEV-SNP is being enabled. It
> additionally keeps hotplug disabled as long as SNP is active to ensure OS CPU
> hotplug does not diverge from firmware expectations.
> 
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index cff285d8ad8e3..e2f69fba09389 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
> 
> [ ... ]
> 
>> @@ -526,14 +525,21 @@ int snp_prepare(void)
>>  
>>  	clear_rmp();
>>  
>> -	cpus_read_lock();
>> +	/*
>> +	 * Disable CPU hotplug before enabling SNP: no CPU may come online
>> +	 * without SnpEn while SNP is active, and none may go offline during
>> +	 * enable.  This keeps cpu_online_mask stable for the check and the
>> +	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
>> +	 * re-enabled in snp_shutdown() once the firmware disables SNP.
>> +	 */
>> +	cpu_hotplug_disable();
> 
> [Severity: High]
> This is a pre-existing issue, but does cpu_hotplug_disable() reliably
> prevent secondary CPUs from coming online without the SnpEn bit set?
> 
> System suspend and resume transitions bypass the sysfs hotplug locks. During a
> suspend and resume cycle, freeze_secondary_cpus() and thaw_secondary_cpus()
> directly invoke internal routines that offline and online CPUs, respectively.
> 
> Because snp_enable() is not registered in a CPU hotplug callback, if a
> secondary CPU is thawed, it appears it would wake up and come online without
> the MSR_AMD64_SYSCFG_SNP_EN bit set. Would this leave the system vulnerable
> by allowing arbitrary host access to SEV-SNP guest memory, since no RMP
> checks would be performed by those CPUs?
> 

Pre-existing base SNP-host behaviour, not introduced here (as tagged). An SNP host doesn't support suspend/resume: 
SnpEn is set on all CPUs once at boot, immediately followed by SNP_INIT, and that state can't be rebuilt on resume without a full re-init.
So a CPU coming back online without SnpEn can't occur in a working config.

Thanks,
Ashish

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

[Kalra, Ashish]

On 7/27/2026 2:22 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Inline assembly for __rmpopt fails to specify EAX/RAX as a clobbered or output register, risking an infinite loop.
> - [High] Missing CPU yield in long-running RMP optimization loops leads to soft lockups on non-preemptible kernels.
> --
> 
> --- Patch [4]: [PATCH v11 4/6] x86/sev: Add support to perform RMP optimizations asynchronously ---
> commit 9b1e0ac7d870b3ba02472132010185afc997e02d
> Author: Ashish Kalra <ashish.kalra@amd.com>
> 
> x86/sev: Add support to perform RMP optimizations asynchronously
> 
> This commit adds support for performing RMP optimizations asynchronously using
> a dedicated workqueue. It introduces a new RMPOPT instruction to skip RMP checks
> for 1GB regions not containing SEV-SNP guest memory. Optimizations are enabled
> for up to 2TB of system RAM starting at RMP initialization time.
> 
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index 8bfd802848361..04b19e64f8329 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
> 
> [ ... ]
> 
>> +static inline bool __rmpopt(u64 pa_start, u64 op_type)
>> +{
>> +	bool optimized;
>> +
>> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
>> +		     : "=@ccc" (optimized)
>> +		     : "a" (pa_start), "c" (op_type)
>> +		     : "memory", "cc");
> 
> [Severity: High]
> Does this inline assembly need to specify EAX/RAX as a clobbered or output
> register?
> 
> If the RMPOPT instruction modifies RAX (as AMD SEV-SNP instructions like
> RMPUPDATE typically return an execution status code in EAX/RAX), the current
> specification of RAX as input-only ("a" (pa_start)) could allow the compiler
> to optimize the loop counter into RAX and silently overwrite it.
> 
> This could lead to an infinite loop during rmpopt_work_handler() execution.
> 

RMPOPT's only output is RFLAGS.CF; it does not modify RAX. RAX is not modified and cannot be clobbered and
so the loop counter can't be corrupted. (RMPUPDATE differs: it returns a status in RAX, hence "=a".)

>> +
>> +	return optimized;
>> +}
> 
> [ ... ]
> 
>> +static void rmpopt_work_handler(struct work_struct *work)
>> +{
> 
> [ ... ]
> 
>> +	migrate_disable();
>> +	this_cpu = smp_processor_id();
>> +
>> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
>> +		       topology_sibling_cpumask(this_cpu));
>> +
>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>> +		rmpopt(pa);
> 
> [Severity: High]
> Does this loop need a cond_resched() to yield the CPU?
> 
> This loops up to 2048 times doing an expensive hardware scan inside a
> migrate_disable() block. On kernels with CONFIG_PREEMPT_NONE or
> PREEMPT_VOLUNTARY, preemption is not implicit.
> 
> Without explicitly yielding the CPU, this could cause severe scheduling latency
> spikes or trigger a soft lockup watchdog panic.
> 

Dropped intentionally: since commit 7dadeaa6e851 x86 has no non-preemptible model (PREEMPT_VOLUNTARY/PREEMPT_NONE aren't selectable), 
so cond_resched() is a no-op and the loops are already preemptible — migrate_disable() pins the CPU but doesn't disable preemption.
No soft-lockup risk.

>> +
>> +	migrate_enable();
>> +
>> +	/*
>> +	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
>> +	 * intentionally not held here: CPU hotplug is disabled for the entire
>> +	 * time SNP is active (see snp_prepare()), and this work only runs while
>> +	 * SNP is active, so the follower set stays valid across the whole scan.
>> +	 */
>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>> +		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);
> 
> [Severity: High]
> Could this secondary follower loop also need a cond_resched() to prevent
> CPU monopolization?
> 
> This loop iterates up to 2048 times broadcasting IPIs via on_each_cpu_mask().
> Similar to the first loop, without a yield, this could lead to soft lockups on
> non-preemptible kernels.
> 

Dropped intentionally: since commit 7dadeaa6e851 x86 has no non-preemptible model (PREEMPT_VOLUNTARY/PREEMPT_NONE aren't selectable), 
so cond_resched() is a no-op and the loops are already preemptible — No soft-lockup risk.

Thanks,
Ashish

>> +
>> +	free_cpumask_var(follower_mask);
>> +}
> 

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Kalra, Ashish]

On 7/27/2026 2:22 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] `snp_cleanup_rmpopt()` unconditionally dereferences `rmpopt_cpumask`, causing a deterministic kernel panic on shutdown if the mask was never allocated (e.g., on CPUs without RMPOPT support or if allocation fails).
> - [High] If SEV-SNP firmware initialization fails, the error path returns directly without cleaning up the state set by `snp_prepare()`, leaving CPU hotplug permanently disabled.
> - [Medium] Re-initializing SEV-SNP after a legacy shutdown leaks the `rmpopt_cpumask` allocation because the old pointer is overwritten without being freed.
> --
> 
> --- Patch [3]: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs ---
> commit e4623d49ab6201adc5b609f5a6043ed6dfb7f55c
> Author: Ashish Kalra <ashish.kalra@amd.com>
> Subject: x86/sev: Initialize RMPOPT configuration MSRs
> 
> The new RMPOPT instruction helps manage per-CPU RMP optimization
> structures inside the CPU. This commit initializes the per-CPU RMPOPT
> table base to enable RMP optimization for up to 2 TB of system RAM.
> 
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index e2f69fba09389..8bfd802848361 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
> [ ... ]
>> @@ -558,6 +561,17 @@ int snp_prepare(void)
>>  }
>>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>>  
>> +static void snp_cleanup_rmpopt(void)
>> +{
>> +	int cpu;
>> +
>> +	for_each_cpu(cpu, rmpopt_cpumask)
> 
> [Severity: High]
> If rmpopt_cpumask was never allocated, does this macro dereference a NULL
> pointer?
> 
> In snp_setup_rmpopt(), if rmpopt_capable() is false, the mask remains
> unallocated. When snp_shutdown() subsequently calls snp_cleanup_rmpopt(),
> it iterates over the mask without checking if it is available.
> 
> Can this cause a kernel panic on system shutdown or module unload for
> systems without RMPOPT support?

Can't happen in the full series: patch 4 adds if (!rmpopt_wq) return;, and rmpopt_wq is non-NULL only after snp_setup_rmpopt() succeeds.
On a host without RMPOPT it stays NULL, so rmpopt_cpumask is never dereferenced.

> 
>> +		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
>> +
>> +	free_cpumask_var(rmpopt_cpumask);
>> +	rmpopt_pa_start = 0;
>> +}
> [ ... ]
>> @@ -578,6 +593,46 @@ void snp_shutdown(void)
>>  }
>>  EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>>  
>> +static bool rmpopt_capable(void)
>> +{
>> +	return cpu_feature_enabled(X86_FEATURE_RMPOPT) &&
>> +	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>> +}
>> +
>> +void snp_setup_rmpopt(void)
>> +{
>> +	u64 rmpopt_base;
>> +	int cpu;
>> +
>> +	if (!rmpopt_capable())
>> +		return;
>> +
>> +	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
> 
> [Severity: Medium]
> Does this unconditional allocation leak the previously allocated cpumask
> when SEV-SNP is re-initialized?
> 
> During a legacy shutdown, snp_shutdown() returns early when the SnpEn
> firmware bit is still set, skipping snp_cleanup_rmpopt():
> 
> arch/x86/virt/svm/sev.c:snp_shutdown() {
> ...
> 	rdmsrq(MSR_AMD64_SYSCFG, syscfg);
> 	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
> 		return;
> ...
> }
> 
> A subsequent call to __sev_snp_init_locked() will invoke snp_setup_rmpopt()
> again and overwrite the old pointer without freeing it.

Handled in the full series: patch 4's re-init guard (if (rmpopt_wq) { queue_delayed_work(...); return; }) re-queues the pass instead of re-allocating,
so there's no leak.

> 
>> +		pr_err("Failed to allocate RMPOPT cpumask\n");
>> +		return;
>> +	}
> [ ... ]
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index f833cb7e4da36..0b40d23a26b45 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1630,6 +1630,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>  	}
>>  
>>  	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
>> +
>> +	snp_setup_rmpopt();
>> +
> 
> [Severity: High]
> If the SEV-SNP firmware initialization fails, does this leave CPU hotplug
> permanently disabled?
> 
> A global cpu_hotplug_disable() call is made inside snp_prepare() right before
> we reach this code. If the subsequent firmware command fails in
> __sev_snp_init_locked(), it returns the error directly without cleaning up:
> 
> drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
> ...
> 	rc = __sev_do_cmd_locked(cmd, arg, error);
> 	if (rc) {
> 		dev_err(sev->dev, "SEV-SNP: %s failed rc %d, error %#x\n",
> 			cmd == SEV_CMD_SNP_INIT_EX ? "SNP_INIT_EX" : "SNP_INIT",
> 			rc, *error);
> 		return rc;
> 	}
> ...
> }
> 
> Since sev->snp_initialized remains false on this error path, snp_shutdown()
> is never executed on teardown, meaning cpu_hotplug_enable() is never invoked
> to release the lock.

Intentional. snp_prepare() re-enables hotplug on its own failure (before SnpEn is set); once SnpEn is set, hotplug must stay disabled — 
a CPU coming online without it would break RMP-check consistency and any SNP_INIT retry (which needs SnpEn on all present CPUs). 
Documented in the commit message, and the triggering failure is already logged.

Thanks,
Ashish

> 
>>  	sev->snp_initialized = true;
>>  	dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
>>  		data.tio_en ? "enabled" : "disabled");
> 

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

[Borislav Petkov]

On Mon, Jul 27, 2026 at 07:04:31PM +0000, Ashish Kalra wrote:
> 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.
> The RMPOPT setup and cleanup added later are introduced after this patch
> and never take the lock for the same reason.

Why do I even bother writing it?

"do ... not talk about future patches because git history is not always
linear"

> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/virt/svm/sev.c | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index cff285d8ad8e..e2f69fba0938 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -513,7 +513,6 @@ static void clear_hsave_pa(void *arg)
>  
>  int snp_prepare(void)
>  {
> -	int ret;
>  	u64 val;
>  
>  	/*
> @@ -526,14 +525,21 @@ int snp_prepare(void)
>  
>  	clear_rmp();
>  
> -	cpus_read_lock();
> +	/*
> +	 * Disable CPU hotplug before enabling SNP: no CPU may come online
> +	 * without SnpEn while SNP is active, and none may go offline during
> +	 * enable.  This keeps cpu_online_mask stable for the check and the
> +	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
> +	 * re-enabled in snp_shutdown() once the firmware disables SNP.
> +	 */
> +	cpu_hotplug_disable();

No need for too much splainin' and besides, that comment'll grow out-of-whack
sooner than you think:

diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index e2f69fba0938..731ea25fba37 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -528,9 +528,7 @@ int snp_prepare(void)
        /*
         * Disable CPU hotplug before enabling SNP: no CPU may come online
         * without SnpEn while SNP is active, and none may go offline during
-        * enable.  This keeps cpu_online_mask stable for the check and the
-        * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
-        * re-enabled in snp_shutdown() once the firmware disables SNP.
+        * enable.
         */
        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 +554,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");
>  
> @@ -565,6 +566,13 @@ void snp_shutdown(void)
>  	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
>  		return;
>  
> +	/*
> +	 * 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();

What happens if CPUs get offlined here after hotplug has been enabled and...

>  	clear_rmp();
>  	on_each_cpu(mfd_reconfigure, NULL, 1);

... they miss the mfd_reconfigure()?

>  }

-- 
Regards/Gruss,
    Boris.

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

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

[Kalra, Ashish]

On 7/28/2026 9:15 PM, Borislav Petkov wrote:
> On Mon, Jul 27, 2026 at 07:04:31PM +0000, Ashish Kalra wrote:
>> 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.
>> The RMPOPT setup and cleanup added later are introduced after this patch
>> and never take the lock for the same reason.
> 
> Why do I even bother writing it?
> 
> "do ... not talk about future patches because git history is not always
> linear"

Will drop the future patch reference from the commit log.

> 
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>  arch/x86/virt/svm/sev.c | 39 +++++++++++++++++++++++++++++----------
>>  1 file changed, 29 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index cff285d8ad8e..e2f69fba0938 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -513,7 +513,6 @@ static void clear_hsave_pa(void *arg)
>>  
>>  int snp_prepare(void)
>>  {
>> -	int ret;
>>  	u64 val;
>>  
>>  	/*
>> @@ -526,14 +525,21 @@ int snp_prepare(void)
>>  
>>  	clear_rmp();
>>  
>> -	cpus_read_lock();
>> +	/*
>> +	 * Disable CPU hotplug before enabling SNP: no CPU may come online
>> +	 * without SnpEn while SNP is active, and none may go offline during
>> +	 * enable.  This keeps cpu_online_mask stable for the check and the
>> +	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
>> +	 * re-enabled in snp_shutdown() once the firmware disables SNP.
>> +	 */
>> +	cpu_hotplug_disable();
> 
> No need for too much splainin' and besides, that comment'll grow out-of-whack
> sooner than you think:
> 

Will trim the comment.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index e2f69fba0938..731ea25fba37 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -528,9 +528,7 @@ int snp_prepare(void)
>         /*
>          * Disable CPU hotplug before enabling SNP: no CPU may come online
>          * without SnpEn while SNP is active, and none may go offline during
> -        * enable.  This keeps cpu_online_mask stable for the check and the
> -        * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
> -        * re-enabled in snp_shutdown() once the firmware disables SNP.
> +        * enable.
>          */
>         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 +554,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");
>>  
>> @@ -565,6 +566,13 @@ void snp_shutdown(void)
>>  	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
>>  		return;
>>  
>> +	/*
>> +	 * 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();
> 
> What happens if CPUs get offlined here after hotplug has been enabled and...
> 
>>  	clear_rmp();
>>  	on_each_cpu(mfd_reconfigure, NULL, 1);
> 
> ... they miss the mfd_reconfigure()?
> 
>>  }
> 

Yes, re-enable should happens last in snp_shutdown() (after clear_rmp()/mfd_reconfigure()).

Thanks,
Ashish

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Borislav Petkov]

On Mon, Jul 27, 2026 at 07:04:57PM +0000, Ashish Kalra wrote:
> @@ -705,10 +760,12 @@ bool snp_probe_rmptable_info(void)
>  	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
>  		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
>  
> -	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
> +	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
>  		return probe_segmented_rmptable_info();
> -	else
> +	} else {
> +		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);

That looks a bit squirreled away in a probing function. Let's make that a bit
more obvious:

bool snp_probe_rmptable_info(void)
{
        if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP)) {
                rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);

                if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
                        return probe_segmented_rmptable_info();
        } else {
                /* RMPOPT is supported only with a segmented RMP table. */
                setup_clear_cpu_cap(X86_FEATURE_RMPOPT);

                return probe_contiguous_rmptable_info();
        }       
}

> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index ca473ca198b8..c002a7ca26a8 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1477,6 +1477,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>  	}
>  
>  	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
> +
> +	snp_setup_rmpopt();

Why is this thing here and not at the end?

SEV-TIO is supposed to always run last or it doesn't matter?

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[K Prateek Nayak]

Hello Boris,

On 7/30/2026 7:37 AM, Borislav Petkov wrote:
> On Mon, Jul 27, 2026 at 07:04:57PM +0000, Ashish Kalra wrote:
>> @@ -705,10 +760,12 @@ bool snp_probe_rmptable_info(void)
>>  	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
>>  		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
>>  
>> -	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
>> +	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
>>  		return probe_segmented_rmptable_info();
>> -	else
>> +	} else {
>> +		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
> 
> That looks a bit squirreled away in a probing function. Let's make that a bit
> more obvious:
> 
> bool snp_probe_rmptable_info(void)
> {
>         if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP)) {
>                 rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
> 
>                 if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
>                         return probe_segmented_rmptable_info();
>         } else {

I believe there can be cases where CPUID advertises
X86_FEATURE_SEGMENTED_RMP but rmf_cfg doesn't set
MSR_AMD64_SEG_RMP_ENABLED because otherwise checking rmp_cfg in
addition to the feature bit makes no sense.

In that case we should attempt a probe_contiguous_rmptable_info() even
for X86_FEATURE_SEGMENTED_RMP.

Essentially drop that "else" and do something like:

bool snp_probe_rmptable_info(void)
{
	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP)) {
		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);

		if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
			return probe_segmented_rmptable_info();
	}

	/*
	 * Segmented RMP is either not supported on the platform or
	 * is disabled by the firmware. RMPOPT is not not supported
	 * without segmented RMP.
	 */
	setup_clear_cpu_cap(X86_FEATURE_RMPOPT);

	return probe_contiguous_rmptable_info();
}

>                 /* RMPOPT is supported only with a segmented RMP table. */
>                 setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
> 
>                 return probe_contiguous_rmptable_info();
>         }       
> }


-- 
Thanks and Regards,
Prateek

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Borislav Petkov]

On Thu, Jul 30, 2026 at 08:25:37AM +0530, K Prateek Nayak wrote:
> I believe there can be cases where CPUID advertises
> X86_FEATURE_SEGMENTED_RMP but rmf_cfg doesn't set
> MSR_AMD64_SEG_RMP_ENABLED because otherwise checking rmp_cfg in
> addition to the feature bit makes no sense.
> 
> In that case we should attempt a probe_contiguous_rmptable_info() even
> for X86_FEATURE_SEGMENTED_RMP.
> 
> Essentially drop that "else" and do something like:
> 
> bool snp_probe_rmptable_info(void)
> {
> 	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP)) {
> 		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
> 
> 		if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
> 			return probe_segmented_rmptable_info();
> 	}
> 
> 	/*
> 	 * Segmented RMP is either not supported on the platform or
> 	 * is disabled by the firmware. RMPOPT is not not supported
> 	 * without segmented RMP.
> 	 */
> 	setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
> 
> 	return probe_contiguous_rmptable_info();
> }

That makes sense.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Kalra, Ashish]

On 7/29/2026 10:39 PM, Borislav Petkov wrote:
> On Thu, Jul 30, 2026 at 08:25:37AM +0530, K Prateek Nayak wrote:
>> I believe there can be cases where CPUID advertises
>> X86_FEATURE_SEGMENTED_RMP but rmf_cfg doesn't set
>> MSR_AMD64_SEG_RMP_ENABLED because otherwise checking rmp_cfg in
>> addition to the feature bit makes no sense.
>>
>> In that case we should attempt a probe_contiguous_rmptable_info() even
>> for X86_FEATURE_SEGMENTED_RMP.
>>
>> Essentially drop that "else" and do something like:
>>
>> bool snp_probe_rmptable_info(void)
>> {
>> 	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP)) {
>> 		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
>>
>> 		if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
>> 			return probe_segmented_rmptable_info();
>> 	}
>>
>> 	/*
>> 	 * Segmented RMP is either not supported on the platform or
>> 	 * is disabled by the firmware. RMPOPT is not not supported
>> 	 * without segmented RMP.
>> 	 */
>> 	setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
>>
>> 	return probe_contiguous_rmptable_info();
>> }
> 
> That makes sense.
> 

Ack.

Thanks,
Ashish

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Kalra, Ashish]

Hello Boris,

On 7/29/2026 9:07 PM, Borislav Petkov wrote:
> 
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index ca473ca198b8..c002a7ca26a8 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1477,6 +1477,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>  	}
>>  
>>  	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
>> +
>> +	snp_setup_rmpopt();
> 
> Why is this thing here and not at the end?
> 
> SEV-TIO is supposed to always run last or it doesn't matter?
> 


No hard dependency — it only needs SNP initialized (post SNP_INIT_EX/DF_FLUSH), and the initial pass is async anyway, so it's independent
of the SEV-TIO block and the panic notifier. So i can move it to the end (after the TIO block so it runs last without touching TIO ordering),
but is there a specific reason you want me to move it to the end, as it can just stay here after SNP_INIT.

Thanks,
Ashish

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Borislav Petkov]

On Thu, Jul 30, 2026 at 03:00:55PM -0500, Kalra, Ashish wrote:
> No hard dependency — it only needs SNP initialized (post
> SNP_INIT_EX/DF_FLUSH), and the initial pass is async anyway, so it's
> independent of the SEV-TIO block and the panic notifier. So i can move it to
> the end (after the TIO block so it runs last without touching TIO ordering),
> but is there a specific reason you want me to move it to the end, as it can
> just stay here after SNP_INIT.

The usual reason is, you add new code at the end. I.e., you append it. That is
usually easier for backporting too.

When you put it *NOT* at the end, then the first question I have is, why is
this thing there? Is there a particular reason for it to be there? Is ordering
important all of a sudden?

-- 
Regards/Gruss,
    Boris.

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

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

[Borislav Petkov]

On Mon, Jul 27, 2026 at 07:05:29PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> When SEV-SNP is enabled, all writes to memory are checked to ensure
> integrity of SNP guest memory. This imposes performance overhead on the

s/SNP//

The checks are done not only on SNP guest memory but on *all* memory, as your
next paragraph suggests.

> whole system.
> 
> RMPOPT is a new instruction that minimizes the performance overhead of
> RMP checks on the hypervisor and on non-SNP guests by allowing RMP
> checks to be skipped for 1GB regions of memory that are known not to
> contain any SEV-SNP guest memory.

Let's tone down the abbreviations. "SNP guest memory" is enough and let's
stick to that.

> Add support for performing RMP optimizations asynchronously using a
> dedicated workqueue.
> 
> Enable RMPOPT optimizations for up to 2TB of system RAM starting from
> the lowest physical memory address aligned down to a 1GB boundary at
> RMP initialization time. RMP checks can initially be skipped for 1GB

Why "initially"? What are you trying to say here?

> memory ranges that do not contain SEV-SNP guest memory (excluding
> preassigned pages such as the RMP table and firmware pages). As SNP
> guests are launched, RMPUPDATE will disable the corresponding RMPOPT
> optimizations.

Because it will add pages to the RMP table?

This paragraph needs clarification.

> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Reviewed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/virt/svm/sev.c | 160 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 158 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 8bfd80284836..04b19e64f832 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -19,6 +19,7 @@
>  #include <linux/iommu.h>
>  #include <linux/amd-iommu.h>
>  #include <linux/nospec.h>
> +#include <linux/workqueue.h>
>  
>  #include <asm/sev.h>
>  #include <asm/processor.h>
> @@ -125,7 +126,18 @@ static void *rmp_bookkeeping __ro_after_init;
>  static u64 probed_rmp_base, probed_rmp_size;
>  
>  static cpumask_var_t rmpopt_cpumask;
> -static phys_addr_t rmpopt_pa_start;
> +static phys_addr_t rmpopt_pa_start, rmpopt_pa_end;
> +
> +enum rmpopt_function {

	rmpopt_op_type

> +	RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS,
> +	RMPOPT_FUNC_REPORT_STATUS

RMPOPT_OP_VERIFY...

> +};
> +

/*
 * This timeout was selected this way because...
 */

> +#define RMPOPT_WORK_TIMEOUT	10000
> +
> +static struct workqueue_struct *rmpopt_wq;
> +static struct delayed_work rmpopt_delayed_work;
> +static DEFINE_MUTEX(rmpopt_wq_mutex);
>  
>  static LIST_HEAD(snp_leaked_pages_list);
>  static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
> @@ -565,11 +577,20 @@ static void snp_cleanup_rmpopt(void)
>  {
>  	int cpu;
>  
> +	guard(mutex)(&rmpopt_wq_mutex);
> +
> +	if (!rmpopt_wq)
> +		return;

If there's no workqueue, you skip all the rest, including undoing things which
are not workqueue-related?

That workqueue pointer must be magical and special. Yet, I don't see anything
explaining that.

> +
> +	cancel_delayed_work_sync(&rmpopt_delayed_work);
> +	destroy_workqueue(rmpopt_wq);
> +
>  	for_each_cpu(cpu, rmpopt_cpumask)
>  		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
>  
>  	free_cpumask_var(rmpopt_cpumask);
> -	rmpopt_pa_start = 0;
> +	rmpopt_pa_start = rmpopt_pa_end = 0;
> +	rmpopt_wq = NULL;
>  }
>  
>  void snp_shutdown(void)
> @@ -599,6 +620,96 @@ static bool rmpopt_capable(void)
>  	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>  }
>  
> +/*
> + * RMPOPT: F2 0F 01 FC
> + *   Input:  RAX = system physical address (1GB aligned)
> + *           RCX = operation type
> + *   Output: CF set if the range was optimized
> + */
> +static inline bool __rmpopt(u64 pa_start, u64 op_type)
> +{
> +	bool optimized;
> +

	/*
	 * needs a comment here which says which binutils version
	 * supports the RMPOPT mnemonic.
	 */
> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     : "=@ccc" (optimized)
> +		     : "a" (pa_start), "c" (op_type)
> +		     : "memory", "cc");
> +
> +	return optimized;
> +}
> +
> +static void rmpopt(u64 pa)
> +{
> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
> +	u64 op_type = RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS;

	enum rmpopt_op_type op = ...

> +
> +	__rmpopt(pa_start, op_type);

Looks like the __rmpopt() carve out is not really necessary and you can merge
it back into rmpopt().

> +}
> +
> +/*
> + * 'val' is a system physical address.
> + */
> +static void rmpopt_smp(void *val)

You don't need that one - you can use rmpopt(). But keep on reading...

> +{
> +	rmpopt((u64)val);
> +}
> +
> +/*
> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this
> + * range of memory does not contain any SNP guest memory.
> + */

Put that comment above rmpopt().

> +static void rmpopt_work_handler(struct work_struct *work)
> +{
> +	cpumask_var_t follower_mask;
> +	phys_addr_t pa;

So either phys_addr_t or u64 but not both for a pa.

> +	int this_cpu;
> +
> +	pr_info("Attempt RMP optimizations on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
> +		rmpopt_pa_start, rmpopt_pa_end);

This is going to spam dmesg every time the workqueue runs?

Nope, zap it.

> +	if (!alloc_cpumask_var(&follower_mask, GFP_KERNEL)) {
> +		pr_warn("RMP optimization pass skipped: cpumask allocation failed\n");
> +		return;
> +	}

Why? Why isn't the follower mask allocated once at init time?

> +
> +	/*
> +	 * RMPOPT scans the RMP table, stores the result of the scan in the
> +	 * reserved processor memory. The RMP scan is the most expensive
> +	 * part. If a second RMPOPT occurs, it can skip the expensive scan
> +	 * if they can see a cached result in the reserved processor memory.
> +	 *
> +	 * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
> +	 * on every other primary thread. Followers are "designed to"
> +	 * skip the scan if they see the "cached" scan results.
> +	 *
> +	 * Pin the worker to the current CPU for the leader loop so that

Isn't worker == leader here?

> +	 * this_cpu remains valid and the RMPOPT instruction executes on
> +	 * the correct CPU. 

> Use migrate_disable() rather than get_cpu() to
> +	 * prevent migration while still allowing preemption.

No need to explain that.

> +	 */
> +	migrate_disable();
> +	this_cpu = smp_processor_id();
> +
> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
> +		       topology_sibling_cpumask(this_cpu));
> +
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		rmpopt(pa);
> +
> +	migrate_enable();
> +
> +	/*
> +	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
> +	 * intentionally not held here: CPU hotplug is disabled for the entire
> +	 * time SNP is active (see snp_prepare()), and this work only runs while
> +	 * SNP is active, so the follower set stays valid across the whole scan.
> +	 */
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);

An IPI per 1G pa?!?!? On each CPU?!

Instead of IPIing each CPU and inside the handler, doing the loop?

Nope.

> +
> +	free_cpumask_var(follower_mask);
> +}

Ok, enough for this part. Part II coming up later.

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Kalra, Ashish]

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

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

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

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

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

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

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

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

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

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

I will add a comment spelling that out.

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

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

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

Ok.

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

Ok.

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

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

>> +
>> +	/*
>> +	 * RMPOPT scans the RMP table, stores the result of the scan in the
>> +	 * reserved processor memory. The RMP scan is the most expensive
>> +	 * part. If a second RMPOPT occurs, it can skip the expensive scan
>> +	 * if they can see a cached result in the reserved processor memory.
>> +	 *
>> +	 * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
>> +	 * on every other primary thread. Followers are "designed to"
>> +	 * skip the scan if they see the "cached" scan results.
>> +	 *
>> +	 * Pin the worker to the current CPU for the leader loop so that
> 
> Isn't worker == leader here?

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

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

Ok.

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

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

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

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

Thanks,
Ashish

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

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

[Tom Lendacky]

On 7/27/26 14:04, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> While SNP is active, every memory write is checked against the RMP to
> protect SEV-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.
> The RMPOPT setup and cleanup added later are introduced after this patch
> and never take the lock for the same reason.
> 
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/virt/svm/sev.c | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index cff285d8ad8e..e2f69fba0938 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -513,7 +513,6 @@ static void clear_hsave_pa(void *arg)
>  
>  int snp_prepare(void)
>  {
> -	int ret;
>  	u64 val;
>  
>  	/*
> @@ -526,14 +525,21 @@ int snp_prepare(void)
>  
>  	clear_rmp();
>  
> -	cpus_read_lock();
> +	/*
> +	 * Disable CPU hotplug before enabling SNP: no CPU may come online
> +	 * without SnpEn while SNP is active, and none may go offline during
> +	 * enable.  This keeps cpu_online_mask stable for the check and the
> +	 * on_each_cpu() calls below, so cpus_read_lock() is not needed.  It is
> +	 * re-enabled in snp_shutdown() once the firmware disables SNP.
> +	 */
> +	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 +554,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");
>  
> @@ -565,6 +566,13 @@ void snp_shutdown(void)
>  	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
>  		return;
>  
> +	/*
> +	 * 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();
> +
>  	clear_rmp();
>  	on_each_cpu(mfd_reconfigure, NULL, 1);
>  }
> @@ -577,6 +585,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 +596,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();
> +

Why not just put the cpu_hotplug_disable() at the start of snp_prepare()
then? Wouldn't that take care of both situations and only end up with a
single disable point?

Thanks,
Tom

>  	/*
>  	 * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
>  	 * notifier is invoked to do SNP IOMMU shutdown before kdump.

Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Tom Lendacky]

On 7/27/26 14:04, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> 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.
> 
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
> Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/include/asm/msr-index.h |  3 ++
>  arch/x86/include/asm/sev.h       |  2 +
>  arch/x86/virt/svm/sev.c          | 67 +++++++++++++++++++++++++++++---
>  drivers/crypto/ccp/sev-dev.c     |  3 ++
>  4 files changed, 70 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 18c4be75e927..d2cb0a7cd0a2 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -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
>  
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 594cfa19cbd4..6fd72a44a51e 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -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
>  
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index e2f69fba0938..8bfd80284836 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -124,6 +124,9 @@ static void *rmp_bookkeeping __ro_after_init;
>  
>  static u64 probed_rmp_base, probed_rmp_size;
>  
> +static cpumask_var_t rmpopt_cpumask;
> +static phys_addr_t rmpopt_pa_start;
> +
>  static LIST_HEAD(snp_leaked_pages_list);
>  static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
>  
> @@ -558,6 +561,17 @@ int snp_prepare(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>  
> +static void snp_cleanup_rmpopt(void)
> +{
> +	int cpu;
> +
> +	for_each_cpu(cpu, rmpopt_cpumask)
> +		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
> +
> +	free_cpumask_var(rmpopt_cpumask);
> +	rmpopt_pa_start = 0;
> +}
> +
>  void snp_shutdown(void)
>  {
>  	u64 syscfg;
> @@ -567,10 +581,11 @@ void snp_shutdown(void)
>  		return;
>  
>  	/*
> -	 * 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.
> +	 * Clear the RMPOPT_BASE MSRs while CPU hotplug is still disabled, then
> +	 * re-enable hotplug now that the firmware has disabled SNP.  A legacy SNP
> +	 * shutdown returns above with SnpEn still set and leaves hotplug disabled.
>  	 */
> +	snp_cleanup_rmpopt();

Why not leave everything unchanged comment wise and just add the
snp_cleanup_rmpopt() before the comment with a simple comment above the
call to snp_cleanup_rmpopt(), e.g.:

	/* Disable RMPOPT while CPU hotplug is disabled */
	snp_cleanup_rmpopt();

	/*
	 * Original comment block
	 */
	cpu_hotplug_enable();

Thanks,
Tom

>  	cpu_hotplug_enable();
>  
>  	clear_rmp();
> @@ -578,6 +593,46 @@ void snp_shutdown(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>  
> +static bool rmpopt_capable(void)
> +{
> +	return cpu_feature_enabled(X86_FEATURE_RMPOPT) &&
> +	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
> +}
> +
> +void snp_setup_rmpopt(void)
> +{
> +	u64 rmpopt_base;
> +	int cpu;
> +
> +	if (!rmpopt_capable())
> +		return;
> +
> +	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
> +		pr_err("Failed to allocate RMPOPT cpumask\n");
> +		return;
> +	}
> +
> +	/*
> +	 * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
> +	 * to set up the RMPOPT_BASE MSR.  All primary threads are online,
> +	 * otherwise SNP would not have been enabled.
> +	 */
> +	for_each_online_cpu(cpu)
> +		if (topology_is_primary_thread(cpu))
> +			cpumask_set_cpu(cpu, rmpopt_cpumask);
> +
> +	rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
> +	rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
> +
> +	/*
> +	 * Per-CPU RMPOPT tables cover at most 2 TB.  Program each core's
> +	 * RMPOPT_BASE with the start of RAM to optimize up to 2 TB.
> +	 */
> +	for_each_cpu(cpu, rmpopt_cpumask)
> +		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> +}
> +EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
> +
>  /*
>   * Do the necessary preparations which are verified by the firmware as
>   * described in the SNP_INIT_EX firmware command description in the SNP
> @@ -705,10 +760,12 @@ bool snp_probe_rmptable_info(void)
>  	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
>  		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
>  
> -	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
> +	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
>  		return probe_segmented_rmptable_info();
> -	else
> +	} else {
> +		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
>  		return probe_contiguous_rmptable_info();
> +	}
>  }
>  
>  /*
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index ca473ca198b8..c002a7ca26a8 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1477,6 +1477,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>  	}
>  
>  	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
> +
> +	snp_setup_rmpopt();
> +
>  	sev->snp_initialized = true;
>  	dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
>  		data.tio_en ? "enabled" : "disabled");

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

[Tom Lendacky]

On 7/27/26 14:05, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> When SEV-SNP is enabled, all writes to memory are checked to ensure
> integrity of SNP guest memory. 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 SEV-SNP guest memory.
> 
> Add support for performing RMP optimizations asynchronously using a
> dedicated workqueue.
> 
> Enable RMPOPT optimizations for up to 2TB of system RAM starting from
> the lowest physical memory address aligned down to a 1GB boundary at
> RMP initialization time. RMP checks can initially be skipped for 1GB
> memory ranges that do not contain SEV-SNP guest memory (excluding
> preassigned pages such as the RMP table and firmware pages). As SNP
> guests are launched, RMPUPDATE will disable the corresponding RMPOPT
> optimizations.
> 
> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Reviewed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/virt/svm/sev.c | 160 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 158 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 8bfd80284836..04b19e64f832 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -19,6 +19,7 @@
>  #include <linux/iommu.h>
>  #include <linux/amd-iommu.h>
>  #include <linux/nospec.h>
> +#include <linux/workqueue.h>
>  
>  #include <asm/sev.h>
>  #include <asm/processor.h>
> @@ -125,7 +126,18 @@ static void *rmp_bookkeeping __ro_after_init;
>  static u64 probed_rmp_base, probed_rmp_size;
>  
>  static cpumask_var_t rmpopt_cpumask;
> -static phys_addr_t rmpopt_pa_start;
> +static phys_addr_t rmpopt_pa_start, rmpopt_pa_end;
> +
> +enum rmpopt_function {
> +	RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS,
> +	RMPOPT_FUNC_REPORT_STATUS
> +};
> +
> +#define RMPOPT_WORK_TIMEOUT	10000

Doing

#define RMPOPT_WORK_TIMEOUT	10 * MSEC_PER_SEC

makes it clear that this is a 10 second timeout or put a comment above it
that this is 10 seconds in milliseconds.

Not a biggee, but if you have to re-spin...

Thanks,
Tom

> +
> +static struct workqueue_struct *rmpopt_wq;
> +static struct delayed_work rmpopt_delayed_work;
> +static DEFINE_MUTEX(rmpopt_wq_mutex);
>  
>  static LIST_HEAD(snp_leaked_pages_list);
>  static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
> @@ -565,11 +577,20 @@ static void snp_cleanup_rmpopt(void)
>  {
>  	int cpu;
>  
> +	guard(mutex)(&rmpopt_wq_mutex);
> +
> +	if (!rmpopt_wq)
> +		return;
> +
> +	cancel_delayed_work_sync(&rmpopt_delayed_work);
> +	destroy_workqueue(rmpopt_wq);
> +
>  	for_each_cpu(cpu, rmpopt_cpumask)
>  		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
>  
>  	free_cpumask_var(rmpopt_cpumask);
> -	rmpopt_pa_start = 0;
> +	rmpopt_pa_start = rmpopt_pa_end = 0;
> +	rmpopt_wq = NULL;
>  }
>  
>  void snp_shutdown(void)
> @@ -599,6 +620,96 @@ static bool rmpopt_capable(void)
>  	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>  }
>  
> +/*
> + * RMPOPT: F2 0F 01 FC
> + *   Input:  RAX = system physical address (1GB aligned)
> + *           RCX = operation type
> + *   Output: CF set if the range was optimized
> + */
> +static inline bool __rmpopt(u64 pa_start, u64 op_type)
> +{
> +	bool optimized;
> +
> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     : "=@ccc" (optimized)
> +		     : "a" (pa_start), "c" (op_type)
> +		     : "memory", "cc");
> +
> +	return optimized;
> +}
> +
> +static void rmpopt(u64 pa)
> +{
> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
> +	u64 op_type = RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS;
> +
> +	__rmpopt(pa_start, op_type);
> +}
> +
> +/*
> + * 'val' is a system physical address.
> + */
> +static void rmpopt_smp(void *val)
> +{
> +	rmpopt((u64)val);
> +}
> +
> +/*
> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this
> + * range of memory does not contain any SNP guest memory.
> + */
> +static void rmpopt_work_handler(struct work_struct *work)
> +{
> +	cpumask_var_t follower_mask;
> +	phys_addr_t pa;
> +	int this_cpu;
> +
> +	pr_info("Attempt RMP optimizations on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
> +		rmpopt_pa_start, rmpopt_pa_end);
> +
> +	if (!alloc_cpumask_var(&follower_mask, GFP_KERNEL)) {
> +		pr_warn("RMP optimization pass skipped: cpumask allocation failed\n");
> +		return;
> +	}
> +
> +	/*
> +	 * RMPOPT scans the RMP table, stores the result of the scan in the
> +	 * reserved processor memory. The RMP scan is the most expensive
> +	 * part. If a second RMPOPT occurs, it can skip the expensive scan
> +	 * if they can see a cached result in the reserved processor memory.
> +	 *
> +	 * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
> +	 * on every other primary thread. Followers are "designed to"
> +	 * skip the scan if they see the "cached" scan results.
> +	 *
> +	 * Pin the worker to the current CPU for the leader loop so that
> +	 * this_cpu remains valid and the RMPOPT instruction executes on
> +	 * the correct CPU.  Use migrate_disable() rather than get_cpu() to
> +	 * prevent migration while still allowing preemption.
> +	 */
> +	migrate_disable();
> +	this_cpu = smp_processor_id();
> +
> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
> +		       topology_sibling_cpumask(this_cpu));
> +
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		rmpopt(pa);
> +
> +	migrate_enable();
> +
> +	/*
> +	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
> +	 * intentionally not held here: CPU hotplug is disabled for the entire
> +	 * time SNP is active (see snp_prepare()), and this work only runs while
> +	 * SNP is active, so the follower set stays valid across the whole scan.
> +	 */
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);
> +
> +	free_cpumask_var(follower_mask);
> +}
> +
>  void snp_setup_rmpopt(void)
>  {
>  	u64 rmpopt_base;
> @@ -607,8 +718,38 @@ 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 snp_cleanup_rmpopt()
> +	 * are skipped, so the workqueue, delayed work, cpumask 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, rmpopt_work_handler);
> +
>  	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
>  		pr_err("Failed to allocate RMPOPT cpumask\n");
> +		destroy_workqueue(rmpopt_wq);
> +		rmpopt_wq = NULL;
>  		return;
>  	}
>  
> @@ -630,6 +771,21 @@ void snp_setup_rmpopt(void)
>  	 */
>  	for_each_cpu(cpu, rmpopt_cpumask)
>  		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);
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
>  

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

[Kalra, Ashish]

Hello Tom,

On 7/31/2026 2:35 PM, Tom Lendacky wrote:
>>  int __init snp_rmptable_init(void)
>>  {
>> +	u64 val;
>> +
>>  	if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
>>  		return -ENOSYS;
>>  
>> @@ -586,6 +596,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();
>> +
> Why not just put the cpu_hotplug_disable() at the start of snp_prepare()
> then? Wouldn't that take care of both situations and only end up with a
> single disable point?

The difference is SnpEn: on normal boot it isn't set until snp_prepare(), so hotplug during that IOMMU init -> ccp load window 
is harmless and the online == present check refuses to enable SNP if the CPU set is incomplete — detect-and-refuse at the enable moment.

On kexec SnpEn could already be set from the previous kernel, so that same window is now dangerous (there'd be a window from 
IOMMU-init -> ccp-load where CPU could be offlined/onlined while SnpEn is set) and additionally there's no enable moment to gate — 
snp_prepare() bails before the check because SNP is already on. So we can't detect-and-refuse and so we have to prevent the window
by disabling hotplug early, at the IOMMU-init point (snp_rmptable_init()).

Thanks,
Ashish

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

[Kalra, Ashish]

Hello Boris,

On 7/31/2026 7:37 AM, Kalra, Ashish wrote:
> 
> On 7/31/2026 12:44 AM, Borislav Petkov wrote:

>>> +	migrate_disable();
>>> +	this_cpu = smp_processor_id();
>>> +
>>> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
>>> +		       topology_sibling_cpumask(this_cpu));
>>> +
>>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>>> +		rmpopt(pa);
>>> +
>>> +	migrate_enable();
>>> +
>>> +	/*
>>> +	 * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
>>> +	 * intentionally not held here: CPU hotplug is disabled for the entire
>>> +	 * time SNP is active (see snp_prepare()), and this work only runs while
>>> +	 * SNP is active, so the follower set stays valid across the whole scan.
>>> +	 */
>>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>>> +		on_each_cpu_mask(follower_mask, rmpopt_smp, (void *)pa, true);
>>
>> An IPI per 1G pa?!?!? On each CPU?!
>>
>> Instead of IPIing each CPU and inside the handler, doing the loop?
>>
>> Nope.
>>
> 
> You're right — an IPI per 1 GB is far too many. Will restructure to a single IPI per follower core: a new on_each_cpu()
> callback can loop over the whole range on the CPU it runs on. The leader will call it directly (migrate-disabled) to populate
> the RMP scan cache, then one on_each_cpu_mask() will run it on the remaining cores.
> 
> This will also fold nicely with the earlier cleanup:  __rmpopt() getting merged into rmpopt().
> 
> One important tradeoff to be aware of: each follower IPI handler will now run a 2048-iteration loop with IRQs disabled — 
> but followers are RMP-scan cache hits (the leader populated the cache), so each rmpopt() there is cheap, and this only runs
> at setup and guest-teardown re-optimization time.
> 

Following up again on the tradeoff above.

Referencing your point about not IPIing per 1 GB, two ways to run the follower scan (followers = primary threads only, one
per core, so ~half the logical CPUs — not every CPU):
  
 A) Loop on the worker, IPI per 1 GB: loop is preemptible / IRQs on, only a single op per IPI runs IRQs-off — but 2048
 IPIs per follower.
 
 B) One IPI per core, loop in the handler: a single IPI per follower, but the whole ~2048-iteration loop runs
 non-preemptible with IRQs disabled.
 
Followers are RMP-scan cache hits (cheap) and this only runs at setup/teardown, so I'm leaning toward (B) — but it does
mean a bounded non-preemptible/IRQ-off window per core. If you'd rather keep it preemptible, I can run the per-core loop
as per-CPU work instead. Which do you prefer?

Thanks,
Ashish

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

[Borislav Petkov]

Kalra, Ashish <ashish.kalra@amd.com> wrote:
>Followers are RMP-scan cache hits (cheap) and this only runs at setup/teardown

How?

I see the workqueue running periodically. And in talking about periodically, RMPOPT_WORK_TIMEOUT is not used anywhere.

What's up?

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

[Kalra, Ashish]

On 8/3/2026 2:24 PM, Borislav Petkov wrote:
> Kalra, Ashish <ashish.kalra@amd.com> wrote:
>> Followers are RMP-scan cache hits (cheap) and this only runs at setup/teardown
> 
> How?
> 
> I see the workqueue running periodically. And in talking about periodically, RMPOPT_WORK_TIMEOUT is not used anywhere.
> 
> What's up?

RMPOPT_WORK_TIMEOUT is used by snp_rmpopt_all_physmem() (mod_delayed_work()), which is added in the next patch — I'll
move the define there so each patch is self-contained. 

It isn't periodic either: the handler doesn't re-arm itself. The work runs at setup and re-init with queue_delayed_work(), 
and then per SNP-guest teardown via mod_delayed_work() (batched by the timeout) once the later patches wire that up.

Thanks,
Ashish

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

[Borislav Petkov]

On August 3, 2026 7:37:59 PM UTC, "Kalra, Ashish" <ashish.kalra@amd.com> wrote:
>RMPOPT_WORK_TIMEOUT is used by snp_rmpopt_all_physmem() (mod_delayed_work()), which is added in the next patch — I'll
>move the define there so each patch is self-contained. 

Yes, very much so.

>It isn't periodic either: the handler doesn't re-arm itself. The work runs at setup and re-init with queue_delayed_work(), 
>and then per SNP-guest teardown via mod_delayed_work() (batched by the timeout) once the later patches wire that up.

Then I am missing an important aspect here: RMPOPT optimizes RMP checks but then what's the point if you do this only on SNP guest teardown and I'm assuming at guest init too? 

What happens to long-running guests which never get restarted? 

This would lead to the system fragmenting ad absurdum and us never optimizing the RMP table practically.

But the guest could, in its lifetime, free up a contiguous 1G. When are we optimizing this one then?

Is that what the sentence about optimizing during 1GB page cleanup in your 0th message all about? 

In any case, your commit message needs to talk about when those optimization runs are happening and why.

Thx.

-- 
Small device. Typos and formatting crap

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

[Borislav Petkov]

On July 31, 2026 12:37:15 PM UTC, "Kalra, Ashish" <ashish.kalra@amd.com> wrote:
>Yes, the checks are done on *all* memory but for ensuring the integrity of SNP 
>guest memory, so that is what the above paragraph is mentioning.

I don't understand: we're checking *all* memory in order to ensure SNP guest memory integrity? 

We're ensuring the integrity of the other memory just as well :-)

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

Ok.

-- 
Small device. Typos and formatting crap

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

[Kalra, Ashish]

On 8/3/2026 4:02 PM, Borislav Petkov wrote:
> On August 3, 2026 7:37:59 PM UTC, "Kalra, Ashish" <ashish.kalra@amd.com> wrote:
>> RMPOPT_WORK_TIMEOUT is used by snp_rmpopt_all_physmem() (mod_delayed_work()), which is added in the next patch — I'll
>> move the define there so each patch is self-contained. 
> 
> Yes, very much so.
> 
>> It isn't periodic either: the handler doesn't re-arm itself. The work runs at setup and re-init with queue_delayed_work(), 
>> and then per SNP-guest teardown via mod_delayed_work() (batched by the timeout) once the later patches wire that up.
> 
> Then I am missing an important aspect here: RMPOPT optimizes RMP checks but then what's the point if you do this only on SNP guest teardown and I'm assuming at guest init too? 
> 
> What happens to long-running guests which never get restarted? 
> 
> This would lead to the system fragmenting ad absurdum and us never optimizing the RMP table practically.
> 
> But the guest could, in its lifetime, free up a contiguous 1G. When are we optimizing this one then?
> 
> Is that what the sentence about optimizing during 1GB page cleanup in your 0th message all about? 

The case where a guest frees a contiguous 1 GB mid-life is exactly the follow-on I mentioned in the cover letter —
yes, that's what the "optimizing during 1 GB page cleanup" sentence is about: once 1 GB hugetlb guest_memfd support lands,
RMPOPT re-optimization will hook that 1 GB cleanup path. I'll spell out these trigger points in the commit message.

Thanks,
Ashish

> 
> In any case, your commit message needs to talk about when those optimization runs are happening and why.
> 
> Thx.
> 

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

[Borislav Petkov]

On August 3, 2026 9:23:41 PM UTC, "Kalra, Ashish" <ashish.kalra@amd.com> wrote:
>yes, that's what the "optimizing during 1 GB page cleanup" sentence is about:

What about doing the optimization runs periodically?

-- 
Small device. Typos and formatting crap

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

[Kalra, Ashish]

On 8/3/2026 4:38 PM, Borislav Petkov wrote:
> On August 3, 2026 9:23:41 PM UTC, "Kalra, Ashish" <ashish.kalra@amd.com> wrote:
>> yes, that's what the "optimizing during 1 GB page cleanup" sentence is about:
> 
> What about doing the optimization runs periodically?
> 

Periodic re-optimization was already considered in an earlier RMPOPT series and dropped because of being too disruptive — 
a timer-based scan would run on all cores every time it fires, even when no memory has been freed. Running it at guest
teardown (batched by the timeout) is a better fit: teardown is when large contiguous regions actually go back to the
hypervisor, so the scan only runs when there's real work to do instead of on a fixed timer. The case of a guest freeing
a 1 GB region mid-life is then handled by the 1 GB-hugetlb page-cleanup follow-on, not by a periodic scan.

I will also like to avoid looping on this — periodic vs teardown was already worked through in an earlier revision, and
this is where we landed. Happy to revisit if there's a specific new concern, but otherwise I'd prefer to keep the
current approach.

Thanks,
Ashish

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

[Borislav Petkov]

On Mon, Aug 03, 2026 at 05:22:17PM -0500, Kalra, Ashish wrote:
> Periodic re-optimization was already considered in an earlier RMPOPT series

Link?

> and dropped because of being too disruptive — a timer-based scan would run
> on all cores every time it fires, even when no memory has been freed.

What if it ran very seldomly, say once an hour?

> Running it at guest teardown (batched by the timeout) is a better fit:
> teardown is when large contiguous regions actually go back to the
> hypervisor, so the scan only runs when there's real work to do instead of on
> a fixed timer. The case of a guest freeing a 1 GB region mid-life is then
> handled by the 1 GB-hugetlb page-cleanup follow-on, not by a periodic scan.

Are you saying a periodic scan won't handle anything?

Because if it catches something, it will handle that something.

> I will also like to avoid looping on this — periodic vs teardown was already
> worked through in an earlier revision, and this is where we landed. Happy to
> revisit if there's a specific new concern, but otherwise I'd prefer to keep
> the current approach.

Sorry, this is not how upstream review works.

-- 
Regards/Gruss,
    Boris.

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

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

[Kalra, Ashish]

On 8/4/2026 7:49 PM, Borislav Petkov wrote:
> On Mon, Aug 03, 2026 at 05:22:17PM -0500, Kalra, Ashish wrote:
>> Periodic re-optimization was already considered in an earlier RMPOPT series
> 
> Link?

Re-optimizing at guest termination rather than via a periodic/runtime interface was Dave Hansen's suggestion in the v2
review — see https://lore.kernel.org/kvm/5102edd8-8eaa-4688-b3f7-3004c4cbc8f3@intel.com/"

> 
>> and dropped because of being too disruptive — a timer-based scan would run
>> on all cores every time it fires, even when no memory has been freed.
> 
> What if it ran very seldomly, say once an hour?
> 
>> Running it at guest teardown (batched by the timeout) is a better fit:
>> teardown is when large contiguous regions actually go back to the
>> hypervisor, so the scan only runs when there's real work to do instead of on
>> a fixed timer. The case of a guest freeing a 1 GB region mid-life is then
>> handled by the 1 GB-hugetlb page-cleanup follow-on, not by a periodic scan.
> 
> Are you saying a periodic scan won't handle anything?
> 
> Because if it catches something, it will handle that something.
A couple more reasons the event-driven trigger fits better than a fixed period:

- Under churn (many SNP guests terminating and respawning), an hourly scan is too coarse — freed regions would sit
un-optimized for up to an hour. The teardown trigger re-optimizes right after each burst of terminations (batched by the
timeout), so it's more responsive than a periodic scan exactly when there's the most optimization work to do.
- When no SNP guests run, a periodic scan achieves nothing: the initial pass already optimized all of memory and
nothing has changed, so it just re-scans physmem for zero benefit.

So the event-driven approach self-scales with real work — it runs as often as memory is actually freed and not at all
when the system is idle — whereas a fixed timer is both too slow under load and pure overhead when idle. The mid-life
contiguous-1 GB case is then caught precisely by the 1 GB-hugetlb page-cleanup hook when that lands, rather than by a
coarse sweep.

Thanks,
Ashish

> 
>> I will also like to avoid looping on this — periodic vs teardown was already
>> worked through in an earlier revision, and this is where we landed. Happy to
>> revisit if there's a specific new concern, but otherwise I'd prefer to keep
>> the current approach.
> 
> Sorry, this is not how upstream review works.
> 

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

[Dave Hansen]

On 8/4/26 17:49, Borislav Petkov wrote:
>> and dropped because of being too disruptive — a timer-based scan would run
>> on all cores every time it fires, even when no memory has been freed.
> What if it ran very seldomly, say once an hour?

The kernel knows what events open up the system to new RMP
optimizations. They don't happen randomly or without the kernel knowing.

A scan once an hour for very long-lived guests will waste resources once
an hour.

A scan once an hour when guests have shorter lifetimes means that there
will be hour-long windows when the system could have been optimized but
was not.

>> Running it at guest teardown (batched by the timeout) is a better fit:
>> teardown is when large contiguous regions actually go back to the
>> hypervisor, so the scan only runs when there's real work to do instead of on
>> a fixed timer. The case of a guest freeing a 1 GB region mid-life is then
>> handled by the 1 GB-hugetlb page-cleanup follow-on, not by a periodic scan.
> Are you saying a periodic scan won't handle anything?

A periodic scan could be useful but only if it coincides with a VM
teardown event, or other event that opens up new RMP optimizations. I
_think_ we decided that with current upstream code, the only event that
matters is VM teardown.

The thing I really don't like about picking an arbitrary window is that
there's no right or wrong time. Folks are then going to want to tune it.
They're going to want knobs, and ABI and documentation.

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

[Borislav Petkov]

On Tue, Aug 04, 2026 at 09:38:05PM -0500, Kalra, Ashish wrote:
> - Under churn (many SNP guests terminating and respawning), an hourly scan
> is too coarse — freed regions would sit un-optimized for up to an hour. The
> teardown trigger re-optimizes right after each burst of terminations
> (batched by the timeout), so it's more responsive than a periodic scan
> exactly when there's the most optimization work to do.

Yes.

> - When no SNP guests run, a periodic scan achieves nothing: the initial pass
> already optimized all of memory and nothing has changed, so it just re-scans
> physmem for zero benefit.

Well, no. You won't even run that when there are no SNP guests present.

> So the event-driven approach self-scales with real work — it runs as often
> as memory is actually freed and not at all when the system is idle — whereas
> a fixed timer is both too slow under load and pure overhead when idle. The
> mid-life contiguous-1 GB case is then caught precisely by the 1 GB-hugetlb
> page-cleanup hook when that lands, rather than by a coarse sweep.

I wanted to have this in case pages do get freed up from time to time and you
get some sort of a fragmentation. But I'm being told that we don't support
balooning yet, neither memory hotplug and when the pages get freed, they
remain guest-owned. So yes, the points in time where those page become
HV-owned and potentially freed, then we should optimize.

Please hold down that in the commit message so that it is clear for future
readers why we did it this way.

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Borislav Petkov]

On Wed, Aug 05, 2026 at 08:12:45AM -0700, Dave Hansen wrote:
> A periodic scan could be useful but only if it coincides with a VM
> teardown event, or other event that opens up new RMP optimizations. I
> _think_ we decided that with current upstream code, the only event that
> matters is VM teardown.

Yeah, see my reply to Ashish just now - it turns out we don't even have events
which will open up potential opportunities for RMP optimizations besides guest
teardown. So yeah, we should be doing it then only.

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Kalra, Ashish]

On 8/5/2026 2:28 PM, Borislav Petkov wrote:
> On Tue, Aug 04, 2026 at 09:38:05PM -0500, Kalra, Ashish wrote:
>> - Under churn (many SNP guests terminating and respawning), an hourly scan
>> is too coarse — freed regions would sit un-optimized for up to an hour. The
>> teardown trigger re-optimizes right after each burst of terminations
>> (batched by the timeout), so it's more responsive than a periodic scan
>> exactly when there's the most optimization work to do.
> 
> Yes.
> 
>> - When no SNP guests run, a periodic scan achieves nothing: the initial pass
>> already optimized all of memory and nothing has changed, so it just re-scans
>> physmem for zero benefit.
> 
> Well, no. You won't even run that when there are no SNP guests present.
> 
>> So the event-driven approach self-scales with real work — it runs as often
>> as memory is actually freed and not at all when the system is idle — whereas
>> a fixed timer is both too slow under load and pure overhead when idle. The
>> mid-life contiguous-1 GB case is then caught precisely by the 1 GB-hugetlb
>> page-cleanup hook when that lands, rather than by a coarse sweep.
> 
> I wanted to have this in case pages do get freed up from time to time and you
> get some sort of a fragmentation. But I'm being told that we don't support
> balooning yet, neither memory hotplug and when the pages get freed, they
> remain guest-owned. So yes, the points in time where those page become
> HV-owned and potentially freed, then we should optimize.
> 
> Please hold down that in the commit message so that it is clear for future
> readers why we did it this way.
> 

Will do — I have updated the patch 6 commit message to spell out why re-optimization is tied to guest teardown: SNP guests don't
support ballooning or memory hotplug yet, so pages freed during a guest's lifetime stay guest-owned, and teardown is the only
point where memory returns to hypervisor ownership and becomes eligible for re-optimization. That will be in the next revision.

Thanks,
Ashish