Re: [PATCH v10 4/6] x86/sev: Add support to perform RMP optimizations asynchronously

["Kalra, Ashish" <ashish.kalra@amd.com>]

Hello Prateek,

On 7/21/2026 10:06 AM, K Prateek Nayak wrote:
> Hello Ashish,
> 
> On 6/30/2026 11:41 PM, Ashish Kalra wrote:
>> +	/*
>> +	 * RMPOPT scans the RMP table, stores the result of the scan in the
>> +	 * reserved processor memory. The RMP scan is the most expensive
>> +	 * part. If a second RMPOPT occurs, it can skip the expensive scan
>> +	 * if they can see a cached result in the reserved processor memory.
>> +	 *
>> +	 * Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
>> +	 * on every other primary thread. Followers are "designed to"
>> +	 * skip the scan if they see the "cached" scan results.
>> +	 *
>> +	 * Pin the worker to the current CPU for the leader loop so that
>> +	 * this_cpu remains valid and the RMPOPT instruction executes on
>> +	 * the correct CPU.  Use migrate_disable() rather than get_cpu() to
>> +	 * prevent migration while still allowing preemption.
>> +	 */
>> +	migrate_disable();
>> +	this_cpu = smp_processor_id();
>> +
>> +	cpumask_andnot(follower_mask, rmpopt_cpumask,
>> +		       topology_sibling_cpumask(this_cpu));
>> +
>> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>> +		rmpopt(pa);
>> +		cond_resched();
>> +	}
>> +	migrate_enable();
>> +
>> +	/*
>> +	 * Followers: run RMPOPT on remaining cores.  CPUs cannot go offline
>> +	 * while SNP is active, so the follower set stays valid across the
>> +	 * scan and cpus_read_lock() is uncontended.
>> +	 */
>> +	scoped_guard(cpus_read_lock) {
> 
> We can only reach here after disabling hotplug. Do we still to hold the
> cpus_read_lock?
> 

Right — hotplug is disabled for the whole SNP-active window and this work only runs while SNP is active, so the follower set is already stable.
I'll drop the cpus_read_lock() and document it:

        /*
         * Followers: run RMPOPT on the remaining cores.  cpus_read_lock() is
         * intentionally not held here: CPU hotplug is disabled for the entire
         * time SNP is active (see snp_prepare()), and this work only runs while
         * SNP is active, so the follower set stays valid across the whole scan.
         */

I'll also trim the matching sentence in patch 3's commit log ("ensures cpus_read_lock() in the scan is uncontended"). 
 
Actually one of the reasons, why i am still holding the cpus_read_lock is that Sashiko is likely to flag the *bare* on_each_cpu_mask() as missing
cpus_read_lock() — the comment above captures why it's safe, so the reasoning is in-tree if it comes up.

>> +		for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>> +			on_each_cpu_mask(follower_mask, rmpopt_smp,
>> +					 (void *)pa, true);
>> +
>> +			/* Give a chance for other threads to run */
>> +			cond_resched();
> 
> Ever since commit 7dadeaa6e851 ("sched: Further restrict the preemption
> modes"), one cannot select PREEMPT_VOLUNTARY on x86 and cond_resched()
> is a nop with all the other preemption models (PREEMPT_NONE will not
> preempt a kernel thread; PREEMPT_LAZY / PREEMPT / PREEMPT_RT all make
> the kernel fully preemptible except for sections with
> preempt_count != 0)
> 
> You can drop these cond_resched() calls in your respin.
> 

Will do — with PREEMPT_VOLUNTARY unavailable on x86 and the remaining models fully preemptible, so cond_resched() are nops. Dropping both.

Thanks,
Ashish

>> +		}
>> +	}
>> +
>> +	free_cpumask_var(follower_mask);
>> +}
>> +
>