On 4/28/26 11:46, 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?
Yeah, probably. > >> 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. > >>>>> 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? >> >>>> 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? No, the VM launch will fail in that case, too. But I thought the difference there would be getting an INVALID_PARAM for specifying an unsupported policy bit, but I think you still get POLICY_FAILURE, so maybe it doesn't matter. Thanks, Tom
