public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Tycho Andersen <tycho@kernel.org>
To: Sean Christopherson <seanjc@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>,
	 Ashish Kalra <ashish.kalra@amd.com>,
	John Allen <john.allen@amd.com>,
	 Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	 Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	 Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@kernel.org>,
	 Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	 Shuah Khan <shuah@kernel.org>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Kim Phillips <kim.phillips@amd.com>,
	Alexey Kardashevskiy <aik@amd.com>,
	 Nikunj A Dadhania <nikunj@amd.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Randy Dunlap <rdunlap@infradead.org>,
	Dapeng Mi <dapeng1.mi@linux.intel.com>,
	 Kees Cook <kees@kernel.org>, Marco Elver <elver@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Li RongQing <lirongqing@baidu.com>,
	Eric Biggers <ebiggers@kernel.org>,
	 "Paul E. McKenney" <paulmck@kernel.org>,
	linux-doc@vger.kernel.org, kvm@vger.kernel.org,
	 linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 3/4] KVM: SEV: Add the kvm-amd.rapl_disable module parameter
Date: Tue, 28 Apr 2026 11:09:38 -0600	[thread overview]
Message-ID: <afDnw8WYpr7TqEHS@tycho.pizza> (raw)
In-Reply-To: <afDkcpcQ5vPsjQkO@google.com>

On Tue, Apr 28, 2026 at 09:46:42AM -0700, Sean Christopherson wrote:
> On Tue, Apr 28, 2026, Tom Lendacky wrote:
> > On 4/28/26 10:53, Sean Christopherson wrote:
> > > On Tue, Apr 28, 2026, Tycho Andersen wrote:
> > >> On Mon, Apr 27, 2026 at 02:20:10PM -0700, Sean Christopherson wrote:
> > >>> I'm pretty sure I said this earlier: KVM absolutely should not be able to disable
> > >>> RAPL for the entire system.  That needs to be a power management thing.
> > >>
> > >> You definitely noted "not CCP", I don't think I quite understood what
> > >> that meant though:
> > >> https://lore.kernel.org/all/aZ86BZWi-GLiHvmt@tycho.pizza/
> > >>
> > >> I'm a little worried that putting it in power management will generate
> > >> some weird dependencies, or weakref symbols that can't change things
> > >> if they are loaded independently of kvm_amd or something. But let me
> > >> see what I can come up with.
> > > 
> > > Ugh, and it's not even powerman per se, it's actually a module in perf.  Oof.
> > > 
> > > I 100% agree it'll be tricky, but I also stand by comments that neither the CCP
> > > driver or KVM should be allowed to silently pull the rug out from under the RAPL
> > > module.
> > 
> > Maybe something that can be added to the current sev= kernel command line
> > parameter, e.g. sev=norapl, or such?
> 
> Yeah.  The only question I have is if we expect end users to want to disable RAPL
> at runtime.  If so, then we probably want a sysfs knob or something.
> 
> However, letting RAPL be toggled on/off will introduce some amount of complexity,
> as the kernel would need to negotiate/coordinate with the RAPL perf module and
> with the CPP driver to ensure RAPL stays in the "correct" state.  E.g. if the
> perf module is loaded, then RAPL is effectively pinned "on".  And if SNP has been
> initialized with RAPL_DIS, then RAPL is effectively pinned "off".  Blech.
> 
> > Maybe even with a kernel config option for a default value?
> 
> Probably overkill?
> 
> > On SNP_SHUTDOWN it will be re-enabled if it was disabled.
> 
> Stating the obvious, if we do this, we open the can of worms I described above.

Unfortunately that's how the firmware works and since we do a shutdown
on module unload, if you have ccp=m this is the behavior.

Maybe it makes sense to go the other way: have perf look for a ccp
symbol that's loaded that says whether RAPL is usable or not, and
refuse to allow access to the counters if it is? But it looks like
there are several UAPIs for this (perf, /dev/amd-hsmp-*, sysfs), so
it's not just one place, which is also ugly.

> > >>> KVM then needs to communicate (and enforce?) the policy to
> > >>> userspace.
> > >>
> > >> KVM doesn't need to enforce anything, the SEV firmware will generate a
> > >> launch error for policy violation if it's not supported.
> > >>
> > >> For communicating to userspace if it's not a kvm module parameter, one
> > >> option is to mask it off in sev_get_snp_supported_policy() if it was
> > 
> > Did you mean sev_get_snp_policy_bits() or were you referring to the KVM
> > ioctl() for retrieving them?

I was thinking of the ioctl() for retrieving them, but doing the
masking in sev_get_snp_policy_bits() since it would be able to
remember whether RAPL_DIS was set or not. Of course I merged the two
in my head when typing the sentence :)

> > >> initialized without the support. Then it'll be visible via
> > >> KVM_X86_SNP_POLICY_BITS.
> > > 
> > > Ya, this is what I was envisioning.
> > 
> > It's still a valid policy bit (if supported by the platform), so I don't
> > think masking it off is appropriate.
> 
> But it's not fully supported, no?  I.e. won't the VM fail if it requests RAPL_DIS?
> 
> Ooh, presumably the subtle difference is that on a platform without RAPL_DIS at
> all, the VM will successfully launch and thus could run with RAPL enabled even
> if the VM requested RAPL_DIS?

I haven't tested this, but I would hope what you describe an error. I
think Tom means it's supported by the architecture, it just needs to
be enabled via reconfiguration.

Tycho

  reply	other threads:[~2026-04-28 17:09 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-27 20:48 [PATCH v2 0/4] Allow disabling RAPL during SNP initialization Tycho Andersen
2026-04-27 20:48 ` [PATCH v2 1/4] crypto/ccp: Pass init_args to __sev_snp_init_locked() Tycho Andersen
2026-04-27 20:48 ` [PATCH v2 2/4] crypto/ccp: Support setting RAPL_DIS in SNP_INIT_EX Tycho Andersen
2026-04-27 20:48 ` [PATCH v2 3/4] KVM: SEV: Add the kvm-amd.rapl_disable module parameter Tycho Andersen
2026-04-27 21:20   ` Sean Christopherson
2026-04-28 14:56     ` Tycho Andersen
2026-04-28 15:53       ` Sean Christopherson
2026-04-28 16:27         ` Tom Lendacky
2026-04-28 16:46           ` Sean Christopherson
2026-04-28 17:09             ` Tycho Andersen [this message]
2026-04-28 17:14               ` Sean Christopherson
2026-04-28 18:04             ` Tom Lendacky
2026-04-27 20:48 ` [PATCH v2 4/4] KVM: selftests: Add a smoke test support for RAPL_DIS Tycho Andersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afDnw8WYpr7TqEHS@tycho.pizza \
    --to=tycho@kernel.org \
    --cc=aik@amd.com \
    --cc=akpm@linux-foundation.org \
    --cc=ashish.kalra@amd.com \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=ebiggers@kernel.org \
    --cc=elver@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=hpa@zytor.com \
    --cc=john.allen@amd.com \
    --cc=kees@kernel.org \
    --cc=kim.phillips@amd.com \
    --cc=kuba@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=lirongqing@baidu.com \
    --cc=mingo@redhat.com \
    --cc=nikunj@amd.com \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rdunlap@infradead.org \
    --cc=seanjc@google.com \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=tglx@kernel.org \
    --cc=thomas.lendacky@amd.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox