Re: [PATCH v2 2/7] x86/sev: add support for enabling RMPOPT

[Tom Lendacky]

On 3/6/26 09:18, Borislav Petkov wrote:
> On Mon, Mar 02, 2026 at 09:35:55PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@xxxxxxx>
>>
>> The new RMPOPT instruction sets bits in a per-CPU RMPOPT table, which
>> indicates whether specific 1GB physical memory regions contain SEV-SNP
> 
> "... which indicate... "
> 
>> guest memory.
>>
>> 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.
> 
> ...
> 
>> +static void __configure_rmpopt(void *val)
>> +{
>> +	u64 rmpopt_base = ((u64)val & PUD_MASK) | MSR_AMD64_RMPOPT_ENABLE;
>> +
>> +	wrmsrq(MSR_AMD64_RMPOPT_BASE, rmpopt_base);
>> +}
>> +
>> +static __init void configure_and_enable_rmpopt(void)
> 
> If the sub-helper is called __configure_rmpopt() then this should be called
> "configure_rmpopt", without the prepended underscores.
> 
>> +	phys_addr_t pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), PUD_SIZE);
>> +
>> +	if (!cpu_feature_enabled(X86_FEATURE_RMPOPT)) {
>> +		pr_debug("RMPOPT not supported on this platform\n");
>> +		return;
>> +	}
>> +
>> +	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP)) {
>> +		pr_debug("RMPOPT optimizations not enabled as SNP support is not enabled\n");
>> +		return;
>> +	}
> 
> Zap this one - snp_rmptable_init() already checked it.
> 
> Also, zap those pr_debugs - you have that information elsewhere already.
> 
>> +
>> +	if (!(rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)) {
> 
> You can't test this one - you need to test the result of
> setup_segmented_rmptable() and whether it did set up the segmented RMP
> properly. Only then you can continue here.

If the segmented RMP setup fails, then CC_ATTR_HOST_SEV_SNP gets cleared,
so it looks like the above check needs to remain then.

Thanks,
Tom

> 
>> +		pr_info("RMPOPT optimizations not enabled, segmented RMP required\n");
> 
> This looks like pr_notice() to me.
> 
>> +		return;
>> +	}
>