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: >>> On Mon, Apr 27, 2026, Tycho Andersen wrote: >>>> From: "Tycho Andersen (AMD)" <[email protected]> >>>> >>>> Add a user-visible way to set the RAPL_DIS bit for SNP init. >>>> >>>> Since setting RAPL_DIS affects the whole system, put the module parameter >>>> in kvm_amd instead of in the CCP driver to hopefully make it more obvious >>>> to admins. >>>> >>>> Signed-off-by: Tycho Andersen (AMD) <[email protected]> >>>> --- >>>> Documentation/admin-guide/kernel-parameters.txt | 5 +++++ >>>> arch/x86/kvm/svm/sev.c | 8 ++++++++ >>>> 2 files changed, 13 insertions(+) >>>> >>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt >>>> b/Documentation/admin-guide/kernel-parameters.txt >>>> index 4d0f545fb3ec..2b50eed8664c 100644 >>>> --- a/Documentation/admin-guide/kernel-parameters.txt >>>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>>> @@ -3207,6 +3207,11 @@ Kernel parameters >>>> max_snp_asid == min_sev_asid-1, will effectively make >>>> SEV-ES unusable. >>>> >>>> + kvm-amd.rapl_disable= [KVM,AMD] Whether to disable RAPL >>>> + (Running Average Power Limit) when initializing the SNP >>>> + firmware. This disables the counters for the entire >>>> system until an >>>> + SNP shutdown command is issued. >>> >>> 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? Maybe even with a kernel config option for a default value? On SNP_SHUTDOWN it will be re-enabled if it was disabled. > >>> 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. Thanks, Tom
