Messages in this thread | | | Date | Thu, 11 Apr 2024 08:33:47 -0500 | From | Tom Lendacky <> | Subject | Re: [PATCH v12 29/29] KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event |
| |
On 3/29/24 17:58, Michael Roth wrote: > Version 2 of GHCB specification added support for the SNP Extended Guest > Request Message NAE event. This event serves a nearly identical purpose > to the previously-added SNP_GUEST_REQUEST event, but allows for > additional certificate data to be supplied via an additional > guest-supplied buffer to be used mainly for verifying the signature of > an attestation report as returned by firmware. > > This certificate data is supplied by userspace, so unlike with > SNP_GUEST_REQUEST events, SNP_EXTENDED_GUEST_REQUEST events are first > forwarded to userspace via a KVM_EXIT_VMGEXIT exit type, and then the > firmware request is made only afterward. > > Implement handling for these events. > > Since there is a potential for race conditions where the > userspace-supplied certificate data may be out-of-sync relative to the > reported TCB or VLEK that firmware will use when signing attestation > reports, make use of the synchronization mechanisms wired up to the > SNP_{PAUSE,RESUME}_ATTESTATION SEV device ioctls such that the guest > will be told to retry the request while attestation has been paused due > to an update being underway on the system. > > Signed-off-by: Michael Roth <michael.roth@amd.com> > --- > Documentation/virt/kvm/api.rst | 26 ++++++++++++ > arch/x86/include/asm/sev.h | 4 ++ > arch/x86/kvm/svm/sev.c | 75 ++++++++++++++++++++++++++++++++++ > arch/x86/kvm/svm/svm.h | 3 ++ > arch/x86/virt/svm/sev.c | 21 ++++++++++ > include/uapi/linux/kvm.h | 6 +++ > 6 files changed, 135 insertions(+) >
> +static int snp_complete_ext_guest_req(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + struct vmcb_control_area *control; > + struct kvm *kvm = vcpu->kvm; > + sev_ret_code fw_err = 0; > + int vmm_ret; > + > + vmm_ret = vcpu->run->vmgexit.ext_guest_req.ret; > + if (vmm_ret) { > + if (vmm_ret == SNP_GUEST_VMM_ERR_INVALID_LEN) > + vcpu->arch.regs[VCPU_REGS_RBX] = > + vcpu->run->vmgexit.ext_guest_req.data_npages; > + goto abort_request; > + } > + > + control = &svm->vmcb->control; > + > + if (!__snp_handle_guest_req(kvm, control->exit_info_1, control->exit_info_2, > + &fw_err)) > + vmm_ret = SNP_GUEST_VMM_ERR_GENERIC; > + > + /* > + * Give errors related to stale transactions precedence to provide more > + * potential options for servicing firmware while guests are running. > + */ > + if (snp_transaction_is_stale(svm->snp_transaction_id)) > + vmm_ret = SNP_GUEST_VMM_ERR_BUSY;
I think having this after the call to the SEV firmware will cause an issue. If the firmware has performed the attestation request successfully in the __snp_handle_guest_req() call, then it will have incremented the sequence number. If you return busy, then the sev-guest driver will attempt to re-issue the request with the original sequence number which will now fail. That failure will then be propagated back to the sev-guest driver which will then disable the VMPCK key.
So I think you need to put this before the call to firmware.
Thanks, Tom
> +
| |