-----Original Message-----
From: Dong, Eddie <eddie.dong@...>
Sent: Monday, December 16, 2019 10:57 AM
To: Fu, Kaige <kaige.fu@...>; acrn-dev@...
Cc: Dong, Eddie <eddie.dong@...>
Subject: RE: [PATCH v5 3/5] HV: Use NMI-window exiting to address req
missing issue

-----Original Message-----
From: Fu, Kaige <kaige.fu@...>
Sent: Friday, December 13, 2019 11:42 PM
To: Dong, Eddie <eddie.dong@...>; acrn-dev@...
Subject: [PATCH v5 3/5] HV: Use NMI-window exiting to address req
missing issue

There is a window where we may miss the current request in the
notification period when the work flow is as the following:

CPUx + + CPUr
| |
| +--+
| | | Handle pending req
| <--+
+--+ |
| | Set req flag |
<--+ |
+------------------>---+
| Send NMI | | Handle NMI
| <--+
| |
| |
| +--> vCPU enter
| |
+ +

So, this patch enables the NMI-window exiting to trigger the next
vmexit once there is no "virtual-NMI blocking" after vCPU enter into VMX

non-root mode.

Then we can process the pending request on time.

Signed-off-by: Kaige Fu <kaige.fu@...>
---
hypervisor/arch/x86/guest/virq.c | 17 ++++++++++++++++
hypervisor/arch/x86/guest/vmcs.c | 2 +-
hypervisor/arch/x86/guest/vmexit.c | 2 +-
hypervisor/arch/x86/irq.c | 31
++++++++++++++++++++++++++++--
hypervisor/include/arch/x86/irq.h | 1 +
5 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/hypervisor/arch/x86/guest/virq.c
b/hypervisor/arch/x86/guest/virq.c
index 8e4658e4..79210717 100644
--- a/hypervisor/arch/x86/guest/virq.c
+++ b/hypervisor/arch/x86/guest/virq.c
@@ -524,3 +524,20 @@ int32_t exception_vmexit_handler(struct

acrn_vcpu

*vcpu)

return status;
}
+
+int nmi_window_vmexit_handler(__unused struct acrn_vcpu *vcpu) {
+ uint32_t value32;
+
+ /*
+ * Disable NMI-window exiting here. We will process
+ * the pending request in acrn_handle_pending_request later
+ */
+ value32 = exec_vmread32(VMX_PROC_VM_EXEC_CONTROLS);
+ value32 &= ~VMX_PROCBASED_CTLS_NMI_WINEXIT;
+ exec_vmwrite32(VMX_PROC_VM_EXEC_CONTROLS, value32);
+
+ vcpu_retain_rip(vcpu);
+
+ return 0;
+}
diff --git a/hypervisor/arch/x86/guest/vmcs.c
b/hypervisor/arch/x86/guest/vmcs.c
index 1efa37db..7dd2d806 100644
--- a/hypervisor/arch/x86/guest/vmcs.c
+++ b/hypervisor/arch/x86/guest/vmcs.c
@@ -582,7 +582,7 @@ void switch_apicv_mode_x2apic(struct acrn_vcpu
*vcpu)
* directly without vmexit. So, here we enable NMI-exiting

and use

NMI
* as notification signal after passthroughing the lapic to vCPU.
*/
- value32 |= VMX_PINBASED_CTLS_NMI_EXIT;
+ value32 |= VMX_PINBASED_CTLS_NMI_EXIT |
VMX_PINBASED_CTLS_VIRT_NMI;
exec_vmwrite32(VMX_PIN_VM_EXEC_CONTROLS, value32);

value32 = exec_vmread32(VMX_EXIT_CONTROLS); diff --git
a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index 23528c2f..0f09741c 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -51,7 +51,7 @@ static const struct vm_exit_dispatch
dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_INTERRUPT_WINDOW] = {
.handler = interrupt_window_vmexit_handler},
[VMX_EXIT_REASON_NMI_WINDOW] = {
- .handler = unhandled_vmexit_handler},
+ .handler = nmi_window_vmexit_handler},
[VMX_EXIT_REASON_TASK_SWITCH] = {
.handler = unhandled_vmexit_handler},
[VMX_EXIT_REASON_CPUID] = {
diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c
index
158165d6..b0ba0351 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -18,6 +18,7 @@
#include <vboot.h>
#include <dump.h>
#include <logmsg.h>
+#include <vmx.h>

static spinlock_t exception_spinlock = { .head = 0U, .tail = 0U, };
static spinlock_t irq_alloc_spinlock = { .head = 0U, .tail = 0U, }; @@
-384,10 +385,36 @@ void dispatch_exception(struct intr_excp_ctx *ctx)

void handle_nmi(__unused struct intr_excp_ctx *ctx) {
+ uint32_t value32;
+
/*
- * Just ignore the NMI here for now.
- * TODO: implement specific NMI handling function.
+ * There is a window where we may miss the current request in this
+ * notification period when the work flow is as the following:
+ *
+ * CPUx + + CPUr
+ * | |
+ * | +--+
+ * | | | Handle pending req
+ * | <--+
+ * +--+ |
+ * | | Set req flag |
+ * <--+ |
+ * +------------------>---+
+ * | Send NMI | | Handle NMI
+ * | <--+
+ * | |
+ * | |
+ * | +--> vCPU enter
+ * | |
+ * + +
+ *
+ * So, here we enable the NMI-window exiting to trigger the next

vmexit

+ * once there is no "virtual-NMI blocking" after vCPU enter into VMX
non-root
+ * mode. Then we can process the pending request on time.
*/
+ value32 = exec_vmread32(VMX_PROC_VM_EXEC_CONTROLS);
+ value32 |= VMX_PROCBASED_CTLS_NMI_WINEXIT;

We didn't inject vNMI yet, why we need to OR
VMX_PROCBASED_CTLS_NMI_WINEXIT?

As I said in the above comment, there is a minor window where we may miss the current request in
this notification period when the work flow is as the following:
1. CPU1 is in root mode already and has processed all the pending request in acrn_handle_pending_request.
2. CPU0 set the request flag and send the NMI to CPU1.
3. CPU1 receive the NMI and process it and then return to vcpu_thread.
4. The CPU1 will then enter VMX non-root mode directly without processing the pending request set by CPU0.

So, here I enable the NMI_WINEXIT to trigger the next vmexit to process this pending request on time.

+ exec_vmwrite32(VMX_PROC_VM_EXEC_CONTROLS, value32);
}

static void init_irq_descs(void)
diff --git a/hypervisor/include/arch/x86/irq.h
b/hypervisor/include/arch/x86/irq.h
index 5673d910..42a1055f 100644
--- a/hypervisor/include/arch/x86/irq.h
+++ b/hypervisor/include/arch/x86/irq.h
@@ -209,6 +209,7 @@ void vcpu_make_request(struct acrn_vcpu *vcpu,
uint16_t eventid);
* @pre vcpu != NULL
*/
int32_t exception_vmexit_handler(struct acrn_vcpu *vcpu);
+int32_t nmi_window_vmexit_handler(struct acrn_vcpu *vcpu);
int32_t interrupt_window_vmexit_handler(struct acrn_vcpu *vcpu);
int32_t external_interrupt_vmexit_handler(struct acrn_vcpu *vcpu);
int32_t acrn_handle_pending_request(struct acrn_vcpu *vcpu);
--
2.20.0