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/[email protected]/ > > > >> > > > >> 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
