Re: [PATCH v16 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] x86/sev: fatal #GP on blind RMPOPT scan across MMIO holes
- [High] x86/sev: soft lockup via IRQ-disabled IPI for cold RMPOPT cache
- [High] x86/sev: fatal #UD race between RMPOPT and firmware SNP shutdown
--

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

x86/sev: Perform RMP optimizations asynchronously

This patch adds support for performing RMP optimizations asynchronously using
a dedicated per-CPU workqueue. It scans physical memory up to 2TB and
executes RMPOPT to skip RMP checks for 1GB regions that do not contain SNP
guest memory.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 25cb486ff61b6..bd97135cacc22 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
[ ... ]
> @@ -557,6 +576,12 @@ int snp_prepare(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>  
> +static void rmpopt_disable(void)
> +{
> +	if (rmpopt_wq)
> +		cancel_delayed_work_sync(&rmpopt_delayed_work);
> +}
> +
>  void snp_shutdown(void)
>  {
>  	u64 syscfg;
> @@ -565,6 +590,8 @@ void snp_shutdown(void)
>  	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
>  		return;
>  
> +	rmpopt_disable();

[Severity: High]
Is it safe to cancel the worker here?

When SNP is disabled, the CCP driver sends SEV_CMD_SNP_SHUTDOWN_EX to the
firmware, which clears the SnpEn hardware state. The driver then calls
snp_shutdown(), which calls cancel_delayed_work_sync() here.

Between the firmware clearing SnpEn and the work being cancelled, the
background worker or an IPI can concurrently execute the RMPOPT instruction.

Executing RMPOPT while SnpEn is clear causes a #UD exception, leading to a
kernel panic.

> +
>  	clear_rmp();
>  	on_each_cpu(mfd_reconfigure, NULL, 1);
>  
[ ... ]
> @@ -580,8 +607,69 @@ EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>  static bool rmpopt_capable(void)
>  {
>  	return cpu_feature_enabled(X86_FEATURE_RMPOPT) &&
> -	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
> +	       cc_platform_has(CC_ATTR_HOST_SEV_SNP) && rmpopt_enabled;
> +}
> +
> +/*
> + * 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);
> +
> +	/* Supported by binutils 2.48+ */
> +	asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
> +		     :: "a" (pa_start), "c" (op)
> +		     : "memory", "cc");
> +}
> +
> +static void rmpopt_scan_range(void *arg)
> +{
> +	u64 pa;
> +
> +	for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
> +		rmpopt(pa);

[Severity: High]
Does this iteration skip non-system RAM such as the PCI MMIO hole (e.g.,
3GB-4GB)?

This iterates over the entire physical address range from min_low_pfn to
max_pfn in 1GB steps. Executing the RMPOPT instruction on addresses not
backed by valid system memory causes a #GP.

Since the rmpopt() inline assembly lacks an exception handler, will this
result in a fatal kernel panic when executing on MMIO holes?

> +}
> +
> +static void do_rmpopt_work(struct work_struct *work)
> +{
> +	/*
> +	 * Warm up the RMPOPT cache on this pinned per-CPU worker with interrupts
> +	 * enabled, 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 IPI cause severe scheduling latency or potential soft lockups?

do_rmpopt_work() calls rmpopt_scan_range() locally to warm up the RMPOPT
cache, then uses on_each_cpu_mask() to run it on all other CPUs via IPI.

Since the RMPOPT table is per-CPU, local warmup on the worker CPU does not
affect remote CPUs. The IPI forces all other CPUs to execute the scan with a
cold cache.

Executing up to 2048 cache-miss hardware instructions inside an IRQ-disabled
IPI handler seems likely to cause severe latency issues.

> +}

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