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

[Borislav Petkov <bp@alien8.de>]

On Mon, Sep 14, 2026 at 03:00:04PM -0500, Kalra, Ashish wrote:
> Thanks, Boris. Splitting setup from start and collapsing to a single export makes sense — a couple of constraints from the RMPOPT
> spec shape how it has to be done.
> 
> RMPOPT_BASE can only be written (RMPOPT_EN set) when SYSCFG[SnpEn] and RMP_CFG[SegmentedRmpEn] are both 1; otherwise the access
> #GP(0)s. So the MSR programming can't run from an init‑time initcall — SnpEn is 0 then and it would #GP. The software setup can,
> though, so the split becomes:
> 
>   - an initcall in this file does the software setup — allocate the workqueue and INIT_DELAYED_WORK(), no export;
>   - snp_enable_rmpopt() (the single export) programs RMPOPT_BASE on the primary threads and queues the pass. ccp calls it after it
>   has enabled SNP, and kvm‑amd calls it on teardown.

This programming sure sounds like something we don't need to repeat each time
we enable RMPOPT...

rmpopt_pa_start is practically static so I'd love it if those MSRs are written
once and that's it. The question is, do they keep their value when we disable
SNP?

And I can basically imagine the answer from hw folks: "yeah, yeah, maybe, but
to be on the safe side, you should always write them after having enabled
SNP."

Because if not, I'd be perfectly fine with us setting them on the *first* SNP
init and not touching them again.

IOW, this pseudo:

	if (!rdmsr(RMPOPT_BASE))
		wrmsr(RMPOPT_BASE, ...):

This should probably be in the snp_enable_rmpopt() function anyway as it
should do what we want.

> The same spec text makes that single entry point safe to call repeatedly: RMPOPT_BASE_ADDR is read‑only once RMPOPT_EN is 1 (and
> RMPOPT_EN can't be cleared while SnpEn is 1), so a later call's write is a probably a no‑op rather than a reprogram. If we want
> to avoid even the redundant IPIs, snp_enable_rmpopt() can read RMPOPT_BASE and skip programming when RMPOPT_EN is already set — a
> hardware‑state check instead of an if (rmpopt_wq).

Yap, or that. Sounds ok to me if it works.

> On clearing X86_FEATURE_RMPOPT when the allocation fails: that hits the problem we ran into in earlier revisions — the workqueue
> allocation is at initcall time, after alternatives are patched, where setup_clear_cpu_cap() isn't reliable (static_cpu_has() is
> already baked in), so clearing the cap won't flip rmpopt_capable(). The setup/enable split removes most of the if (rmpopt_wq)
> checks anyway; the only one left is a single guard in snp_enable_rmpopt() for the (rare) allocation‑failure case, which I will
> probably like to keep rather than rely on clearing the feature.

Or, you can introduce that bool rmpopt_enabled and clear it and test it in
rmpopt_capable(). As long as it stays a static var, only visible in this
compilation unit and not exported, that's good enough.

> I'll respin as v15 with the setup/enable split once we settle the feature‑clear question and the RMPOPT_BASE MSR programming
> question (i.e., skipping it if RMPOPT_EN is already set).

Thx.

Btw, you can respin the last two patches only and send them as a reply to that
thread - I have applied the first 3 already so no need to resend them again.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette