On Thu, May 15, 2025 at 3:40 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Thu, May 15, 2025, Dionna Glaze wrote:
> > The AMD-SP is a precious resource that doesn't have a scheduler other
> > than a mutex lock queue. To avoid customers from causing a DoS, a
> > mem_enc_ioctl command for rate limiting guest requests is added.
> >
> > Recommended values are {.interval_ms = 1000, .burst = 1} or
> > {.interval_ms = 2000, .burst = 2} to average 1 request every second.
> > You may need to allow 2 requests back to back to allow for the guest
> > to query the certificate length in an extended guest request without
> > a pause. The 1 second average is our target for quality of service
> > since empirical tests show that 64 VMs can concurrently request an
> > attestation report with a maximum latency of 1 second. We don't
>
> Who is we?
>
> > anticipate more concurrency than that for a seldom used request for
> > a majority well-behaved set of VMs. The majority point is decided as
> > >64 VMs given the assumed 128 VM count for "extreme load".
> >
> > Cc: Thomas Lendacky <Thomas.Lendacky@xxxxxxx>
> > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Cc: Joerg Roedel <jroedel@xxxxxxx>
> > Cc: Peter Gonda <pgonda@xxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> >
> > Signed-off-by: Dionna Glaze <dionnaglaze@xxxxxxxxxx>
> > ---
> > .../virt/kvm/x86/amd-memory-encryption.rst | 23 +++++++++++++
> > arch/x86/include/uapi/asm/kvm.h | 7 ++++
> > arch/x86/kvm/svm/sev.c | 33 +++++++++++++++++++
> > arch/x86/kvm/svm/svm.h | 3 ++
> > 4 files changed, 66 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> > index 1ddb6a86ce7f..1b5b4fc35aac 100644
> > --- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> > +++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
> > @@ -572,6 +572,29 @@ Returns: 0 on success, -negative on error
> > See SNP_LAUNCH_FINISH in the SEV-SNP specification [snp-fw-abi]_ for further
> > details on the input parameters in ``struct kvm_sev_snp_launch_finish``.
> >
> > +21. KVM_SEV_SNP_SET_REQUEST_THROTTLE_RATE
> > +-----------------------------------------
> > +
> > +The KVM_SEV_SNP_SET_REQUEST_THROTTLE_RATE command is used to set a per-VM rate
> > +limit on responding to requests for AMD-SP to process a guest request.
> > +The AMD-SP is a global resource with limited capacity, so to avoid noisy
> > +neighbor effects, the host may set a request rate for guests.
> > +
> > +Parameters (in): struct kvm_sev_snp_set_request_throttle_rate
> > +
> > +Returns: 0 on success, -negative on error
> > +
> > +::
> > +
> > + struct kvm_sev_snp_set_request_throttle_rate {
> > + __u32 interval_ms;
> > + __u32 burst;
> > + };
> > +
> > +The interval will be translated into jiffies, so if it after transformation
>
> I assume this is a limitation of the __ratelimit() interface?
It is.
>
> > +the interval is 0, the command will return ``-EINVAL``. The ``burst`` value
> > +must be greater than 0.
>
> Ugh, whose terribly idea was a per-VM capability? Oh, mine[*]. *sigh*
>
> Looking at this again, a per-VM capability doesn't change anything. In fact,
> it's far, far worse. At least with a module param there's guaranteed to be some
> amount of ratelimiting. Relying on the VMM to opt-in to ratelimiting its VM if
> userspace is compromised is completely nonsensical.
>
> Unless someone has a better idea, let's just go with a module param.
Thanks for that. Do you want the module param to be in units of KHZ (1
interval / x milliseconds),
and treat 0 as unlimited?
The original burst value of 2 is due to an oddity of an older version
of the kernel that would ratelimit
before handling the certificate buffer length negotiation, so we could
simply have a single module
parameter and set the burst rate to 1 unconditionally.
I'd generally prefer this to go in after Michael Roth's patch that
adds the extended guest request support.
>
> [*] https://lore.kernel.org/all/Y8rEFpbMV58yJIKy@xxxxxxxxxx
>
> > @@ -4015,6 +4042,12 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
> >
> > mutex_lock(&sev->guest_req_mutex);
> >
> > + if (!__ratelimit(&sev->snp_guest_msg_rs)) {
> > + svm_vmgexit_no_action(svm, SNP_GUEST_ERR(SNP_GUEST_VMM_ERR_BUSY, 0));
> > + ret = 1;
> > + goto out_unlock;
>
> Can you (or anyone) explain what a well-behaved guest will do in in response to
> BUSY? And/or explain why KVM injecting an error into the guest is better than
> exiting to userspace.
--
-Dionna Glaze, PhD, CISSP, CCSP (she/her)
