From: Nathan Chancellor <nathan@kernel.org>
To: Michael Roth <michael.roth@amd.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Sean Christopherson <seanjc@google.com>,
llvm@lists.linux.dev
Subject: Re: [PULL 18/19] KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event
Date: Mon, 13 May 2024 08:19:20 -0700 [thread overview]
Message-ID: <20240513151920.GA3061950@thelio-3990X> (raw)
In-Reply-To: <20240510211024.556136-19-michael.roth@amd.com>
Hi Michael,
On Fri, May 10, 2024 at 04:10:23PM -0500, 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 structure, and then
> the firmware request is made after the certificate data has been fetched
> from userspace.
>
> 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, a hook is also provided so that userspace can be informed once
> the attestation request is actually completed. See the updates to
> Documentation/ for more details on these aspects.
>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> Message-ID: <20240501085210.2213060-20-michael.roth@amd.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
...
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 00d29d278f6e..398266bef2ca 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
...
> +static int snp_begin_ext_guest_req(struct kvm_vcpu *vcpu)
> +{
> + int vmm_ret = SNP_GUEST_VMM_ERR_GENERIC;
> + struct vcpu_svm *svm = to_svm(vcpu);
> + unsigned long data_npages;
> + sev_ret_code fw_err;
> + gpa_t data_gpa;
> +
> + if (!sev_snp_guest(vcpu->kvm))
> + goto abort_request;
> +
> + data_gpa = vcpu->arch.regs[VCPU_REGS_RAX];
> + data_npages = vcpu->arch.regs[VCPU_REGS_RBX];
> +
> + if (!IS_ALIGNED(data_gpa, PAGE_SIZE))
> + goto abort_request;
> +
> + /*
> + * Grab the certificates from userspace so that can be bundled with
> + * attestation/guest requests.
> + */
> + vcpu->run->exit_reason = KVM_EXIT_VMGEXIT;
> + vcpu->run->vmgexit.type = KVM_USER_VMGEXIT_REQ_CERTS;
> + vcpu->run->vmgexit.req_certs.data_gpa = data_gpa;
> + vcpu->run->vmgexit.req_certs.data_npages = data_npages;
> + vcpu->run->vmgexit.req_certs.flags = 0;
> + vcpu->run->vmgexit.req_certs.status = KVM_USER_VMGEXIT_REQ_CERTS_STATUS_PENDING;
> + vcpu->arch.complete_userspace_io = snp_complete_ext_guest_req;
> +
> + return 0; /* forward request to userspace */
> +
> +abort_request:
> + ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, fw_err));
> + return 1; /* resume guest */
> +}
This patch is now in -next as commit 32fde9e18b3f ("KVM: SEV: Provide
support for SNP_EXTENDED_GUEST_REQUEST NAE event"), where it causes a
clang warning (or hard error when CONFIG_WERROR is enabled):
arch/x86/kvm/svm/sev.c:4078:67: error: variable 'fw_err' is uninitialized when used here [-Werror,-Wuninitialized]
4078 | ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, fw_err));
| ^~~~~~
include/uapi/linux/sev-guest.h:94:24: note: expanded from macro 'SNP_GUEST_ERR'
94 | SNP_GUEST_FW_ERR(fw_err))
| ^~~~~~
include/uapi/linux/sev-guest.h:92:32: note: expanded from macro 'SNP_GUEST_FW_ERR'
92 | #define SNP_GUEST_FW_ERR(x) ((x) & SNP_GUEST_FW_ERR_MASK)
| ^
arch/x86/kvm/svm/sev.c:4051:2: note: variable 'fw_err' is declared here
4051 | sev_ret_code fw_err;
| ^
1 error generated.
Seems legitimate to me. What was the intention here?
Cheers,
Nathan
next parent reply other threads:[~2024-05-13 15:19 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240510211024.556136-1-michael.roth@amd.com>
[not found] ` <20240510211024.556136-19-michael.roth@amd.com>
2024-05-13 15:19 ` Nathan Chancellor [this message]
2024-05-13 16:53 ` [PULL 18/19] KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event Paolo Bonzini
2024-05-13 17:05 ` Michael Roth
2024-05-13 17:20 ` Paolo Bonzini
2024-05-13 21:18 ` Michael Roth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240513151920.GA3061950@thelio-3990X \
--to=nathan@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=michael.roth@amd.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).