On Tue, Jan 21, 2025 at 09:19:08AM -0800, Dionna Amalie Glaze wrote:
> On Tue, Jan 21, 2025 at 8:52 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> >
> > On Tue, Jan 21, 2025, Dionna Amalie Glaze wrote:
> > > On Mon, Jan 20, 2025 at 1:58 PM Melody Wang <huibo.wang@xxxxxxx> wrote:
> > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > index 943bd074a5d3..4896c34ed318 100644
> > > > --- a/arch/x86/kvm/svm/sev.c
> > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > @@ -4064,6 +4064,30 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
> > > > return ret;
> > > > }
> > > >
> > > > +static int snp_complete_req_certs(struct kvm_vcpu *vcpu)
> > > > +{
> > > > + struct vcpu_svm *svm = to_svm(vcpu);
> > > > + struct vmcb_control_area *control = &svm->vmcb->control;
> > > > +
> > > > + if (vcpu->run->snp_req_certs.ret) {
> > > > + if (vcpu->run->snp_req_certs.ret == ENOSPC) {
> > > > + vcpu->arch.regs[VCPU_REGS_RBX] = vcpu->run->snp_req_certs.npages;
> > > > + ghcb_set_sw_exit_info_2(svm->sev_es.ghcb,
> > > > + SNP_GUEST_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN, 0));
> > > > + } else if (vcpu->run->snp_req_certs.ret == EAGAIN) {
> > > > + ghcb_set_sw_exit_info_2(svm->sev_es.ghcb,
> > > > + SNP_GUEST_ERR(SNP_GUEST_VMM_ERR_BUSY, 0));
> > >
> > > Discussion, not a change request: given that my proposed patch [1] to
> > > add rate-limiting for guest messages to the PSP generally was
> > > rejected,
> >
> > For the record, it wasn't rejected outright. I pointed out flaws in the proposed
> > behavior[*], and AFAICT no one ever responded. If I fully reject something, I
> > promise I will make it abundantly clear :-)
> >
>
> Okay, well it was a no to the implementation strategy I had chosen and
> did not have bandwidth to change. Your suggestion to exit to userspace
> is more aligned with the topic of discussion now.
>
> > [*] https://lore.kernel.org/all/Y8rEFpbMV58yJIKy@xxxxxxxxxx
> >
> > > do we think it'd be proper to add a KVM_EXIT_SNP_REQ_MSG or
> > > some such for the VMM to decide if the guest should have access to the
> > > globally shared resource (PSP) via EAGAIN or 0?
> >
> > Can you elaborate? I don't quite understand what you're suggesting.
> >
>
> I just mean that instead of only exiting to the VMM on an extended
> guest request and this capability enabled, all guest requests exit to
> the VMM to make the decision to permit access to the device. EAGAIN
> means busy, try again later, and 0 means permit the request. That
> allows for implementation-specific throttling policies to be
> implemented.
Discussed this with Sean on the PUCK call last week and our thinking was
that tying throttling support to a new KVM_EXIT_* type would not be
compatible with existing userspaces unless it is opt-in, and requiring
userspace to opt-in would not be ideal if DoS on the part of guest-owner is
a concern.
So most likely the more generalized exit-to-userspace throttling behavior
would be tied to something pre-existing, like existing EAGAIN/EINTR handling
for the KVM_RUN ioctl or something along that line.
And if the throttling occurs as part of this particular path, we still have
the option of using the SNP_GUEST_VMM_ERR_BUSY response to guest to force a
retry while still giving userspace the immediate_exit-triggered exit it
needs to release the lock and complete things on its end.
So, since throttling support doesn't seem like a blocker for this series, I
went ahead and sent a v5 with some minor documentation updates:
https://lore.kernel.org/kvm/20250219151505.3538323-1-michael.roth@xxxxxxx/
Thanks!
-Mike
>
> > > [1] https://patchwork.kernel.org/project/kvm/cover/20230119213426.379312-1-dionnaglaze@xxxxxxxxxx/
> > >
> > > > + } else {
> > > > + ghcb_set_sw_exit_info_2(svm->sev_es.ghcb,
> > > > + SNP_GUEST_ERR(SNP_GUEST_VMM_ERR_GENERIC, 0));
> > > > + }
> > > > +
> > > > + return 1; /* resume guest */
> > > > + }
> > > > +
> > > > + return snp_handle_guest_req(svm, control->exit_info_1, control->exit_info_2);
> > > > +}
>
>
>
> --
> -Dionna Glaze, PhD, CISSP, CCSP (she/her)
>
