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

[Kalra, Ashish]

Hello Dave,

On 3/2/2026 4:32 PM, Dave Hansen wrote:

>> +static __init void configure_and_enable_rmpopt(void)
>> +{
>> +	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;
>> +	}
> 
> To be honest, I think those two are just plain noise ^^.

They are basically pr_debug's, so won't really cause noise generally.

> 
>> +	if (!(rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)) {
>> +		pr_info("RMPOPT optimizations not enabled, segmented RMP required\n");
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * Per-CPU RMPOPT tables support at most 2 TB of addressable memory for RMP optimizations.
>> +	 *
>> +	 * Set per-core RMPOPT base to min_low_pfn to enable RMP optimization for
>> +	 * up to 2TB of system RAM on all CPUs.
>> +	 */
> 
> Please at least be consistent with your comments. This is both over 80
> columns *and* not even consistent in the two sentences.

Sure.

> 
>> +	on_each_cpu_mask(cpu_online_mask, __configure_rmpopt, (void *)pa_start, true);
>> +}
> 
> What's wrong with:
> 
> 	u64 rmpopt_base = pa_start | MSR_AMD64_RMPOPT_ENABLE;
> 	...
> 	for_each_online_cpu(cpu)
> 		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> 
> Then there's at least no ugly casting.
> 

RMOPT_BASE MSRs don't need to be set serially, therefore, by
using the cpu_online_mask and on_each_cpu_mask(), we can setup the MSRs
concurrently and in parallel. Using for_each_online_cpu() will be slower than
doing on_each_cpu_mask() as it doesn't send IPIs in parallel, right.

Thanks,
Ashish