Re: [PATCH v2 3/4] KVM: SEV: Add the kvm-amd.rapl_disable module parameter

[Tom Lendacky]

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)" <tycho@xxxxxxxxxx>
>>>>
>>>> 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) <tycho@xxxxxxxxxx>
>>>> ---
>>>>  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/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? 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