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

[Tom Lendacky]

On 11/2/26 00:12, K Prateek Nayak wrote:
> Hello Boris,
> 
> On 11/2/2026 10:58 AM, Borislav Petkov wrote:
>> Segmented RMP support is advertised via X86_FEATURE_SEGMENTED_RMP, and the
>> firmware enables it via MSR_AMD64_RMP_CFG[SEG_RMP_ENABLED]. The latter can be
>> cleared in platform BIOS by the user, in which case the system runs in
>> contiguous RMP setting.
> 
> nit. In the latter case, SNP is disabled entirely in bsp_determine_snp and
> the system won't continue with contiguous RMP.

Heh, I don't think that is a nit, it is broken.

I think the code as it was read fine, but that's just my opinion.

Thanks,
Tom
> 
>> snp_probe_rmptable_info() does not clear X86_FEATURE_RMPOPT in the
>> advertised-but-firmware-disabled case.
>>
>> As a result, this does not prevent snp_enable_rmpopt() from trying to program
>> MSR_AMD64_RMPOPT_BASE, which raises a #GP on the MSR access.
>>
>> Make sure X86_FEATURE_RMPOPT is cleared in all cases where segmented RMP is
>> not enabled.
>>
>>   [ bp:
>>    - Put the commit message on Ozempic
>>    - Flip the logic in the function so that the segmented RMP disabled
>>      case goes first and the flow gets simpler this way. ]
>>
>> Fixes: 15d974be0a3b ("x86/sev: Initialize RMPOPT configuration MSRs")
>> Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
>> Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
>> Link: https://patch.msgid.link/56da047d63b22f03a7e709fc172a5543da894d67.1793216372.git.ashish.kalra@xxxxxxx
> 
> Feel free to include:
> 
> Reviewed-by: K Prateek Nayak <kprateek.nayak@xxxxxxx>
> 
>> ---
>>  arch/x86/virt/svm/sev.c | 21 ++++++++++-----------
>>  1 file changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index bd70c4d0b774..ff6d17795054 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -817,21 +817,20 @@ 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 (!cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP)) {
>> +		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
>>  
>> -		if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
>> -			if (probe_segmented_rmptable_info())
>> -				return true;
>> +		return probe_contiguous_rmptable_info();
>> +	}
>>  
>> -			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) {
>> +		if (probe_segmented_rmptable_info())
>> +			return true;
>>  	}
>>  
>> -	return probe_contiguous_rmptable_info();
>> +	setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
>> +	return false;
>>  }
>>  
>>  /*
>