* [PATCH v13 1/5] x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag
2026-09-02 21:27 [PATCH v13 0/5] Add RMPOPT support Ashish Kalra
@ 2026-09-02 21:27 ` Ashish Kalra
2026-09-02 21:28 ` [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active Ashish Kalra
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Ashish Kalra @ 2026-09-02 21:27 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
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>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
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 f70ee74b5f92..3b5b32d3391b 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 8665a6474806..d1795ce219da 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active
2026-09-02 21:27 [PATCH v13 0/5] Add RMPOPT support Ashish Kalra
2026-09-02 21:27 ` [PATCH v13 1/5] x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag Ashish Kalra
@ 2026-09-02 21:28 ` Ashish Kalra
2026-09-02 21:51 ` sashiko-bot
2026-09-02 21:28 ` [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs Ashish Kalra
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Ashish Kalra @ 2026-09-02 21:28 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
From: Ashish Kalra <ashish.kalra@amd.com>
While SNP is active, every memory write is checked against the RMP to
protect SNP guest memory. A core performs these RMP checks only once
SNP has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is
set on that core; the firmware requires the SNP-enable bit to be set on
every present CPU before SNP initialization.
A core that is not SNP-enabled and not SNP-initialized performs no RMP
checks at all, so there is no valid configuration with SNP active and any
CPU exempt from RMP checks.
The firmware determines which CPUs are present from the processor and the
BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates
them at SNP init; it is not aware of the OS bringing CPUs online or
offline afterwards.
SNP_INIT fails unless SnpEn is set on all CPUs, so a CPU that is offline
when SNP_INIT is issued, does not have SnpEn set, SNP_INIT fails, and
there can be no SNP guest memory. OS CPU hotplug can thus diverge from
the firmware's expectations and break SNP.
Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
disabled SNP.
If snp_prepare() fails before enabling SNP it re-enables hotplug itself;
once SNP is enabled hotplug stays disabled, including across a failed
SNP_INIT and across the legacy SNP_SHUTDOWN_EX path, both of which leave
SNP enabled.
A kexec target that boots with SNP already enabled, disables hotplug once
in snp_rmptable_init(), since snp_prepare() bails when SNP is already
enabled.
With CPU hotplug now disabled while SNP is active, the online CPU mask is
stable, so the cpus_read_lock() previously taken in snp_prepare() to
iterate it is redundant. Drop cpus_read_lock()/cpus_read_unlock() here.
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/virt/svm/sev.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index cff285d8ad8e..558f7924a3f8 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,18 @@ int snp_prepare(void)
clear_rmp();
- cpus_read_lock();
+ /*
+ * No CPU may come online without SnpEn while SNP is active; disable
+ * hotplug here and re-enable it in snp_shutdown().
+ */
+ cpu_hotplug_disable();
if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
- ret = -EOPNOTSUPP;
+ cpu_hotplug_enable();
pr_warn("SNP init failed: not all CPUs online. (%*pbl online <-> %*pbl present masks).\n",
cpumask_pr_args(cpu_online_mask),
cpumask_pr_args(cpu_present_mask));
- goto unlock;
+ return -EOPNOTSUPP;
}
wbinvd_on_all_cpus();
@@ -548,12 +551,7 @@ int snp_prepare(void)
/* SNP_INIT requires MSR_VM_HSAVE_PA to be cleared on all CPUs. */
on_each_cpu(clear_hsave_pa, NULL, 1);
- ret = 0;
-
-unlock:
- cpus_read_unlock();
-
- return ret;
+ return 0;
}
EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
@@ -567,6 +565,13 @@ void snp_shutdown(void)
clear_rmp();
on_each_cpu(mfd_reconfigure, NULL, 1);
+
+ /*
+ * The firmware has disabled SNP (SnpEn is clear), so re-enable CPU
+ * hotplug. A legacy SNP shutdown returns above with SnpEn still set and
+ * leaves hotplug disabled.
+ */
+ cpu_hotplug_enable();
}
EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
@@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
*/
int __init snp_rmptable_init(void)
{
+ u64 val;
+
if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
return -ENOSYS;
@@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
if (!setup_rmptable())
return -ENOSYS;
+ /*
+ * On a kexec boot SNP may already be enabled (legacy firmware leaves
+ * SnpEn set across shutdown), in which case snp_prepare() bails without
+ * disabling CPU hotplug, so disable it here.
+ */
+ rdmsrq(MSR_AMD64_SYSCFG, val);
+ if (val & MSR_AMD64_SYSCFG_SNP_EN)
+ cpu_hotplug_disable();
+
/*
* Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
* notifier is invoked to do SNP IOMMU shutdown before kdump.
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active
2026-09-02 21:28 ` [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active Ashish Kalra
@ 2026-09-02 21:51 ` sashiko-bot
2026-09-02 22:12 ` Kalra, Ashish
0 siblings, 1 reply; 16+ messages in thread
From: sashiko-bot @ 2026-09-02 21:51 UTC (permalink / raw)
To: Ashish Kalra; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] PM Suspend (S3) bypasses CPU hotplug disable, leading to security breakdown
- [High] CPU hotplug remains enabled on kexec boot if setup_rmptable() fails
--
commit c91487bb659a8ce73e581c086c8aa1aa19f4b9aa
Author: Ashish Kalra <ashish.kalra@amd.com>
x86/sev: Disable CPU hotplug while SNP is active
This patch disables CPU hotplug while SNP is active to ensure no CPU comes
online without the SnpEn bit set. It ties hotplug to the SNP-enable bit by
disabling it in snp_prepare() and re-enabling it in snp_shutdown().
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index cff285d8ad8e3..558f7924a3f85 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
[ ... ]
> @@ -526,14 +525,18 @@ int snp_prepare(void)
>
> clear_rmp();
>
> - cpus_read_lock();
> + /*
> + * No CPU may come online without SnpEn while SNP is active; disable
> + * hotplug here and re-enable it in snp_shutdown().
> + */
> + cpu_hotplug_disable();
[Severity: Critical]
Does this sequence prevent CPUs from being onlined without SnpEn set during
power management transitions?
While cpu_hotplug_disable() blocks sysfs-initiated hotplug operations, it
appears that system suspend (S3) and hibernation (S4) bypass this mechanism.
During suspend, the PM core uses freeze_secondary_cpus() and
thaw_secondary_cpus(), which explicitly call _cpu_down() and _cpu_up()
regardless of the cpu_hotplug_disabled counter.
Additionally, MSR_AMD64_SYSCFG is not included in the list of MSRs saved and
restored during PM transitions in arch/x86/power/cpu.c.
If a system with SNP active goes to sleep and resumes, will the secondary
CPUs wake up without the SnpEn bit set, potentially breaking the SEV-SNP
security model by failing to enforce RMP checks?
[ ... ]
> @@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
> */
> int __init snp_rmptable_init(void)
> {
> + u64 val;
> +
> if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
> return -ENOSYS;
>
> @@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
> if (!setup_rmptable())
> return -ENOSYS;
[Severity: High]
If setup_rmptable() fails and returns -ENOSYS early, do we leave CPU hotplug
enabled even though the hardware might still have SNP enabled from a previous
kernel?
When booting via kexec from an SNP-enabled kernel, the CPUs retain the SnpEn
bit in MSR_AMD64_SYSCFG. If setup_rmptable() fails here (for example, due to
a memremap failure), the function exits before reaching the new check that
disables CPU hotplug.
Because the function failed, the kernel clears CC_ATTR_HOST_SEV_SNP. If a CPU
is subsequently hotplugged via sysfs, the new kernel will not set SnpEn on
it, resulting in a state where boot CPUs have SnpEn set but the hotplugged
CPU does not.
Could this early return be moved after the MSR_AMD64_SYSCFG_SNP_EN check to
ensure hotplug is disabled whenever the hardware indicates SNP is active?
> +
> + /*
> + * 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.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788296083.git.ashish.kalra@amd.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active
2026-09-02 21:51 ` sashiko-bot
@ 2026-09-02 22:12 ` Kalra, Ashish
0 siblings, 0 replies; 16+ messages in thread
From: Kalra, Ashish @ 2026-09-02 22:12 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm
On 9/2/2026 4:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] PM Suspend (S3) bypasses CPU hotplug disable, leading to security breakdown
> - [High] CPU hotplug remains enabled on kexec boot if setup_rmptable() fails
> --
>
> commit c91487bb659a8ce73e581c086c8aa1aa19f4b9aa
> Author: Ashish Kalra <ashish.kalra@amd.com>
> x86/sev: Disable CPU hotplug while SNP is active
>
> This patch disables CPU hotplug while SNP is active to ensure no CPU comes
> online without the SnpEn bit set. It ties hotplug to the SNP-enable bit by
> disabling it in snp_prepare() and re-enabling it in snp_shutdown().
>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index cff285d8ad8e3..558f7924a3f85 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>
> [ ... ]
>
>> @@ -526,14 +525,18 @@ int snp_prepare(void)
>>
>> clear_rmp();
>>
>> - cpus_read_lock();
>> + /*
>> + * No CPU may come online without SnpEn while SNP is active; disable
>> + * hotplug here and re-enable it in snp_shutdown().
>> + */
>> + cpu_hotplug_disable();
>
> [Severity: Critical]
> Does this sequence prevent CPUs from being onlined without SnpEn set during
> power management transitions?
>
> While cpu_hotplug_disable() blocks sysfs-initiated hotplug operations, it
> appears that system suspend (S3) and hibernation (S4) bypass this mechanism.
> During suspend, the PM core uses freeze_secondary_cpus() and
> thaw_secondary_cpus(), which explicitly call _cpu_down() and _cpu_up()
> regardless of the cpu_hotplug_disabled counter.
>
> Additionally, MSR_AMD64_SYSCFG is not included in the list of MSRs saved and
> restored during PM transitions in arch/x86/power/cpu.c.
>
> If a system with SNP active goes to sleep and resumes, will the secondary
> CPUs wake up without the SnpEn bit set, potentially breaking the SEV-SNP
> security model by failing to enforce RMP checks?
SEV-SNP hosts do not support suspend-to-RAM (S3) or hibernation (S4). The
SNP security state that would have to survive a PM transition — the per-CPU
SnpEn bit, the RMP table, and the SNP-locked AMD IOMMU — is not saved and
restored across suspend/resume, so a resumed SNP host would be in an
inconsistent state independently of CPU hotplug. (That is also why
MSR_AMD64_SYSCFG is not in the PM save/restore set.)
This patch does not change any of that. It replaces the existing
cpus_read_lock() in snp_prepare() with cpu_hotplug_disable(); neither
touches the PM suspend/resume path (freeze_secondary_cpus() /
thaw_secondary_cpus()). The patch's purpose is narrower: to stop a runtime,
sysfs-initiated CPU online from bringing a CPU up without SnpEn while SNP is
active. The S3/S4 vector is pre-existing.
>
> [ ... ]
>
>> @@ -577,6 +582,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>> */
>> int __init snp_rmptable_init(void)
>> {
>> + u64 val;
>> +
>> if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
>> return -ENOSYS;
>>
>> @@ -586,6 +593,15 @@ int __init snp_rmptable_init(void)
>> if (!setup_rmptable())
>> return -ENOSYS;
>
> [Severity: High]
> If setup_rmptable() fails and returns -ENOSYS early, do we leave CPU hotplug
> enabled even though the hardware might still have SNP enabled from a previous
> kernel?
>
> When booting via kexec from an SNP-enabled kernel, the CPUs retain the SnpEn
> bit in MSR_AMD64_SYSCFG. If setup_rmptable() fails here (for example, due to
> a memremap failure), the function exits before reaching the new check that
> disables CPU hotplug.
>
> Because the function failed, the kernel clears CC_ATTR_HOST_SEV_SNP. If a CPU
> is subsequently hotplugged via sysfs, the new kernel will not set SnpEn on
> it, resulting in a state where boot CPUs have SnpEn set but the hotplugged
> CPU does not.
>
> Could this early return be moved after the MSR_AMD64_SYSCFG_SNP_EN check to
> ensure hotplug is disabled whenever the hardware indicates SNP is active?
>
On any platform that supports RMPOPT (which requires a segmented RMP), a
kexec performs a full SNP shutdown: the CCP driver's .shutdown handler
clears SnpEn (SNP_SHUTDOWN via __sev_snp_shutdown_locked). So on a kexec
boot of such a platform SnpEn is already clear — snp_rmptable_init() reads
it as clear and the machine takes the normal path, where snp_prepare()
disables CPU hotplug. The "SnpEn still set from the previous kernel" case
does not arise here.
That case only happens with legacy firmware that leaves SnpEn set across a
kexec shutdown, and those platforms do not support RMPOPT. On such a
platform, if setup_rmptable() additionally fails, SNP is not brought up at
all — snp_rmptable_init() returns -ENOSYS and CC_ATTR_HOST_SEV_SNP is
cleared — so no SNP guest runs. The SnpEn-set / hotplug-enabled state there
is pre-existing SEV-SNP behaviour (snp_rmptable_init() did not touch CPU
hotplug before this series), not a regression introduced here.
Thanks,
Ashish
>> +
>> + /*
>> + * 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.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs
2026-09-02 21:27 [PATCH v13 0/5] Add RMPOPT support Ashish Kalra
2026-09-02 21:27 ` [PATCH v13 1/5] x86/cpufeatures: Add X86_FEATURE_RMPOPT feature flag Ashish Kalra
2026-09-02 21:28 ` [PATCH v13 2/5] x86/sev: Disable CPU hotplug while SNP is active Ashish Kalra
@ 2026-09-02 21:28 ` Ashish Kalra
2026-09-02 21:44 ` sashiko-bot
2026-09-02 21:28 ` [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously Ashish Kalra
2026-09-02 21:29 ` [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown Ashish Kalra
4 siblings, 1 reply; 16+ messages in thread
From: Ashish Kalra @ 2026-09-02 21:28 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
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>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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 | 58 +++++++++++++++++++++++++++++---
drivers/crypto/ccp/sev-dev.c | 2 ++
4 files changed, 60 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 3a8e51a0c9e8..1635e2e1c576 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 9e7a077c445d..5638d09b5132 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 558f7924a3f8..07f178eb76c7 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -124,6 +124,8 @@ static void *rmp_bookkeeping __ro_after_init;
static u64 probed_rmp_base, probed_rmp_size;
+static phys_addr_t rmpopt_pa_start;
+
static LIST_HEAD(snp_leaked_pages_list);
static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
@@ -555,6 +557,16 @@ int snp_prepare(void)
}
EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
+static void rmpopt_disable(void)
+{
+ int cpu;
+
+ for_each_cpu(cpu, cpu_primary_thread_mask)
+ wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
+
+ rmpopt_pa_start = 0;
+}
+
void snp_shutdown(void)
{
u64 syscfg;
@@ -563,6 +575,8 @@ void snp_shutdown(void)
if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
return;
+ rmpopt_disable();
+
clear_rmp();
on_each_cpu(mfd_reconfigure, NULL, 1);
@@ -575,6 +589,32 @@ 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;
+
+ 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, cpu_primary_thread_mask)
+ 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
@@ -699,13 +739,21 @@ static bool probe_segmented_rmptable_info(void)
bool snp_probe_rmptable_info(void)
{
- if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
+ 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
- return probe_contiguous_rmptable_info();
+ 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 supported without
+ * segmented RMP.
+ */
+ 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 f833cb7e4da3..287a8345854b 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
sev_es_tmr_size = SNP_TMR_SIZE;
+ snp_setup_rmpopt();
+
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs
2026-09-02 21:28 ` [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs Ashish Kalra
@ 2026-09-02 21:44 ` sashiko-bot
2026-09-02 21:56 ` Kalra, Ashish
0 siblings, 1 reply; 16+ messages in thread
From: sashiko-bot @ 2026-09-02 21:44 UTC (permalink / raw)
To: Ashish Kalra; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] CPU hotplug lock is leaked if SEV-SNP initialization fails.
--
commit 16bd9580b377dc645541bd4f478c471c109e65ae
Author: Ashish Kalra <ashish.kalra@amd.com>
x86/sev: Initialize RMPOPT configuration MSRs
This commit introduces the RMPOPT configuration MSRs and adds support to
set up and enable RMPOPT once SNP is initialized. The RMPOPT table base
is initialized to enable RMP optimization for up to 2 TB of system RAM.
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index f833cb7e4da36..287a8345854ba 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>
> sev_es_tmr_size = SNP_TMR_SIZE;
>
> + snp_setup_rmpopt();
> +
[Severity: High]
This is a pre-existing issue, but does __sev_snp_init_locked() permanently
leak the CPU hotplug lock if SEV-SNP initialization fails?
If snp_prepare() succeeds, it disables CPU hotplug. However, if an error
occurs later in the function (for example, if walk_iomem_res_desc() fails),
the function returns the error code without re-enabling CPU hotplug:
drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
...
rc = snp_prepare();
if (rc)
return rc;
...
rc = walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_MEM, 0, ~0,
snp_range_list, snp_filter_reserved_mem_regions);
if (rc) {
dev_err(sev->dev,
"SEV: SNP_INIT_EX walk_iomem_res_desc failed rc = %d\n", rc);
return rc;
}
...
}
This leaves cpu_hotplug_disabled strictly greater than zero, which would
permanently prevent CPUs from being onlined or offlined. Does this function
need an error handling path that calls cpu_hotplug_enable() before returning
on these failure paths?
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788296083.git.ashish.kalra@amd.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs
2026-09-02 21:44 ` sashiko-bot
@ 2026-09-02 21:56 ` Kalra, Ashish
0 siblings, 0 replies; 16+ messages in thread
From: Kalra, Ashish @ 2026-09-02 21:56 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm
On 9/2/2026 4:44 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] CPU hotplug lock is leaked if SEV-SNP initialization fails.
> --
>
> commit 16bd9580b377dc645541bd4f478c471c109e65ae
> Author: Ashish Kalra <ashish.kalra@amd.com>
>
> x86/sev: Initialize RMPOPT configuration MSRs
>
> This commit introduces the RMPOPT configuration MSRs and adds support to
> set up and enable RMPOPT once SNP is initialized. The RMPOPT table base
> is initialized to enable RMP optimization for up to 2 TB of system RAM.
>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index f833cb7e4da36..287a8345854ba 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1663,6 +1663,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>
>> sev_es_tmr_size = SNP_TMR_SIZE;
>>
>> + snp_setup_rmpopt();
>> +
>
> [Severity: High]
> This is a pre-existing issue, but does __sev_snp_init_locked() permanently
> leak the CPU hotplug lock if SEV-SNP initialization fails?
>
> If snp_prepare() succeeds, it disables CPU hotplug. However, if an error
> occurs later in the function (for example, if walk_iomem_res_desc() fails),
> the function returns the error code without re-enabling CPU hotplug:
>
> drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked() {
> ...
> rc = snp_prepare();
> if (rc)
> return rc;
> ...
> rc = walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_MEM, 0, ~0,
> snp_range_list, snp_filter_reserved_mem_regions);
> if (rc) {
> dev_err(sev->dev,
> "SEV: SNP_INIT_EX walk_iomem_res_desc failed rc = %d\n", rc);
> return rc;
> }
> ...
> }
>
> This leaves cpu_hotplug_disabled strictly greater than zero, which would
> permanently prevent CPUs from being onlined or offlined. Does this function
> need an error handling path that calls cpu_hotplug_enable() before returning
> on these failure paths?
>
Thanks — but this is intentional, not a leak. CPU hotplug is tied to
SnpEn, not to the success of __sev_snp_init_locked().
snp_prepare() disables hotplug and then sets SnpEn on all CPUs via
on_each_cpu(snp_enable, ...) before it returns 0. So by the time
walk_iomem_res_desc() (and the other post-snp_prepare() steps) run,
SnpEn is already set on every online CPU and the machine is in SNP mode.
While SnpEn is set, a CPU must not come online without it — that is
exactly what disabling hotplug guards against. Re-enabling hotplug on
the walk_iomem_res_desc() failure path, with SnpEn still set, would
reopen that unsafe window, so hotplug is deliberately left disabled
there. It is re-enabled only once SnpEn is actually cleared, on the
x86_snp_shutdown path: snp_shutdown() verifies SnpEn is clear and then
calls cpu_hotplug_enable().
The one snp_prepare() failure that occurs before SnpEn is set — the
"not all CPUs online" check — does re-enable hotplug itself. There is no
failure path between cpu_hotplug_disable() and snp_enable() that can
return with hotplug disabled and SnpEn clear (the intervening steps are
void). So hotplug tracks SnpEn on every path.
Thanks,
Ashish
>> return 0;
>> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously
2026-09-02 21:27 [PATCH v13 0/5] Add RMPOPT support Ashish Kalra
` (2 preceding siblings ...)
2026-09-02 21:28 ` [PATCH v13 3/5] x86/sev: Initialize RMPOPT configuration MSRs Ashish Kalra
@ 2026-09-02 21:28 ` Ashish Kalra
2026-09-02 21:36 ` Dave Hansen
2026-09-02 21:57 ` sashiko-bot
2026-09-02 21:29 ` [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown Ashish Kalra
4 siblings, 2 replies; 16+ messages in thread
From: Ashish Kalra @ 2026-09-02 21:28 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
From: Ashish Kalra <ashish.kalra@amd.com>
When SNP is enabled, all writes to memory are checked to ensure memory
integrity. This imposes performance overhead on the whole system.
RMPOPT is a new instruction that minimizes the performance overhead of
RMP checks on the hypervisor and on non-SNP guests by allowing RMP
checks to be skipped for 1GB regions of memory that are known not to
contain any SNP guest memory.
Add support for performing RMP optimizations asynchronously using a
dedicated workqueue.
At RMP initialization time, run an optimization pass over all physical
memory (up to 2TB of system RAM, starting from the lowest physical
memory address aligned down to a 1GB boundary), skipping RMP checks for
1GB regions that do not contain SNP guest memory (excluding preassigned
pages such as the RMP table and firmware pages).
As SNP guests are launched, RMPUPDATE assigns their private pages to
guest-owned state; when such a page falls within an optimized 1GB
region, the hardware clears that region's RMPOPT optimization and RMP
checks resume there to protect the guest memory.
Since launching SNP guests clears these optimizations, perform them
again asynchronously using the dedicated workqueue.
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/virt/svm/sev.c | 113 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 111 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 07f178eb76c7..a4c0fe49b9ec 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>
@@ -124,7 +125,16 @@ static void *rmp_bookkeeping __ro_after_init;
static u64 probed_rmp_base, probed_rmp_size;
-static phys_addr_t rmpopt_pa_start;
+static u64 rmpopt_pa_start, rmpopt_pa_end;
+
+enum rmpopt_op_type {
+ RMPOPT_OP_VERIFY_AND_REPORT_STATUS,
+ RMPOPT_OP_REPORT_STATUS
+};
+
+static struct workqueue_struct *rmpopt_wq;
+static struct delayed_work rmpopt_delayed_work;
+static DEFINE_MUTEX(rmpopt_wq_mutex);
static LIST_HEAD(snp_leaked_pages_list);
static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
@@ -561,10 +571,26 @@ static void rmpopt_disable(void)
{
int cpu;
+ guard(mutex)(&rmpopt_wq_mutex);
+
+ /*
+ * rmpopt_wq is non-NULL only after RMPOPT has been fully set up: the
+ * workqueue is allocated and the RMPOPT_BASE MSRs are programmed.
+ * snp_setup_rmpopt() resets it to NULL if any of those steps fail, so a
+ * NULL rmpopt_wq means nothing was set up and there is nothing to tear
+ * down.
+ */
+ if (!rmpopt_wq)
+ return;
+
+ cancel_delayed_work_sync(&rmpopt_delayed_work);
+ destroy_workqueue(rmpopt_wq);
+
for_each_cpu(cpu, cpu_primary_thread_mask)
wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
- rmpopt_pa_start = 0;
+ rmpopt_pa_start = rmpopt_pa_end = 0;
+ rmpopt_wq = NULL;
}
void snp_shutdown(void)
@@ -595,6 +621,44 @@ static bool rmpopt_capable(void)
cc_platform_has(CC_ATTR_HOST_SEV_SNP);
}
+/*
+ * RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
+ * memory does not contain any SNP guest memory.
+ *
+ * @pa is a system physical address; RMPOPT operates on the containing 1GB.
+ */
+static void rmpopt(u64 pa)
+{
+ enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
+ u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
+
+ asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
+ :: "a" (pa_start), "c" (op)
+ : "memory", "cc");
+}
+
+/* on_each_cpu() callback: optimize the whole RMPOPT range on this CPU. */
+static void rmpopt_scan_range(void *arg)
+{
+ u64 pa;
+
+ for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
+ rmpopt(pa);
+}
+
+static void do_rmpopt_work(struct work_struct *work)
+{
+ /*
+ * RMPOPT caches the results of a RMP table scan in reserved processor
+ * memory, allowing future invocations to skip such costly operations.
+ */
+ migrate_disable();
+ rmpopt_scan_range(NULL);
+ migrate_enable();
+
+ on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
+}
+
void snp_setup_rmpopt(void)
{
u64 rmpopt_base;
@@ -603,6 +667,34 @@ void snp_setup_rmpopt(void)
if (!rmpopt_capable())
return;
+ guard(mutex)(&rmpopt_wq_mutex);
+
+ /*
+ * On re-initialization after a legacy SNP shutdown (SNP_SHUTDOWN_EX
+ * with x86_snp_shutdown=0), snp_shutdown() and thus rmpopt_disable() are
+ * skipped, so the workqueue, delayed work and per-CPU RMPOPT_BASE MSRs
+ * are still set up and valid (SnpEn stayed set and CPU hotplug stayed
+ * disabled). Rather than re-doing the setup, which would leak the
+ * existing state, just re-queue the optimization pass to re-optimize any
+ * memory the previous SNP session de-optimized.
+ */
+ if (rmpopt_wq) {
+ queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
+ return;
+ }
+
+ /*
+ * Create an RMPOPT-specific workqueue to avoid scheduling
+ * RMPOPT workitem on the global system workqueue.
+ */
+ rmpopt_wq = alloc_workqueue("rmpopt_wq", WQ_UNBOUND, 1);
+ if (!rmpopt_wq) {
+ pr_err("Failed to allocate RMPOPT workqueue\n");
+ return;
+ }
+
+ INIT_DELAYED_WORK(&rmpopt_delayed_work, do_rmpopt_work);
+
rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
@@ -612,6 +704,23 @@ void snp_setup_rmpopt(void)
*/
for_each_cpu(cpu, cpu_primary_thread_mask)
wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
+
+ rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
+
+ /* Limit memory scanning to 2TB of RAM */
+ if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
+ pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
+ rmpopt_pa_start + SZ_2T);
+ rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
+ }
+
+ /*
+ * Once all per-CPU RMPOPT tables have been configured, enable RMPOPT
+ * optimizations on all physical memory.
+ */
+ queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
+
+ pr_info("RMPOPT optimizations enabled\n");
}
EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously
2026-09-02 21:28 ` [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously Ashish Kalra
@ 2026-09-02 21:36 ` Dave Hansen
2026-09-02 21:57 ` sashiko-bot
1 sibling, 0 replies; 16+ messages in thread
From: Dave Hansen @ 2026-09-02 21:36 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
On 9/2/26 14:28, Ashish Kalra wrote:
> Add support for performing RMP optimizations asynchronously using a
> dedicated workqueue.
This is looking fine to me at this point:
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously
2026-09-02 21:28 ` [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously Ashish Kalra
2026-09-02 21:36 ` Dave Hansen
@ 2026-09-02 21:57 ` sashiko-bot
2026-09-02 22:36 ` Kalra, Ashish
1 sibling, 1 reply; 16+ messages in thread
From: sashiko-bot @ 2026-09-02 21:57 UTC (permalink / raw)
To: Ashish Kalra; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Declare %rax as clobbered or output in rmpopt() to prevent infinite loops.
- [High] Schedule work in process context; executing on_each_cpu_mask triggers hard lockups.
- [High] Use ALIGN_DOWN for rmpopt_pa_end to prevent out-of-bounds RMP table reads.
--
commit 0121e9d5bd127536db9548e1a9f202c9576ef5ff
Author: Ashish Kalra <ashish.kalra@amd.com>
x86/sev: Add support to perform RMP optimizations asynchronously
This commit introduces support for asynchronous RMP checks on 1GB physical
memory regions to optimize performance on hardware supporting RMPOPT. A
dedicated workqueue triggers the optimization process during SNP
initialization and after legacy shutdowns.
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 07f178eb76c7e..a4c0fe49b9ec7 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
[ ... ]
> @@ -595,6 +621,44 @@ static bool rmpopt_capable(void)
> cc_platform_has(CC_ATTR_HOST_SEV_SNP);
> }
>
> +/*
> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
> + * memory does not contain any SNP guest memory.
> + *
> + * @pa is a system physical address; RMPOPT operates on the containing 1GB.
> + */
> +static void rmpopt(u64 pa)
> +{
> + enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
> + u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
> +
> + asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> + :: "a" (pa_start), "c" (op)
> + : "memory", "cc");
[Severity: Critical]
Does this inline assembly need to declare %rax as an output or clobber?
Since the RMPOPT hardware instruction returns a status code in %rax, could
GCC assume %rax is unmodified because it is only provided as an input? If
rmpopt() is inlined, GCC might allocate %rax for the induction variable pa
in the calling loop. This would cause the loop step to compute incorrectly,
potentially resulting in an infinite loop during boot.
> +}
> +
> +/* on_each_cpu() callback: optimize the whole RMPOPT range on this CPU. */
> +static void rmpopt_scan_range(void *arg)
> +{
> + u64 pa;
> +
> + for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> + rmpopt(pa);
> +}
> +
> +static void do_rmpopt_work(struct work_struct *work)
> +{
> + /*
> + * RMPOPT caches the results of a RMP table scan in reserved processor
> + * memory, allowing future invocations to skip such costly operations.
> + */
> + migrate_disable();
> + rmpopt_scan_range(NULL);
> + migrate_enable();
> +
> + on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
[Severity: High]
Could executing on_each_cpu_mask() here with wait=true trigger hard lockups?
This call runs the rmpopt_scan_range() loop synchronously on all primary
threads in an IPI context with hardirqs disabled. Since the loop can
iterate up to 2048 times invoking the slow RMPOPT hardware instruction,
is it possible for remote CPUs with cold caches (like on multi-socket
systems) to stall for seconds inside the IPI handler and trigger an NMI
watchdog panic?
> +}
[ ... ]
> @@ -612,6 +704,23 @@ void snp_setup_rmpopt(void)
> */
> for_each_cpu(cpu, cpu_primary_thread_mask)
> wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> +
> + rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
[Severity: High]
Should rmpopt_pa_end be calculated using ALIGN_DOWN() instead of ALIGN()?
If max_pfn is not 1GB aligned, aligning upwards creates a hardware scanning
range that extends into unbacked physical memory. When rmpopt_scan_range()
processes this final 1GB block, could the hardware instruction attempt to
read RMP entries beyond the allocated RMP table and cause a Machine Check
Exception or memory corruption?
> +
> + /* 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;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788296083.git.ashish.kalra@amd.com?part=4
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously
2026-09-02 21:57 ` sashiko-bot
@ 2026-09-02 22:36 ` Kalra, Ashish
0 siblings, 0 replies; 16+ messages in thread
From: Kalra, Ashish @ 2026-09-02 22:36 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm
On 9/2/2026 4:57 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] Declare %rax as clobbered or output in rmpopt() to prevent infinite loops.
> - [High] Schedule work in process context; executing on_each_cpu_mask triggers hard lockups.
> - [High] Use ALIGN_DOWN for rmpopt_pa_end to prevent out-of-bounds RMP table reads.
> --
>
> commit 0121e9d5bd127536db9548e1a9f202c9576ef5ff
> Author: Ashish Kalra <ashish.kalra@amd.com>
>
> x86/sev: Add support to perform RMP optimizations asynchronously
>
> This commit introduces support for asynchronous RMP checks on 1GB physical
> memory regions to optimize performance on hardware supporting RMPOPT. A
> dedicated workqueue triggers the optimization process during SNP
> initialization and after legacy shutdowns.
>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index 07f178eb76c7e..a4c0fe49b9ec7 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>
> [ ... ]
>
>> @@ -595,6 +621,44 @@ static bool rmpopt_capable(void)
>> cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>> }
>>
>> +/*
>> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
>> + * memory does not contain any SNP guest memory.
>> + *
>> + * @pa is a system physical address; RMPOPT operates on the containing 1GB.
>> + */
>> +static void rmpopt(u64 pa)
>> +{
>> + enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
>> + u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
>> +
>> + asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
>> + :: "a" (pa_start), "c" (op)
>> + : "memory", "cc");
>
> [Severity: Critical]
> Does this inline assembly need to declare %rax as an output or clobber?
>
> Since the RMPOPT hardware instruction returns a status code in %rax, could
> GCC assume %rax is unmodified because it is only provided as an input? If
> rmpopt() is inlined, GCC might allocate %rax for the induction variable pa
> in the calling loop. This would cause the loop step to compute incorrectly,
> potentially resulting in an infinite loop during boot.
>
RMPOPT does not modify RAX — its only output is the carry flag (RFLAGS.CF),
which the asm already covers with the "cc" clobber. RAX (the 1GB-aligned
SPA) and RCX (the op type) are inputs only and are left unchanged, unlike
RMPREAD/PSMASH/RMPUPDATE which do return a status in RAX. So the input-only
"a"(pa_start) constraint describes RMPOPT correctly; GCC may keep the loop's
pa in RAX and the loop step is correct. (rmpopt() intentionally discards CF
on this path; only the debug/report path reads it, via a separate asm with a
"=@ccc" output.)
>> +}
>> +
>> +/* on_each_cpu() callback: optimize the whole RMPOPT range on this CPU. */
>> +static void rmpopt_scan_range(void *arg)
>> +{
>> + u64 pa;
>> +
>> + for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>> + rmpopt(pa);
>> +}
>> +
>> +static void do_rmpopt_work(struct work_struct *work)
>> +{
>> + /*
>> + * RMPOPT caches the results of a RMP table scan in reserved processor
>> + * memory, allowing future invocations to skip such costly operations.
>> + */
>> + migrate_disable();
>> + rmpopt_scan_range(NULL);
>> + migrate_enable();
>> +
>> + on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
>
> [Severity: High]
> Could executing on_each_cpu_mask() here with wait=true trigger hard lockups?
>
> This call runs the rmpopt_scan_range() loop synchronously on all primary
> threads in an IPI context with hardirqs disabled. Since the loop can
> iterate up to 2048 times invoking the slow RMPOPT hardware instruction,
> is it possible for remote CPUs with cold caches (like on multi-socket
> systems) to stall for seconds inside the IPI handler and trigger an NMI
> watchdog panic?
>
The fan-out is reached only after the single warm-up scan that precedes it:
do_rmpopt_work() runs rmpopt_scan_range() once on the current CPU under
migrate_disable() (interrupts on, preemptible), which does the expensive RMP
scan and stores the result in the RMPOPT table. All primary threads are
programmed with the same RMPOPT_BASE, so that table is shared; the subsequent
on_each_cpu_mask() therefore issues only cache-hit RMPOPTs — reads of the
already-computed result, including on remote sockets — not re-scans.
Measured over a 2 TB window: the cache-hit fan-out is ~0.32 s per core —
~30x under the 10 s NMI watchdog even with ~2048 iterations and cold data caches.
The IRQ-disabled window per follower is bounded by the cache-hit cost, not the
scan cost.
>> +}
>
> [ ... ]
>
>> @@ -612,6 +704,23 @@ void snp_setup_rmpopt(void)
>> */
>> for_each_cpu(cpu, cpu_primary_thread_mask)
>> wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
>> +
>> + rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
>
> [Severity: High]
> Should rmpopt_pa_end be calculated using ALIGN_DOWN() instead of ALIGN()?
>
> If max_pfn is not 1GB aligned, aligning upwards creates a hardware scanning
> range that extends into unbacked physical memory. When rmpopt_scan_range()
> processes this final 1GB block, could the hardware instruction attempt to
> read RMP entries beyond the allocated RMP table and cause a Machine Check
> Exception or memory corruption?
RMPOPT does not read outside the RMP table in a way that can fault. It only
reads RMP table entries to determine whether the 1 GB region is entirely HV-owned;
for any address in the region that lies outside the RMP table's range, the
instruction simply treats that address as HV-owned. It does not raise a #MC
and does not perform an out-of-bounds access.
Rounding rmpopt_pa_end up with ALIGN() is therefore intentional and safe: it
lets the final, partially-populated 1 GB of RAM be optimized — the backed
part is checked normally and any tail beyond the RMP table is treated as
HV-owned — with no risk of reading past the table. Using ALIGN_DOWN() would
instead leave that last partial-1 GB region un-optimized for no safety
benefit, so ALIGN() is deliberate here.
Thanks,
Ashish
>
>> +
>> + /* 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;
>> + }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown
2026-09-02 21:27 [PATCH v13 0/5] Add RMPOPT support Ashish Kalra
` (3 preceding siblings ...)
2026-09-02 21:28 ` [PATCH v13 4/5] x86/sev: Add support to perform RMP optimizations asynchronously Ashish Kalra
@ 2026-09-02 21:29 ` Ashish Kalra
2026-09-02 21:37 ` Dave Hansen
2026-09-02 22:01 ` sashiko-bot
4 siblings, 2 replies; 16+ messages in thread
From: Ashish Kalra @ 2026-09-02 21:29 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, x86, hpa, seanjc, peterz,
thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
From: Ashish Kalra <ashish.kalra@amd.com>
The RMPOPT table is a per-CPU table which indicates whether 1GB regions
of physical memory are entirely hypervisor-owned.
When performing host memory accesses in hypervisor mode as well as
non-SNP guest mode, the processor may consult the RMPOPT table to
potentially skip an RMP access and improve performance.
Normal guest events disable RMP optimizations: pages are converted from
shared to private as SNP guests are launched, and large pages are split
and collapsed during guest operation -- both disable the RMPOPT
optimizations for the affected 1GB regions.
When guests are torn down, their pages are converted back to shared, so
those regions may become eligible for RMPOPT optimization again. Without
some intervention, all RMP optimizations would eventually be lost, so
re-optimize all of physical memory on SNP guest teardown.
Perform the re-optimization after a delay, using mod_delayed_work() so
that the delay timer is reset on each call. This batches multiple guest
terminations into a single pass: the re-optimization runs 10 seconds
after the *last* termination rather than after the first.
mod_delayed_work() also re-queues work that is already in-flight, so a
re-scan request during an active scan is not silently dropped.
Guest teardown is currently the only event that returns guest memory to
hypervisor ownership: SNP guests do not support ballooning or memory
hotplug, so pages freed during a guest's lifetime remain guest-owned.
It is therefore the only point at which memory becomes eligible for RMP
re-optimization, which is why re-optimization is driven by guest
teardown rather than by a periodic scan.
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/sev.h | 2 ++
arch/x86/kvm/svm/sev.c | 10 ++++++++++
arch/x86/virt/svm/sev.c | 27 +++++++++++++++++++++++++++
3 files changed, 39 insertions(+)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 5638d09b5132..3235e171647d 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/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5705723f1f41..0babf96e38c6 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3032,6 +3032,16 @@ void sev_vm_destroy(struct kvm *kvm)
*/
if (snp_decommission_context(kvm))
return;
+
+ /*
+ * Perform RMP optimizations on memory freed by terminating
+ * guests. The scan is deferred, so it normally runs after
+ * sev_gmem_invalidate() has converted this guest's pages back to
+ * shared, and picks them up then. A very large guest whose
+ * conversion has not finished by then is picked up by a later
+ * teardown's scan.
+ */
+ snp_rmpopt_all_physmem();
} else {
sev_unbind_asid(kvm, sev->handle);
}
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index a4c0fe49b9ec..fb66a1aa6fc3 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -659,6 +659,33 @@ static void do_rmpopt_work(struct work_struct *work)
on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
}
+/*
+ * Delay, in milliseconds, before the RMP re-optimization pass runs after an
+ * SNP guest is torn down. snp_rmpopt_all_physmem() re-arms the delayed work
+ * with mod_delayed_work() on each teardown, so the pass fires this long after
+ * the last teardown. This coalesces a burst of teardowns into a single scan
+ * and gives each guest's pages time to be converted back to the shared,
+ * hypervisor-owned state before the scan re-optimizes their 1GB regions. The
+ * 10 second value is a heuristic trading re-optimization latency against
+ * scanning too eagerly.
+ */
+#define RMPOPT_WORK_TIMEOUT (10 * MSEC_PER_SEC)
+
+void snp_rmpopt_all_physmem(void)
+{
+ if (!rmpopt_capable())
+ return;
+
+ guard(mutex)(&rmpopt_wq_mutex);
+
+ if (!rmpopt_wq)
+ return;
+
+ mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
+ msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
+}
+EXPORT_SYMBOL_FOR_MODULES(snp_rmpopt_all_physmem, "kvm-amd");
+
void snp_setup_rmpopt(void)
{
u64 rmpopt_base;
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown
2026-09-02 21:29 ` [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown Ashish Kalra
@ 2026-09-02 21:37 ` Dave Hansen
2026-09-02 22:01 ` sashiko-bot
1 sibling, 0 replies; 16+ messages in thread
From: Dave Hansen @ 2026-09-02 21:37 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
On 9/2/26 14:29, Ashish Kalra wrote:
> Perform the re-optimization after a delay, using mod_delayed_work() so
> that the delay timer is reset on each call. This batches multiple guest
> terminations into a single pass: the re-optimization runs 10 seconds
> after the *last* termination rather than after the first.
> mod_delayed_work() also re-queues work that is already in-flight, so a
> re-scan request during an active scan is not silently dropped.
Simple enough.
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown
2026-09-02 21:29 ` [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown Ashish Kalra
2026-09-02 21:37 ` Dave Hansen
@ 2026-09-02 22:01 ` sashiko-bot
2026-09-02 22:45 ` Kalra, Ashish
1 sibling, 1 reply; 16+ messages in thread
From: sashiko-bot @ 2026-09-02 22:01 UTC (permalink / raw)
To: Ashish Kalra; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Redundant and costly RMP optimization scan in hardirq context causing severe CPU stalls
--
commit f95e55ff4f3219c5a1e0b2d2fc34fde539f227f7
Author: Ashish Kalra <ashish.kalra@amd.com>
x86/sev: Re-enable RMP optimizations on SNP guest shutdown
This commit introduces a mechanism to re-optimize memory on SNP guest
shutdown using a delayed work queue to batch multiple terminations.
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index a4c0fe49b9ec7..fb66a1aa6fc35 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
[ ... ]
> @@ -659,6 +659,33 @@ static void do_rmpopt_work(struct work_struct *work)
> on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
> }
>
> +/*
> + * Delay, in milliseconds, before the RMP re-optimization pass runs after an
> + * SNP guest is torn down.
[ ... ]
> +void snp_rmpopt_all_physmem(void)
> +{
> + if (!rmpopt_capable())
> + return;
> +
> + guard(mutex)(&rmpopt_wq_mutex);
> +
> + if (!rmpopt_wq)
> + return;
> +
> + mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
> + msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
> +}
[Severity: High]
This is a pre-existing issue, but queueing the do_rmpopt_work task upon guest
teardown exposes the host to severe CPU stalls.
Looking at do_rmpopt_work, it broadcasts an IPI to all primary threads:
arch/x86/virt/svm/sev.c:do_rmpopt_work() {
...
migrate_disable();
rmpopt_scan_range(NULL);
migrate_enable();
on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
}
The IPI handler rmpopt_scan_range executes the rmpopt hardware instruction
in hardirq context over up to 2TB of memory:
arch/x86/virt/svm/sev.c:rmpopt_scan_range() {
...
for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
rmpopt(pa);
...
}
Since rmpopt is a very slow instruction, does iterating it over massive
memory ranges inside an IPI handler lead to soft lockups or host denial of
service, especially if frequent VM churn repeatedly tears down SNP guests
to trigger this scan?
Additionally, because rmpopt_wq is created as WQ_UNBOUND, the worker
thread might run on an SMT sibling. If so, wouldn't the initial local scan
execute on a CPU where MSR_AMD64_RMPOPT_BASE was never configured,
potentially leading to undefined behavior or faults?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788296083.git.ashish.kalra@amd.com?part=5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v13 5/5] x86/sev: Re-enable RMP optimizations on SNP guest shutdown
2026-09-02 22:01 ` sashiko-bot
@ 2026-09-02 22:45 ` Kalra, Ashish
0 siblings, 0 replies; 16+ messages in thread
From: Kalra, Ashish @ 2026-09-02 22:45 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm
On 9/2/2026 5:01 PM, sashiko-bot@kernel.org wrote:
> Pre-existing issues:
> - [High] Redundant and costly RMP optimization scan in hardirq context causing severe CPU stalls
> --
>
> commit f95e55ff4f3219c5a1e0b2d2fc34fde539f227f7
> Author: Ashish Kalra <ashish.kalra@amd.com>
> x86/sev: Re-enable RMP optimizations on SNP guest shutdown
>
> This commit introduces a mechanism to re-optimize memory on SNP guest
> shutdown using a delayed work queue to batch multiple terminations.
>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index a4c0fe49b9ec7..fb66a1aa6fc35 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
> [ ... ]
>> @@ -659,6 +659,33 @@ static void do_rmpopt_work(struct work_struct *work)
>> on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
>> }
>>
>> +/*
>> + * Delay, in milliseconds, before the RMP re-optimization pass runs after an
>> + * SNP guest is torn down.
> [ ... ]
>> +void snp_rmpopt_all_physmem(void)
>> +{
>> + if (!rmpopt_capable())
>> + return;
>> +
>> + guard(mutex)(&rmpopt_wq_mutex);
>> +
>> + if (!rmpopt_wq)
>> + return;
>> +
>> + mod_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
>> + msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
>> +}
> [Severity: High]
> This is a pre-existing issue, but queueing the do_rmpopt_work task upon guest
> teardown exposes the host to severe CPU stalls.
>
> Looking at do_rmpopt_work, it broadcasts an IPI to all primary threads:
>
> arch/x86/virt/svm/sev.c:do_rmpopt_work() {
> ...
> migrate_disable();
> rmpopt_scan_range(NULL);
> migrate_enable();
>
> on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);
> }
>
> The IPI handler rmpopt_scan_range executes the rmpopt hardware instruction
> in hardirq context over up to 2TB of memory:
>
> arch/x86/virt/svm/sev.c:rmpopt_scan_range() {
> ...
> for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> rmpopt(pa);
> ...
> }
>
> Since rmpopt is a very slow instruction, does iterating it over massive
> memory ranges inside an IPI handler lead to soft lockups or host denial of
> service, especially if frequent VM churn repeatedly tears down SNP guests
> to trigger this scan?
Two things bound this:
1. Coalescing. snp_rmpopt_all_physmem() uses mod_delayed_work() with a 10 s
delay that is reset on every call, so a burst of guest teardowns does not
queue one scan per teardown — the pass runs once, 10 s after the last
teardown. Frequent VM churn is coalesced into a single pass, not amplified.
2. The IPI does not run an uncached scan. do_rmpopt_work() first runs
rmpopt_scan_range() once on the worker's own CPU under migrate_disable()
(interrupts on, preemptible), which does the expensive part and populates
the shared RMPOPT table — all primary threads share the same RMPOPT_BASE.
Only then does on_each_cpu_mask() fan out, and those RMPOPTs are cache hits
(reads of the already-computed result), so the IRQ-disabled window per CPU
is bounded by the cache-hit cost, not the scan cost — measured at ~320 ms
over the largest configuration tested, well under the soft-lockup/NMI
thresholds.
>
> Additionally, because rmpopt_wq is created as WQ_UNBOUND, the worker
> thread might run on an SMT sibling. If so, wouldn't the initial local scan
> execute on a CPU where MSR_AMD64_RMPOPT_BASE was never configured,
> potentially leading to undefined behavior or faults?
MSR_AMD64_RMPOPT_BASE has core scope, so programming it on each core's primary
thread configures it for both SMT threads of that core. A WQ_UNBOUND worker
that happens to run on a secondary thread therefore still executes RMPOPT
against a validly-configured RMPOPT_BASE (its core's), and since CPU hotplug
is disabled while SNP is active, every core stays configured. There is no CPU
on which RMPOPT_BASE is unconfigured, so no fault or undefined behaviour.
Thanks,
Ashish
^ permalink raw reply [flat|nested] 16+ messages in thread