Re: [PATCH v14 4/5] x86/sev: Perform RMP optimizations asynchronously

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Re-initializing SNP after a CPU hotplug cycle skips programming the RMPOPT_BASE MSR on the newly onlined CPU, leading to an Invalid Opcode (#UD) exception when executing RMPOPT.
- [High] The inline assembly for the RMPOPT instruction fails to declare `%rax` as an output constraint or clobber, leading to silent register corruption and unpredictable loop behavior.
- [High] The RMPOPT 'warm-up' executes on a single CPU, causing the subsequent cross-CPU IPI to execute millions of slow RMPOPT instructions with IRQs disabled on all other CPUs with cold caches, triggering hard lockups.
--

commit e2c3ac4ab3639e4c47a6c85a3acca0d88b6fb5a3
Author: Ashish Kalra <ashish.kalra@amd.com>

x86/sev: Perform RMP optimizations asynchronously

This patch introduces asynchronous RMP optimizations by scanning and
optimizing up to 2TB of system memory at 1GB boundaries using a per-CPU
workqueue. When SNP guests are launched and regions are allocated, hardware
clears the optimizations, so the workqueue asynchronously reapplies them.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index a059327dc1071..35678b1f535d4 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
[ ... ]
> +static void rmpopt(u64 pa)
> +{
> +	enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
> +
> +	/* Supported by binutils 2.48+ */
> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     :: "a" (pa_start), "c" (op)
> +		     : "memory", "cc");

[Severity: High]
Does this inline assembly need to declare %rax as an output or clobbered
register?

Since RMPOPT_OP_VERIFY_AND_REPORT_STATUS instructs the hardware to return a
status code in %rax, the compiler might allocate the loop counter pa to %rax
in rmpopt_scan_range(). If hardware overwrites %rax, the loop counter would
be reset, potentially causing an infinite loop.

> +}
> +
> +/* on_each_cpu() callback: optimize the whole RMPOPT range on this CPU. */
> +static void rmpopt_scan_range(void *arg)
> +{
> +	u64 pa;
> +
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		rmpopt(pa);
> +}
> +
> +static void do_rmpopt_work(struct work_struct *work)
> +{
> +	/*
> +	 * Warm up the RMPOPT cache on this pinned per-CPU worker with interrupts
> +	 * on, so the IRQ-disabled fan-out below only issues cache-hit RMPOPTs.
> +	 */
> +	rmpopt_scan_range(NULL);
> +
> +	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);

[Severity: High]
Could this fan-out cause significant stalls on other CPUs?

The RMPOPT cache is warmed up locally on the current CPU, but because the
caches are strictly per-CPU, all other CPUs will still have a cold cache.
When they receive the IPI, they will perform the full up to 2TB scan in
hardirq context with a cold cache, which seems likely to trigger NMI
watchdog timeouts across the system.

> +}
> +
> +void snp_setup_rmpopt(void)
> +{
> +	u64 rmpopt_base;
> +	int cpu;
> +
> +	if (!rmpopt_capable())
> +		return;
> +
> +	guard(mutex)(&rmpopt_wq_mutex);
> +
> +	/*
> +	 * Set up once: the workqueue and RMPOPT_BASE MSRs are left in place on
> +	 * shutdown, so a later re-initialization just re-queues the optimization
> +	 * pass rather than redoing the setup.
> +	 */
> +	if (rmpopt_wq) {
> +		queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work, 0);
> +		return;

[Severity: High]
Is it safe to skip programming MSR_AMD64_RMPOPT_BASE on newly onlined CPUs?

If a CPU is hot-unplugged and hot-plugged back, its MSRs are reset to their
defaults. If SNP is disabled and re-enabled during this cycle,
snp_setup_rmpopt() will see that rmpopt_wq is already allocated and return
early. This skips the MSR programming loop for the hotplugged CPU, which
would lead to an Invalid Opcode (#UD) exception when executing RMPOPT on it.

> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788984053.git.ashish.kalra@amd.com?part=4