From: Sean Christopherson <seanjc@google.com>
To: Peter Gonda <pgonda@google.com>
Cc: kvm@vger.kernel.org, Vitaly Kuznetsov <vkuznets@redhat.com>,
Borislav Petkov <bp@alien8.de>,
Tom Lendacky <thomas.lendacky@amd.com>,
Brijesh Singh <brijesh.singh@amd.com>,
Joerg Roedel <jroedel@suse.de>, Marc Orr <marcorr@google.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH V4] KVM, SEV: Add KVM_EXIT_SYSTEM_EVENT metadata for SEV-ES
Date: Tue, 5 Apr 2022 22:56:55 +0000 [thread overview]
Message-ID: <YkzJN6kKMenpT0mm@google.com> (raw)
In-Reply-To: <20220405183506.2138403-1-pgonda@google.com>
On Tue, Apr 05, 2022, Peter Gonda wrote:
> SEV-ES guests can request termination using the GHCB's MSR protocol. See
> AMD's GHCB spec section '4.1.13 Termination Request'. Currently when a
> guest does this the userspace VMM sees an KVM_EXIT_UNKNOWN (-EVINAL)
s/EVINAL/EINVAL
> return code from KVM_RUN. By adding a KVM_EXIT_SYSTEM_EVENT to kvm_run
> struct the userspace VMM can clearly see the guest has requested a SEV-ES
> termination including the termination reason code set and reason code.
Nit, phrase that last part as a command, nowhere in the changelog is it actually
stated that the patch converts to use KVM_EXIT_SYSTEM_EVENT.
And my personal preference is to lead with the "what", especially when there's
already a fair amount of assumed knowledge, e.g. someone that's familiar with
SEV-ES probably already knows the guest can request termination, or at least won't
be surprised by the news, whereas leading with the SEV-ES and GHCB info is just
going to add to the confusion of someone who's clueless about SEV-ES.
If an SEV-ES guest requests termination, exit to userspace with
KVM_EXIT_SYSTEM_EVENT and a dedicated SEV_TERM type instead of -EINVAL
so that userspace can take appropriate action.
See AMD's GHCB spec section '4.1.13 Termination Request' for more details.
> Signed-off-by: Peter Gonda <pgonda@google.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Joerg Roedel <jroedel@suse.de>
> Cc: Marc Orr <marcorr@google.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> ---
>
> V4
> * Switch to using KVM_SYSTEM_EVENT exit reason.
>
> V3
> * Add Documentation/ update.
> * Updated other KVM_EXIT_SHUTDOWN exits to clear ndata and set reason
> to KVM_SHUTDOWN_REQ.
>
> V2
> * Add KVM_CAP_EXIT_SHUTDOWN_REASON check for KVM_CHECK_EXTENSION.
>
> Tested by making an SEV-ES guest call sev_es_terminate() with hardcoded
> reason code set and reason code and then observing the codes from the
> userspace VMM in the kvm_run.system_event fields.
>
> ---
> arch/x86/kvm/svm/sev.c | 7 +++++--
> include/uapi/linux/kvm.h | 1 +
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 75fa6dd268f0..039b241a9fb5 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2735,8 +2735,11 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
> pr_info("SEV-ES guest requested termination: %#llx:%#llx\n",
> reason_set, reason_code);
>
> - ret = -EINVAL;
> - break;
> + vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
Wrong exit reason.
> + vcpu->run->system_event.type = KVM_SYSTEM_EVENT_SEV_TERM;
> + vcpu->run->system_event.flags = control->ghcb_gpa;
> +
> + return 0;
> }
> default:
> /* Error, keep GHCB MSR value as-is */
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 8616af85dc5d..d9d24db12930 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -444,6 +444,7 @@ struct kvm_run {
> #define KVM_SYSTEM_EVENT_SHUTDOWN 1
> #define KVM_SYSTEM_EVENT_RESET 2
> #define KVM_SYSTEM_EVENT_CRASH 3
> +#define KVM_SYSTEM_EVENT_SEV_TERM 4
> __u32 type;
> __u64 flags;
@type isn't properly padded, so this needs to be changed when using flags. And
we definitely want to grab more room than just a single u64.
Per Paolo and I's combined powers[*], use bit 31 of the type to enumerate that ndata
is valid, and then change the sub-struct to:
struct {
#define KVM_SYSTEM_EVENT_SHUTDOWN 1
#define KVM_SYSTEM_EVENT_RESET 2
#define KVM_SYSTEM_EVENT_CRASH 3
#define KVM_SYSTEM_EVENT_SEV_TERM 4
#define KVM_SYSTEM_EVENT_NDATA_VALID (1u << 31)
__u32 type;
__u32 ndata;
__u64 data[16];
} system_event;
[*] https://lore.kernel.org/all/e0285020-49d9-8168-be4d-90940a30a048@redhat.com
> } system_event;
> --
> 2.35.1.1094.g7c7d902a7c-goog
>
prev parent reply other threads:[~2022-04-06 5:30 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-05 18:35 [PATCH V4] KVM, SEV: Add KVM_EXIT_SYSTEM_EVENT metadata for SEV-ES Peter Gonda
2022-04-05 22:56 ` Sean Christopherson [this message]
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=YkzJN6kKMenpT0mm@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=brijesh.singh@amd.com \
--cc=jroedel@suse.de \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcorr@google.com \
--cc=pgonda@google.com \
--cc=thomas.lendacky@amd.com \
--cc=vkuznets@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.