Re: [PATCH v8 2/7] x86/sev: Initialize RMPOPT configuration MSRs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] MSR_AMD64_RMPOPT_BASE is not restored after suspend/resume, silently disabling the RMPOPT feature and leading to unhandled faults.
- [High] wrmsrq_on_cpu() silently swallows hardware faults, making WARN_ON_ONCE() checks ineffective and masking MSR setup failures.
--

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

x86/sev: Initialize RMPOPT configuration MSRs

The new RMPOPT instruction helps manage per-CPU RMP optimization
structures inside the CPU. Initialize the per-CPU RMPOPT table base to
the starting physical address to enable RMP optimization for up to 2 TB
of system RAM on all CPUs.

> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 8bcdce98f6dce..1b5c18408f0bf 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c

[ ... ]

> @@ -555,6 +564,21 @@ int snp_prepare(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>  
> +static void rmpopt_cleanup(void)
> +{
> +	int cpu;
> +
> +	cpus_read_lock();
> +
> +	for_each_cpu(cpu, &rmpopt_cpumask)
> +		WARN_ON_ONCE(wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0));

[Severity: High]
Does wrmsrq_on_cpu() return an error if the MSR write causes a hardware
fault?

Looking at wrmsrq_on_cpu(), it uses wrmsr() which relies on the exception
table handler to log the error and resume execution, and always returns 0
if the IPI succeeds.

If the MSR write causes a fault, it seems this WARN_ON_ONCE() won't trigger,
masking a potential disablement failure. Should wrmsrq_safe_on_cpu() be used
here instead?

> +
> +	cpus_read_unlock();
> +
> +	cpumask_clear(&rmpopt_cpumask);
> +	rmpopt_pa_start = 0;
> +}
> +

[ ... ]

> +void snp_setup_rmpopt(void)
> +{
> +	u64 rmpopt_base;
> +	int cpu;

[ ... ]

> +	/*
> +	 * Per-CPU RMPOPT tables support at most 2 TB of addressable memory
> +	 * for RMP optimizations. Initialize the per-CPU RMPOPT table base
> +	 * to the starting physical address to enable RMP optimizations for
> +	 * up to 2 TB of system RAM on all CPUs.
> +	 */
> +	for_each_cpu(cpu, &rmpopt_cpumask)
> +		WARN_ON_ONCE(wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base));

[Severity: High]
If a system goes into a sleep state like suspend-to-RAM (S3), CPU MSRs are
typically reset.

Since this MSR is initialized here during boot, how is it restored when the
system wakes up?

Without a power management callback like syscore_ops resume hooks or CPU PM
notifiers, will the RMPOPT feature be silently disabled in hardware? While
a later patch disables CPU hotplug, S3 suspend via freeze_secondary_cpus()
explicitly bypasses the cpu_hotplug_disabled check, making this a reachable
path.

Also, similar to rmpopt_cleanup(), if this MSR setup silently fails because
wrmsrq_on_cpu() swallows hardware faults, could subsequent RMPOPT instructions
cause unhandled faults and lead to a kernel panic?

> +
> +	cpus_read_unlock();
> +}
> +EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");

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