Re: [PATCH v4 1/2] x86/sev: Clear RMPOPT when segmented RMP is disabled by firmware

[Borislav Petkov]

On Mon, Nov 02, 2026 at 09:50:32AM -0600, Kalra, Ashish wrote:
> The original patch returns false if segmented RMP is enabled but
> probe_segmented_rmptable_info() fails, your version falls through to
> probe_contiguous_rmptable_info() instead. 
> 
> I believe it makes sense to keep the return false there: on a
> segmented RMP enabled platform the RMP is segmented, not contiguous, so the
> contiguous probe shouldn't run.

That simplifies the code even more:

bool snp_probe_rmptable_info(void)
{
        if (!cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
                goto out;

        rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
        if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
                return probe_segmented_rmptable_info();

out:
        setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
        return probe_contiguous_rmptable_info();
}

Along with the below, which should have happened too:

1. RMPOPT depends on SEV_SNP, ofc. This would make sure RMPOPT is cleared
   everytime SNP is cleared, as it should be.

2. snp_enable_rmpopt() needs to check SNP first, ofc, and then RMPOPT.

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 99801e844b30..acbcec394ef1 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -93,6 +93,7 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_SPEC_CTRL_SSBD,		X86_FEATURE_SPEC_CTRL },
 	{ X86_FEATURE_LASS,			X86_FEATURE_SMAP      },
 	{ X86_FEATURE_INVLPGB,			X86_FEATURE_PCID      },
+	{ X86_FEATURE_RMPOPT,			X86_FEATURE_SEV_SNP   },
 	{}
 };
 
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index ff6d17795054..a47b812b2ea3 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -669,10 +669,10 @@ void snp_enable_rmpopt(void)
 	u64 base;
 	int cpu;
 
-	if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
+	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
 		return;
 
-	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+	if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
 		return;
 
 	if (!rmpopt_soft_init)

---

here's the full diff. Have a look pls and let me know what else I am missing:

---

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 99801e844b30..acbcec394ef1 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -93,6 +93,7 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_SPEC_CTRL_SSBD,		X86_FEATURE_SPEC_CTRL },
 	{ X86_FEATURE_LASS,			X86_FEATURE_SMAP      },
 	{ X86_FEATURE_INVLPGB,			X86_FEATURE_PCID      },
+	{ X86_FEATURE_RMPOPT,			X86_FEATURE_SEV_SNP   },
 	{}
 };
 
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index bd70c4d0b774..a47b812b2ea3 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -669,10 +669,10 @@ void snp_enable_rmpopt(void)
 	u64 base;
 	int cpu;
 
-	if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
+	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
 		return;
 
-	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+	if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
 		return;
 
 	if (!rmpopt_soft_init)
@@ -817,20 +817,15 @@ static bool probe_segmented_rmptable_info(void)
 
 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 (probe_segmented_rmptable_info())
-				return true;
+	if (!cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
+		goto out;
 
-			setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
-			return false;
-		}
-	} else {
-		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
-	}
+	rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
+	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
+		return probe_segmented_rmptable_info();
 
+out:
+	setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
 	return probe_contiguous_rmptable_info();
 }

---

Thx.

-- 
Regards/Gruss,
    Boris.

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