linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
@ 2022-03-21 15:02 Peter Gonda
  2022-03-21 15:27 ` Paolo Bonzini
  2022-03-21 17:25 ` Marc Orr
  0 siblings, 2 replies; 9+ messages in thread
From: Peter Gonda @ 2022-03-21 15:02 UTC (permalink / raw)
  To: kvm
  Cc: Peter Gonda, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Marc Orr, Sean Christopherson, linux-kernel

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)
return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY 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.

Signed-off-by: Peter Gonda <pgonda@google.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: Sean Christopherson <seanjc@google.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---

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.shutdown.data fields.

---
 arch/x86/kvm/svm/sev.c   |  9 +++++++--
 include/uapi/linux/kvm.h | 12 ++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 75fa6dd268f0..5f9d37dd3f6f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2735,8 +2735,13 @@ 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;
+		vcpu->run->shutdown.reason = KVM_SHUTDOWN_SEV_TERM;
+		vcpu->run->shutdown.ndata = 2;
+		vcpu->run->shutdown.data[0] = reason_set;
+		vcpu->run->shutdown.data[1] = reason_code;
+
+		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..12138b8f290c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -271,6 +271,12 @@ struct kvm_xen_exit {
 #define KVM_EXIT_XEN              34
 #define KVM_EXIT_RISCV_SBI        35
 
+/* For KVM_EXIT_SHUTDOWN */
+/* Standard VM shutdown request. No additional metadata provided. */
+#define KVM_SHUTDOWN_REQ	0
+/* SEV-ES termination request */
+#define KVM_SHUTDOWN_SEV_TERM	1
+
 /* For KVM_EXIT_INTERNAL_ERROR */
 /* Emulate instruction failed. */
 #define KVM_INTERNAL_ERROR_EMULATION	1
@@ -311,6 +317,12 @@ struct kvm_run {
 		struct {
 			__u64 hardware_exit_reason;
 		} hw;
+		/* KVM_EXIT_SHUTDOWN_ENTRY */
+		struct {
+			__u64 reason;
+			__u32 ndata;
+			__u64 data[16];
+		} shutdown;
 		/* KVM_EXIT_FAIL_ENTRY */
 		struct {
 			__u64 hardware_entry_failure_reason;
-- 
2.35.1.894.gb6a874cedc-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 15:02 [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES Peter Gonda
@ 2022-03-21 15:27 ` Paolo Bonzini
  2022-03-21 15:42   ` Peter Gonda
  2022-03-21 17:25 ` Marc Orr
  1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2022-03-21 15:27 UTC (permalink / raw)
  To: Peter Gonda, kvm
  Cc: Borislav Petkov, Tom Lendacky, Brijesh Singh, Joerg Roedel,
	Marc Orr, Sean Christopherson, linux-kernel

On 3/21/22 16:02, 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)
> return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY 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.
> 
> Signed-off-by: Peter Gonda <pgonda@google.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: Sean Christopherson <seanjc@google.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

Looks good, but it has to also add a capability.

> +		/* KVM_EXIT_SHUTDOWN_ENTRY */

Just KVM_EXIT_SHUTDOWN.

Paolo

> +		struct {
> +			__u64 reason;
> +			__u32 ndata;
> +			__u64 data[16];
> +		} shutdown;
>   		/* KVM_EXIT_FAIL_ENTRY */
>   		struct {
>   			__u64 hardware_entry_failure_reason;


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 15:27 ` Paolo Bonzini
@ 2022-03-21 15:42   ` Peter Gonda
  2022-03-21 17:08     ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Gonda @ 2022-03-21 15:42 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Marc Orr, Sean Christopherson, LKML

On Mon, Mar 21, 2022 at 9:27 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 3/21/22 16:02, 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)
> > return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY 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.
> >
> > Signed-off-by: Peter Gonda <pgonda@google.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: Sean Christopherson <seanjc@google.com>
> > Cc: kvm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
>
> Looks good, but it has to also add a capability.

Thanks for the quick review! Just so I understand. I should add
KVM_CAP_SEV_TERM or something, then if that has been enabled do the
new functionality, else keep the old functionality?

>
> > +             /* KVM_EXIT_SHUTDOWN_ENTRY */
>
> Just KVM_EXIT_SHUTDOWN.
>

Will do.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 15:42   ` Peter Gonda
@ 2022-03-21 17:08     ` Paolo Bonzini
  2022-03-21 17:22       ` Peter Gonda
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2022-03-21 17:08 UTC (permalink / raw)
  To: Peter Gonda
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Marc Orr, Sean Christopherson, LKML

On 3/21/22 16:42, Peter Gonda wrote:
> On Mon, Mar 21, 2022 at 9:27 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 3/21/22 16:02, 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)
>>> return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY 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.
>>>
>>> Signed-off-by: Peter Gonda <pgonda@google.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: Sean Christopherson <seanjc@google.com>
>>> Cc: kvm@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>
>> Looks good, but it has to also add a capability.
> 
> Thanks for the quick review! Just so I understand. I should add
> KVM_CAP_SEV_TERM or something, then if that has been enabled do the
> new functionality, else keep the old functionality?

No, much simpler; just something for which KVM_CHECK_EXTENSION returns 
1, so that userspace knows that there is a "shutdown" member to be 
filled by KVM_EXIT_SHUTDOWN.  e.g. KVM_CAP_EXIT_SHUTDOWN_REASON.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 17:08     ` Paolo Bonzini
@ 2022-03-21 17:22       ` Peter Gonda
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Gonda @ 2022-03-21 17:22 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Marc Orr, Sean Christopherson, LKML

On Mon, Mar 21, 2022 at 11:08 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 3/21/22 16:42, Peter Gonda wrote:
> > On Mon, Mar 21, 2022 at 9:27 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 3/21/22 16:02, 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)
> >>> return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY 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.
> >>>
> >>> Signed-off-by: Peter Gonda <pgonda@google.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: Sean Christopherson <seanjc@google.com>
> >>> Cc: kvm@vger.kernel.org
> >>> Cc: linux-kernel@vger.kernel.org
> >>
> >> Looks good, but it has to also add a capability.
> >
> > Thanks for the quick review! Just so I understand. I should add
> > KVM_CAP_SEV_TERM or something, then if that has been enabled do the
> > new functionality, else keep the old functionality?
>
> No, much simpler; just something for which KVM_CHECK_EXTENSION returns
> 1, so that userspace knows that there is a "shutdown" member to be
> filled by KVM_EXIT_SHUTDOWN.  e.g. KVM_CAP_EXIT_SHUTDOWN_REASON.

Makes sense, thanks for help. Will do for V2.

>
> Paolo
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 15:02 [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES Peter Gonda
  2022-03-21 15:27 ` Paolo Bonzini
@ 2022-03-21 17:25 ` Marc Orr
  2022-03-21 18:08   ` Peter Gonda
  1 sibling, 1 reply; 9+ messages in thread
From: Marc Orr @ 2022-03-21 17:25 UTC (permalink / raw)
  To: Peter Gonda
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Sean Christopherson, LKML

On Mon, Mar 21, 2022 at 8:02 AM Peter Gonda <pgonda@google.com> 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)
> return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY 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.
>
> Signed-off-by: Peter Gonda <pgonda@google.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: Sean Christopherson <seanjc@google.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> ---
>
> 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.shutdown.data fields.
>
> ---
>  arch/x86/kvm/svm/sev.c   |  9 +++++++--
>  include/uapi/linux/kvm.h | 12 ++++++++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 75fa6dd268f0..5f9d37dd3f6f 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2735,8 +2735,13 @@ 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;
> +               vcpu->run->shutdown.reason = KVM_SHUTDOWN_SEV_TERM;
> +               vcpu->run->shutdown.ndata = 2;
> +               vcpu->run->shutdown.data[0] = reason_set;
> +               vcpu->run->shutdown.data[1] = reason_code;
> +
> +               return 0;

Maybe I'm missing something, but don't we want to keep returning an error?

rationale: Current behavior: return -EINVAL to userpsace, but
userpsace cannot infer where the -EINVAL came from. After this patch:
We should still return -EINVAL to userspace, but now userspace can
parse this new info in the KVM run struct to properly terminate.

>         }
>         default:
>                 /* Error, keep GHCB MSR value as-is */
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 8616af85dc5d..12138b8f290c 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -271,6 +271,12 @@ struct kvm_xen_exit {
>  #define KVM_EXIT_XEN              34
>  #define KVM_EXIT_RISCV_SBI        35
>
> +/* For KVM_EXIT_SHUTDOWN */
> +/* Standard VM shutdown request. No additional metadata provided. */
> +#define KVM_SHUTDOWN_REQ       0
> +/* SEV-ES termination request */
> +#define KVM_SHUTDOWN_SEV_TERM  1
> +
>  /* For KVM_EXIT_INTERNAL_ERROR */
>  /* Emulate instruction failed. */
>  #define KVM_INTERNAL_ERROR_EMULATION   1
> @@ -311,6 +317,12 @@ struct kvm_run {
>                 struct {
>                         __u64 hardware_exit_reason;
>                 } hw;
> +               /* KVM_EXIT_SHUTDOWN_ENTRY */
> +               struct {
> +                       __u64 reason;
> +                       __u32 ndata;
> +                       __u64 data[16];
> +               } shutdown;
>                 /* KVM_EXIT_FAIL_ENTRY */
>                 struct {
>                         __u64 hardware_entry_failure_reason;
> --
> 2.35.1.894.gb6a874cedc-goog
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 17:25 ` Marc Orr
@ 2022-03-21 18:08   ` Peter Gonda
  2022-03-21 19:45     ` Marc Orr
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Gonda @ 2022-03-21 18:08 UTC (permalink / raw)
  To: Marc Orr
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Sean Christopherson, LKML

> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 75fa6dd268f0..5f9d37dd3f6f 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2735,8 +2735,13 @@ 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;
> > +               vcpu->run->shutdown.reason = KVM_SHUTDOWN_SEV_TERM;
> > +               vcpu->run->shutdown.ndata = 2;
> > +               vcpu->run->shutdown.data[0] = reason_set;
> > +               vcpu->run->shutdown.data[1] = reason_code;
> > +
> > +               return 0;
>
> Maybe I'm missing something, but don't we want to keep returning an error?
>
> rationale: Current behavior: return -EINVAL to userpsace, but
> userpsace cannot infer where the -EINVAL came from. After this patch:
> We should still return -EINVAL to userspace, but now userspace can
> parse this new info in the KVM run struct to properly terminate.
>

I removed the error return code here since an SEV guest may request a
termination due to no fault of the host at all. This is now inline
with any other shutdown requested by the guest. I don't have a strong
preference here but EINVAL doesn't seem correct in all cases, do
others have any thoughts on this?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 18:08   ` Peter Gonda
@ 2022-03-21 19:45     ` Marc Orr
  2022-03-25 15:28       ` Peter Gonda
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Orr @ 2022-03-21 19:45 UTC (permalink / raw)
  To: Peter Gonda
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Sean Christopherson, LKML

On Mon, Mar 21, 2022 at 11:08 AM Peter Gonda <pgonda@google.com> wrote:
>
> > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > index 75fa6dd268f0..5f9d37dd3f6f 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -2735,8 +2735,13 @@ 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;
> > > +               vcpu->run->shutdown.reason = KVM_SHUTDOWN_SEV_TERM;
> > > +               vcpu->run->shutdown.ndata = 2;
> > > +               vcpu->run->shutdown.data[0] = reason_set;
> > > +               vcpu->run->shutdown.data[1] = reason_code;
> > > +
> > > +               return 0;
> >
> > Maybe I'm missing something, but don't we want to keep returning an error?
> >
> > rationale: Current behavior: return -EINVAL to userpsace, but
> > userpsace cannot infer where the -EINVAL came from. After this patch:
> > We should still return -EINVAL to userspace, but now userspace can
> > parse this new info in the KVM run struct to properly terminate.
> >
>
> I removed the error return code here since an SEV guest may request a
> termination due to no fault of the host at all. This is now inline
> with any other shutdown requested by the guest. I don't have a strong
> preference here but EINVAL doesn't seem correct in all cases, do
> others have any thoughts on this?

Makes sense. Yeah, let's see if others have an opinion. Otherwise, I'm
fine either way. Now that you mention it, returning an error to
userspace when the guest triggered the self-termination, and could've
done so for reasons outside the host's control, is a little odd. But
at the same time, it's just as likely that the guest is
self-terminating due to a host-side error. So I guess it's not clear
whether returning an error here is "right" or "wrong".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES
  2022-03-21 19:45     ` Marc Orr
@ 2022-03-25 15:28       ` Peter Gonda
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Gonda @ 2022-03-25 15:28 UTC (permalink / raw)
  To: Marc Orr
  Cc: kvm list, Borislav Petkov, Tom Lendacky, Brijesh Singh,
	Joerg Roedel, Sean Christopherson, LKML

On Mon, Mar 21, 2022 at 1:45 PM Marc Orr <marcorr@google.com> wrote:
>
> On Mon, Mar 21, 2022 at 11:08 AM Peter Gonda <pgonda@google.com> wrote:
> >
> > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > index 75fa6dd268f0..5f9d37dd3f6f 100644
> > > > --- a/arch/x86/kvm/svm/sev.c
> > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > @@ -2735,8 +2735,13 @@ 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;
> > > > +               vcpu->run->shutdown.reason = KVM_SHUTDOWN_SEV_TERM;
> > > > +               vcpu->run->shutdown.ndata = 2;
> > > > +               vcpu->run->shutdown.data[0] = reason_set;
> > > > +               vcpu->run->shutdown.data[1] = reason_code;
> > > > +
> > > > +               return 0;
> > >
> > > Maybe I'm missing something, but don't we want to keep returning an error?
> > >
> > > rationale: Current behavior: return -EINVAL to userpsace, but
> > > userpsace cannot infer where the -EINVAL came from. After this patch:
> > > We should still return -EINVAL to userspace, but now userspace can
> > > parse this new info in the KVM run struct to properly terminate.
> > >
> >
> > I removed the error return code here since an SEV guest may request a
> > termination due to no fault of the host at all. This is now inline
> > with any other shutdown requested by the guest. I don't have a strong
> > preference here but EINVAL doesn't seem correct in all cases, do
> > others have any thoughts on this?
>
> Makes sense. Yeah, let's see if others have an opinion. Otherwise, I'm
> fine either way. Now that you mention it, returning an error to
> userspace when the guest triggered the self-termination, and could've
> done so for reasons outside the host's control, is a little odd. But
> at the same time, it's just as likely that the guest is
> self-terminating due to a host-side error. So I guess it's not clear
> whether returning an error here is "right" or "wrong".

Since no one has expressed a strong opinion have have left this part
of the patch unchanged in the V2. Happy to revise again if people
prefer something else.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-03-25 15:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21 15:02 [PATCH] Add KVM_EXIT_SHUTDOWN metadata for SEV-ES Peter Gonda
2022-03-21 15:27 ` Paolo Bonzini
2022-03-21 15:42   ` Peter Gonda
2022-03-21 17:08     ` Paolo Bonzini
2022-03-21 17:22       ` Peter Gonda
2022-03-21 17:25 ` Marc Orr
2022-03-21 18:08   ` Peter Gonda
2022-03-21 19:45     ` Marc Orr
2022-03-25 15:28       ` Peter Gonda

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 NNTP newsgroup(s).