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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Declare %rax as clobbered or output in rmpopt() to prevent infinite loops.
- [High] Schedule work in process context; executing on_each_cpu_mask triggers hard lockups.
- [High] Use ALIGN_DOWN for rmpopt_pa_end to prevent out-of-bounds RMP table reads.
--

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

x86/sev: Add support to perform RMP optimizations asynchronously

This commit introduces support for asynchronous RMP checks on 1GB physical
memory regions to optimize performance on hardware supporting RMPOPT. A
dedicated workqueue triggers the optimization process during SNP
initialization and after legacy shutdowns.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 07f178eb76c7e..a4c0fe49b9ec7 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c

[ ... ]

> @@ -595,6 +621,44 @@ static bool rmpopt_capable(void)
>  	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>  }
>  
> +/*
> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this range of
> + * memory does not contain any SNP guest memory.
> + *
> + * @pa is a system physical address; RMPOPT operates on the containing 1GB.
> + */
> +static void rmpopt(u64 pa)
> +{
> +	enum rmpopt_op_type op = RMPOPT_OP_VERIFY_AND_REPORT_STATUS;
> +	u64 pa_start = ALIGN_DOWN(pa, SZ_1G);
> +
> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     :: "a" (pa_start), "c" (op)
> +		     : "memory", "cc");

[Severity: Critical]
Does this inline assembly need to declare %rax as an output or clobber?

Since the RMPOPT hardware instruction returns a status code in %rax, could
GCC assume %rax is unmodified because it is only provided as an input? If
rmpopt() is inlined, GCC might allocate %rax for the induction variable pa
in the calling loop. This would cause the loop step to compute incorrectly,
potentially resulting in an infinite loop during boot.

> +}
> +
> +/* 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)
> +{
> +	/*
> +	 * RMPOPT caches the results of a RMP table scan in reserved processor
> +	 * memory, allowing future invocations to skip such costly operations.
> +	 */
> +	migrate_disable();
> +	rmpopt_scan_range(NULL);
> +	migrate_enable();
> +
> +	on_each_cpu_mask(cpu_primary_thread_mask, rmpopt_scan_range, NULL, true);

[Severity: High]
Could executing on_each_cpu_mask() here with wait=true trigger hard lockups?

This call runs the rmpopt_scan_range() loop synchronously on all primary
threads in an IPI context with hardirqs disabled. Since the loop can
iterate up to 2048 times invoking the slow RMPOPT hardware instruction,
is it possible for remote CPUs with cold caches (like on multi-socket
systems) to stall for seconds inside the IPI handler and trigger an NMI
watchdog panic?

> +}

[ ... ]

> @@ -612,6 +704,23 @@ void snp_setup_rmpopt(void)
>  	 */
>  	for_each_cpu(cpu, cpu_primary_thread_mask)
>  		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> +
> +	rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);

[Severity: High]
Should rmpopt_pa_end be calculated using ALIGN_DOWN() instead of ALIGN()?

If max_pfn is not 1GB aligned, aligning upwards creates a hardware scanning
range that extends into unbacked physical memory. When rmpopt_scan_range()
processes this final 1GB block, could the hardware instruction attempt to
read RMP entries beyond the allocated RMP table and cause a Machine Check
Exception or memory corruption?

> +
> +	/* Limit memory scanning to 2TB of RAM */
> +	if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T) {
> +		pr_info("RMPOPT coverage limited to 2TB; memory above 0x%llx not optimized\n",
> +			rmpopt_pa_start + SZ_2T);
> +		rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
> +	}

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