Re: [PATCH v5 2/5] HV: Don't make NMI injection req when notifying vCPU


Kaige Fu
 

Hi Eddie,

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Eddie Dong
Sent: Monday, December 16, 2019 9:24 AM
To: acrn-dev@...
Cc: Dong, Eddie <eddie.dong@...>
Subject: Re: [acrn-dev] [PATCH v5 2/5] HV: Don't make NMI injection req
when notifying vCPU



-----Original Message-----
From: acrn-dev@... <acrn-dev@...>
On Behalf Of Kaige Fu
Sent: Friday, December 13, 2019 11:42 PM
To: Dong, Eddie <eddie.dong@...>; acrn-dev@...
Subject: [acrn-dev] [PATCH v5 2/5] HV: Don't make NMI injection req
when notifying vCPU

The NMI for notification should not be inject to guest. So, this patch
drops NMI injection request when we use NMI to notify vCPUs.
Meanwhile, ACRN doesn't support vNMI well and there is no
well-designed way to check if the NMI is for notification or for guest
now. So, we take all the NMIs as notificaton NMI for hard rtvm
temporarily. It means that the hard rtvm will never receive NMI with this
patch applied.

We know the vNMI support is not ready yet and it could be hard.
Make it as TODO or to-revisit.
Got it. Will make it as TODO.


TODO: Add vNMI support.

Signed-off-by: Kaige Fu <kaige.fu@...>
---
hypervisor/arch/x86/guest/virq.c | 8 +++++++-
hypervisor/arch/x86/guest/vmexit.c | 9 +++++++--
hypervisor/arch/x86/notify.c | 24 ++++++++++++++++++++++++
hypervisor/include/arch/x86/irq.h | 2 ++
4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/hypervisor/arch/x86/guest/virq.c
b/hypervisor/arch/x86/guest/virq.c
index a0be3c3d..8e4658e4 100644
--- a/hypervisor/arch/x86/guest/virq.c
+++ b/hypervisor/arch/x86/guest/virq.c
@@ -202,7 +202,13 @@ int32_t vcpu_queue_exception(struct acrn_vcpu
*vcpu, uint32_t vector_arg, uint32
} else {
arch->exception_info.error = 0U;
}
- vcpu_make_request(vcpu, ACRN_REQUEST_EXCP);
+
+ if ((vector == IDT_NMI) &&
is_notification_nmi_triggered(vcpu->vm)) {
Didn't catch here. If ACRN inject a vNMI here, it could come from various
reasons, why we need is_notification_nmi_triggered(vcpu->vm)? This is just
one reason.
Here is what I think. As the hypervisor will not own the NMIs, the NMI is either for
notification or for guest. If the NMI is for notification, we drop the vNMI injection request here.
Otherwise, we treat it as vNMI for guest and make the vNMI injection request here.

+ /* This NMI is used as notification signal, we
will not inject
back to the guest */
+ pr_dbg("This NMI is used as notification
signal. So ignore
it.");
Add TODO here.
Will do in next version.

+ } else {
+ vcpu_make_request(vcpu,
ACRN_REQUEST_EXCP);
+ }
}
}

diff --git a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index 43c979f8..23528c2f 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -194,8 +194,13 @@ int32_t vmexit_handler(struct acrn_vcpu *vcpu)
(void)vcpu_queue_exception(vcpu, vector,
err_code);
vcpu->arch.idt_vectoring_info = 0U;
} else if (type == VMX_INT_TYPE_NMI) {
- vcpu_make_request(vcpu,
ACRN_REQUEST_NMI);
- vcpu->arch.idt_vectoring_info = 0U;
+ /* This NMI is used as notification signal, we
will not inject
back to the guest */
+ if (is_notification_nmi_triggered(vcpu->vm)) {
+ pr_dbg("This NMI is used as
notification signal. So
ignore it.");
+ } else {
+ vcpu_make_request(vcpu,
ACRN_REQUEST_NMI);
+ vcpu->arch.idt_vectoring_info = 0U;
+ }
} else {
/* No action on EXT_INT or SW exception. */
}
diff --git a/hypervisor/arch/x86/notify.c
b/hypervisor/arch/x86/notify.c index 6c8aafd0..e8d9bc7e 100644
--- a/hypervisor/arch/x86/notify.c
+++ b/hypervisor/arch/x86/notify.c
@@ -12,6 +12,7 @@
#include <cpu.h>
#include <per_cpu.h>
#include <lapic.h>
+#include <vm.h>

static uint32_t notification_irq = IRQ_INVALID;

@@ -114,3 +115,26 @@ void setup_posted_intr_notification(void)
pr_err("Failed to setup posted-intr notification");
}
}
+
+/**
+ * @brief Check if notification NMI is triggered
+ *
+ * @return true, if the NMI is triggered for notifying vCPU
+ * @return false, if the NMI is triggered for other purpose */ bool
+is_notification_nmi_triggered(struct acrn_vm *vm) {
+ bool ret;
This name is hard to me. Is_hypervisor_nmi or is_notification_nmi?
Will use is_notification_nmi in next version.

+
+ /*
+ * Currently, acrn doesn't support vNMI well. Here, we take all
+ * the NMIs as notificaton NMI for hard rtvm. It means that we
+ * will drop all the NMI injection request to hard rtvm.
+ *
+ * TODO: Add code to check if the NMI is triggered for notification
+ * purpose in order to support vNMI.
+ */
+ ret = is_lapic_pt_configured(vm);
+
+ return ret;
+}
diff --git a/hypervisor/include/arch/x86/irq.h
b/hypervisor/include/arch/x86/irq.h
index cb8a7c61..5673d910 100644
--- a/hypervisor/include/arch/x86/irq.h
+++ b/hypervisor/include/arch/x86/irq.h
@@ -65,6 +65,7 @@
#define IRQF_PT (1U << 2U) /* 1: for passthrough dev */

struct acrn_vcpu;
+struct acrn_vm;

/*
* Definition of the stack frame layout @@ -87,6 +88,7 @@ struct
smp_call_info_data { };

void smp_call_function(uint64_t mask, smp_call_func_t func, void
*data);
+bool is_notification_nmi_triggered(struct acrn_vm *vm);

void init_default_irqs(uint16_t cpu_id);

--
2.20.0



Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.