[v19,102/130] KVM: TDX: handle EXCEPTION_NMI and EXTERNAL_INTERRUPT

Commit Message

Isaku Yamahata Feb. 26, 2024, 8:26 a.m. UTC

From: Isaku Yamahata <[e-mail]>

Because guest TD state is protected, exceptions in guest TDs can't be
intercepted.  TDX VMM doesn't need to handle exceptions.
tdx_handle_exit_irqoff() handles NMI and machine check.  Ignore NMI and
machine check and continue guest TD execution.

For external interrupt, increment stats same to the VMX case.

Signed-off-by: Isaku Yamahata <[e-mail]>
Reviewed-by: Paolo Bonzini <[e-mail]>
---
 arch/x86/kvm/vmx/tdx.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Chao Gao April 1, 2024, 8:22 a.m. UTC | #1

On Mon, Feb 26, 2024 at 12:26:44AM -0800, [e-mail] wrote:
>From: Isaku Yamahata <[e-mail]>
>
>Because guest TD state is protected, exceptions in guest TDs can't be
>intercepted.  TDX VMM doesn't need to handle exceptions.
>tdx_handle_exit_irqoff() handles NMI and machine check.  Ignore NMI and

tdx_handle_exit_irqoff() doesn't handle NMIs.

>machine check and continue guest TD execution.
>
>For external interrupt, increment stats same to the VMX case.
>
>Signed-off-by: Isaku Yamahata <[e-mail]>
>Reviewed-by: Paolo Bonzini <[e-mail]>
>---
> arch/x86/kvm/vmx/tdx.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
>diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>index 0db80fa020d2..bdd74682b474 100644
>--- a/arch/x86/kvm/vmx/tdx.c
>+++ b/arch/x86/kvm/vmx/tdx.c
>@@ -918,6 +918,25 @@ void tdx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
> 		vmx_handle_exception_irqoff(vcpu, tdexit_intr_info(vcpu));
> }
> 
>+static int tdx_handle_exception(struct kvm_vcpu *vcpu)
>+{
>+	u32 intr_info = tdexit_intr_info(vcpu);
>+
>+	if (is_nmi(intr_info) || is_machine_check(intr_info))
>+		return 1;

Add a comment in code as well.

>+
>+	kvm_pr_unimpl("unexpected exception 0x%x(exit_reason 0x%llx qual 0x%lx)\n",
>+		intr_info,
>+		to_tdx(vcpu)->exit_reason.full, tdexit_exit_qual(vcpu));
>+	return -EFAULT;

-EFAULT looks incorrect.

>+}
>+
>+static int tdx_handle_external_interrupt(struct kvm_vcpu *vcpu)
>+{
>+	++vcpu->stat.irq_exits;
>+	return 1;
>+}
>+
> static int tdx_handle_triple_fault(struct kvm_vcpu *vcpu)
> {
> 	vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
>@@ -1390,6 +1409,10 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
> 	WARN_ON_ONCE(fastpath != EXIT_FASTPATH_NONE);
> 
> 	switch (exit_reason.basic) {
>+	case EXIT_REASON_EXCEPTION_NMI:
>+		return tdx_handle_exception(vcpu);
>+	case EXIT_REASON_EXTERNAL_INTERRUPT:
>+		return tdx_handle_external_interrupt(vcpu);
> 	case EXIT_REASON_EPT_VIOLATION:
> 		return tdx_handle_ept_violation(vcpu);
> 	case EXIT_REASON_EPT_MISCONFIG:
>-- 
>2.25.1
>
>

Isaku Yamahata April 3, 2024, 6:51 p.m. UTC | #2

On Mon, Apr 01, 2024 at 04:22:00PM +0800,
Chao Gao <[e-mail]> wrote:

> On Mon, Feb 26, 2024 at 12:26:44AM -0800, [e-mail] wrote:
> >From: Isaku Yamahata <[e-mail]>
> >
> >Because guest TD state is protected, exceptions in guest TDs can't be
> >intercepted.  TDX VMM doesn't need to handle exceptions.
> >tdx_handle_exit_irqoff() handles NMI and machine check.  Ignore NMI and
> 
> tdx_handle_exit_irqoff() doesn't handle NMIs.

Will it to tdx_handle_exception().


> >machine check and continue guest TD execution.
> >
> >For external interrupt, increment stats same to the VMX case.
> >
> >Signed-off-by: Isaku Yamahata <[e-mail]>
> >Reviewed-by: Paolo Bonzini <[e-mail]>
> >---
> > arch/x86/kvm/vmx/tdx.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> >diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >index 0db80fa020d2..bdd74682b474 100644
> >--- a/arch/x86/kvm/vmx/tdx.c
> >+++ b/arch/x86/kvm/vmx/tdx.c
> >@@ -918,6 +918,25 @@ void tdx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
> > 		vmx_handle_exception_irqoff(vcpu, tdexit_intr_info(vcpu));
> > }
> > 
> >+static int tdx_handle_exception(struct kvm_vcpu *vcpu)
> >+{
> >+	u32 intr_info = tdexit_intr_info(vcpu);
> >+
> >+	if (is_nmi(intr_info) || is_machine_check(intr_info))
> >+		return 1;
> 
> Add a comment in code as well.

Sure.


> >+
> >+	kvm_pr_unimpl("unexpected exception 0x%x(exit_reason 0x%llx qual 0x%lx)\n",
> >+		intr_info,
> >+		to_tdx(vcpu)->exit_reason.full, tdexit_exit_qual(vcpu));
> >+	return -EFAULT;
> 
> -EFAULT looks incorrect.

As this is unexpected exception, we should exit to to the user-space with
KVM_EXIT_EXCEPTION. Then QEMU will abort with message.

Binbin Wu April 17, 2024, 3:05 a.m. UTC | #3

On 4/4/2024 2:51 AM, Isaku Yamahata wrote:
> On Mon, Apr 01, 2024 at 04:22:00PM +0800,
> Chao Gao <[e-mail]> wrote:
>
>> On Mon, Feb 26, 2024 at 12:26:44AM -0800, [e-mail] wrote:
>>> From: Isaku Yamahata <[e-mail]>
>>>
>>> Because guest TD state is protected, exceptions in guest TDs can't be
>>> intercepted.  TDX VMM doesn't need to handle exceptions.
>>> tdx_handle_exit_irqoff() handles NMI and machine check.  Ignore NMI and
>> tdx_handle_exit_irqoff() doesn't handle NMIs.
> Will it to tdx_handle_exception().

I don't get  why tdx_handle_exception()?

NMI is handled in tdx_vcpu_enter_exit() prior to leaving the safety of 
noinstr, according to patch 098.
https://lore.kernel.org/kvm/88920c[e-mail]/

@@ -837,6 +857,12 @@ static noinstr void tdx_vcpu_enter_exit(struct 
vcpu_tdx *tdx)
      WARN_ON_ONCE(!kvm_rebooting &&
               (tdx->exit_reason.full & TDX_SW_ERROR) == TDX_SW_ERROR);

+    if ((u16)tdx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI &&
+        is_nmi(tdexit_intr_info(vcpu))) {
+        kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
+        vmx_do_nmi_irqoff();
+        kvm_after_interrupt(vcpu);
+    }
      guest_state_exit_irqoff();
  }

>
>
>>> machine check and continue guest TD execution.
>>>
>>> For external interrupt, increment stats same to the VMX case.
>>>
>>> Signed-off-by: Isaku Yamahata <[e-mail]>
>>> Reviewed-by: Paolo Bonzini <[e-mail]>
>>> ---
>>> arch/x86/kvm/vmx/tdx.c | 23 +++++++++++++++++++++++
>>> 1 file changed, 23 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>>> index 0db80fa020d2..bdd74682b474 100644
>>> --- a/arch/x86/kvm/vmx/tdx.c
>>> +++ b/arch/x86/kvm/vmx/tdx.c
>>> @@ -918,6 +918,25 @@ void tdx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
>>> 		vmx_handle_exception_irqoff(vcpu, tdexit_intr_info(vcpu));
>>> }
>>>
>>> +static int tdx_handle_exception(struct kvm_vcpu *vcpu)

Should this function be named as tdx_handle_exception_nmi() since it's 
checking nmi as well?

>>> +{
>>> +	u32 intr_info = tdexit_intr_info(vcpu);
>>> +
>>> +	if (is_nmi(intr_info) || is_machine_check(intr_info))
>>> +		return 1;
>>

Isaku Yamahata April 18, 2024, 12:08 a.m. UTC | #4

On Wed, Apr 17, 2024 at 11:05:05AM +0800,
Binbin Wu <[e-mail]> wrote:

> 
> 
> On 4/4/2024 2:51 AM, Isaku Yamahata wrote:
> > On Mon, Apr 01, 2024 at 04:22:00PM +0800,
> > Chao Gao <[e-mail]> wrote:
> > 
> > > On Mon, Feb 26, 2024 at 12:26:44AM -0800, [e-mail] wrote:
> > > > From: Isaku Yamahata <[e-mail]>
> > > > 
> > > > Because guest TD state is protected, exceptions in guest TDs can't be
> > > > intercepted.  TDX VMM doesn't need to handle exceptions.
> > > > tdx_handle_exit_irqoff() handles NMI and machine check.  Ignore NMI and
> > > tdx_handle_exit_irqoff() doesn't handle NMIs.
> > Will it to tdx_handle_exception().
> 
> I don't get  why tdx_handle_exception()?
> 
> NMI is handled in tdx_vcpu_enter_exit() prior to leaving the safety of
> noinstr, according to patch 098.
> https://lore.kernel.org/kvm/88920c[e-mail]/
> 
> @@ -837,6 +857,12 @@ static noinstr void tdx_vcpu_enter_exit(struct vcpu_tdx
> *tdx)
>      WARN_ON_ONCE(!kvm_rebooting &&
>               (tdx->exit_reason.full & TDX_SW_ERROR) == TDX_SW_ERROR);
> 
> +    if ((u16)tdx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI &&
> +        is_nmi(tdexit_intr_info(vcpu))) {
> +        kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
> +        vmx_do_nmi_irqoff();
> +        kvm_after_interrupt(vcpu);
> +    }
>      guest_state_exit_irqoff();
>  }

You're correct. tdx_vcpu_enter_exit() handles EXIT_REASON_EXCEPTION_NMI for NMI,
and tdx_handle_exeption() ignores NMI case.

The commit message should be updated with tdx_vcpu_enter_exit().


> > > > machine check and continue guest TD execution.
> > > > 
> > > > For external interrupt, increment stats same to the VMX case.
> > > > 
> > > > Signed-off-by: Isaku Yamahata <[e-mail]>
> > > > Reviewed-by: Paolo Bonzini <[e-mail]>
> > > > ---
> > > > arch/x86/kvm/vmx/tdx.c | 23 +++++++++++++++++++++++
> > > > 1 file changed, 23 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > > index 0db80fa020d2..bdd74682b474 100644
> > > > --- a/arch/x86/kvm/vmx/tdx.c
> > > > +++ b/arch/x86/kvm/vmx/tdx.c
> > > > @@ -918,6 +918,25 @@ void tdx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
> > > > 		vmx_handle_exception_irqoff(vcpu, tdexit_intr_info(vcpu));
> > > > }
> > > > 
> > > > +static int tdx_handle_exception(struct kvm_vcpu *vcpu)
> 
> Should this function be named as tdx_handle_exception_nmi() since it's
> checking nmi as well?

Ok, tdx_handle_exception_nmi() is more consistent.

Patch

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 0db80fa020d2..bdd74682b474 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -918,6 +918,25 @@  void tdx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
 		vmx_handle_exception_irqoff(vcpu, tdexit_intr_info(vcpu));
 }
 
+static int tdx_handle_exception(struct kvm_vcpu *vcpu)
+{
+	u32 intr_info = tdexit_intr_info(vcpu);
+
+	if (is_nmi(intr_info) || is_machine_check(intr_info))
+		return 1;
+
+	kvm_pr_unimpl("unexpected exception 0x%x(exit_reason 0x%llx qual 0x%lx)\n",
+		intr_info,
+		to_tdx(vcpu)->exit_reason.full, tdexit_exit_qual(vcpu));
+	return -EFAULT;
+}
+
+static int tdx_handle_external_interrupt(struct kvm_vcpu *vcpu)
+{
+	++vcpu->stat.irq_exits;
+	return 1;
+}
+
 static int tdx_handle_triple_fault(struct kvm_vcpu *vcpu)
 {
 	vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
@@ -1390,6 +1409,10 @@  int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
 	WARN_ON_ONCE(fastpath != EXIT_FASTPATH_NONE);
 
 	switch (exit_reason.basic) {
+	case EXIT_REASON_EXCEPTION_NMI:
+		return tdx_handle_exception(vcpu);
+	case EXIT_REASON_EXTERNAL_INTERRUPT:
+		return tdx_handle_external_interrupt(vcpu);
 	case EXIT_REASON_EPT_VIOLATION:
 		return tdx_handle_ept_violation(vcpu);
 	case EXIT_REASON_EPT_MISCONFIG: