public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Tycho Andersen <tycho@kernel.org>
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 10:14:40 -0700	[thread overview]
Message-ID: <afDrAJPmCIm1HT8l@google.com> (raw)
In-Reply-To: <afDnw8WYpr7TqEHS@tycho.pizza>

On Tue, Apr 28, 2026, Tycho Andersen wrote:
> 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.

Right, but that's just at the hardware level.  The kernel can still leave RAPL
"disabled" at a software level, i.e. can still disallow loading the RAPL perf
module.

> 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?

Yeah, this is what I suggesting.  Or rather, trying to suggest :-)

> 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:14 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
2026-04-28 17:14               ` Sean Christopherson [this message]
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=afDrAJPmCIm1HT8l@google.com \
    --to=seanjc@google.com \
    --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=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=tglx@kernel.org \
    --cc=thomas.lendacky@amd.com \
    --cc=tycho@kernel.org \
    --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