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

[Tom Lendacky <thomas.lendacky@amd.com>]

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

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