[PATCH v2 0/6] Nested VMX: APIC-v related bug fixing

[PATCH v2 0/6] Nested VMX: APIC-v related bug fixing

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

The following patches fix the issue that fail to boot L2 guest on APIC-v
available machine. The main problem is that with APIC-v, virtual interrupt inject
L1 is totally through APIC-v. But if virtual interrupt is arrived when L2 is running,
L1 will detect interrupt through vmexit with reason external interrupt. If this happens,
we should update RVI/SVI to make APIC-v working accordingly.

Changes from v1:
* rebase on top of Xen.
* Minor adjustment according the comments.

Yang Zhang (6):
  Nested VMX: Introduce interrupt source supporting
  Nested VMX: Force check ISR when L2 is running
  Nested VMX: Add interface to update vPPR
  Nested VMX: Check whether interrupt is blocked by TPR
  Nested VMX: Update APIC-v(RVI/SVI) when vmexit to L1
  Nested VMX: Clear APIC-v control bit in vmcs02

 xen/arch/x86/hvm/irq.c             |    2 +-
 xen/arch/x86/hvm/vlapic.c          |   24 +++++++++++++-----
 xen/arch/x86/hvm/vmx/intr.c        |    9 +++++-
 xen/arch/x86/hvm/vmx/vmx.c         |   14 ++++++----
 xen/arch/x86/hvm/vmx/vvmx.c        |   45 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/vlapic.h   |    3 +-
 xen/include/asm-x86/hvm/vmx/vmx.h  |    2 +-
 xen/include/asm-x86/hvm/vmx/vvmx.h |    1 +
 8 files changed, 82 insertions(+), 18 deletions(-)

[PATCH v2 1/6] Nested VMX: Introduce interrupt source supporting

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

APIC-v only supports to delivery the interrupt from LAPIC. So when retriving
the external interrupt info from vmcs12, we need to know whether it is from
PIC or LAPIC. And only handle the LAPIC case. But currently, there is no hint
to tell the source of external interrupt when injecting an interrupt to L1.
This patch will record the interrupt source when injecting it to L1 and check it
when doing APIC-v vmcs field updating in later patch.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/intr.c        |    4 ++--
 xen/arch/x86/hvm/vmx/vmx.c         |   14 ++++++++------
 xen/include/asm-x86/hvm/vmx/vmx.h  |    2 +-
 xen/include/asm-x86/hvm/vmx/vvmx.h |    1 +
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index e376f3c..cb120f2 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -180,7 +180,7 @@ static int nvmx_intr_intercept(struct vcpu *v, struct hvm_intack intack)
             if ( !(ctrl & PIN_BASED_EXT_INTR_MASK) )
                 return 0;
 
-            vmx_inject_extint(intack.vector);
+            vmx_inject_extint(intack.vector, intack.source);
 
             ctrl = __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, VM_EXIT_CONTROLS);
             if ( ctrl & VM_EXIT_ACK_INTR_ON_EXIT )
@@ -309,7 +309,7 @@ void vmx_intr_assist(void)
     else
     {
         HVMTRACE_2D(INJ_VIRQ, intack.vector, /*fake=*/ 0);
-        vmx_inject_extint(intack.vector);
+        vmx_inject_extint(intack.vector, intack.source);
         pt_intr_post(v, intack);
     }
 
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 8ed7026..011a817 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1208,7 +1208,7 @@ static void vmx_update_guest_efer(struct vcpu *v)
 }
 
 void nvmx_enqueue_n2_exceptions(struct vcpu *v, 
-            unsigned long intr_fields, int error_code)
+            unsigned long intr_fields, int error_code, uint8_t source)
 {
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
 
@@ -1216,6 +1216,7 @@ void nvmx_enqueue_n2_exceptions(struct vcpu *v,
         /* enqueue the exception till the VMCS switch back to L1 */
         nvmx->intr.intr_info = intr_fields;
         nvmx->intr.error_code = error_code;
+        nvmx->intr.source = source;
         vcpu_nestedhvm(v).nv_vmexit_pending = 1;
         return;
     }
@@ -1227,7 +1228,8 @@ void nvmx_enqueue_n2_exceptions(struct vcpu *v,
 
 static int nvmx_vmexit_trap(struct vcpu *v, struct hvm_trap *trap)
 {
-    nvmx_enqueue_n2_exceptions(v, trap->vector, trap->error_code);
+    nvmx_enqueue_n2_exceptions(v, trap->vector, trap->error_code,
+                               hvm_intsrc_none);
     return NESTEDHVM_VMEXIT_DONE;
 }
 
@@ -1258,7 +1260,7 @@ static void __vmx_inject_exception(int trap, int type, int error_code)
         curr->arch.hvm_vmx.vmx_emulate = 1;
 }
 
-void vmx_inject_extint(int trap)
+void vmx_inject_extint(int trap, uint8_t source)
 {
     struct vcpu *v = current;
     u32    pin_based_cntrl;
@@ -1269,7 +1271,7 @@ void vmx_inject_extint(int trap)
         if ( pin_based_cntrl & PIN_BASED_EXT_INTR_MASK ) {
             nvmx_enqueue_n2_exceptions (v, 
                INTR_INFO_VALID_MASK | (X86_EVENTTYPE_EXT_INTR<<8) | trap,
-               HVM_DELIVER_NO_ERROR_CODE);
+               HVM_DELIVER_NO_ERROR_CODE, source);
             return;
         }
     }
@@ -1288,7 +1290,7 @@ void vmx_inject_nmi(void)
         if ( pin_based_cntrl & PIN_BASED_NMI_EXITING ) {
             nvmx_enqueue_n2_exceptions (v, 
                INTR_INFO_VALID_MASK | (X86_EVENTTYPE_NMI<<8) | TRAP_nmi,
-               HVM_DELIVER_NO_ERROR_CODE);
+               HVM_DELIVER_NO_ERROR_CODE, hvm_intsrc_nmi);
             return;
         }
     }
@@ -1356,7 +1358,7 @@ static void vmx_inject_trap(struct hvm_trap *trap)
     {
         nvmx_enqueue_n2_exceptions (curr, 
             INTR_INFO_VALID_MASK | (_trap.type<<8) | _trap.vector,
-            _trap.error_code); 
+            _trap.error_code, hvm_intsrc_none);
         return;
     }
     else
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index c33b9f9..f4d759b 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -448,7 +448,7 @@ static inline int __vmxon(u64 addr)
 
 void vmx_get_segment_register(struct vcpu *, enum x86_segment,
                               struct segment_register *);
-void vmx_inject_extint(int trap);
+void vmx_inject_extint(int trap, uint8_t source);
 void vmx_inject_nmi(void);
 
 int ept_p2m_init(struct p2m_domain *p2m);
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 3874525..be2b5c6 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -36,6 +36,7 @@ struct nestedvmx {
     struct {
         unsigned long intr_info;
         u32           error_code;
+        uint8_t       source;
     } intr;
     struct {
         bool_t   enabled;
-- 
1.7.1

[PATCH v2 2/6] Nested VMX: Force check ISR when L2 is running

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

External interrupt is allowed to notify CPU only when it has higher
priority than current in servicing interrupt. With APIC-v, the priority
comparing is done by hardware and hardware will inject the interrupt to
VCPU when it recognizes an interrupt. Currently, there is no virtual
APIC-v feature available for L1 to use, so when L2 is running, we still need
to compare interrupt priority with ISR in hypervisor instead via hardware.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vlapic.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 7a154f9..f1530fd 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -37,6 +37,7 @@
 #include <asm/hvm/io.h>
 #include <asm/hvm/support.h>
 #include <asm/hvm/vmx/vmx.h>
+#include <asm/hvm/nestedhvm.h>
 #include <public/hvm/ioreq.h>
 #include <public/hvm/params.h>
 
@@ -1037,7 +1038,8 @@ int vlapic_has_pending_irq(struct vcpu *v)
     if ( irr == -1 )
         return -1;
 
-    if ( vlapic_virtual_intr_delivery_enabled() )
+    if ( vlapic_virtual_intr_delivery_enabled() &&
+         !nestedhvm_vcpu_in_guestmode(v) )
         return irr;
 
     isr = vlapic_find_highest_isr(vlapic);
-- 
1.7.1

[PATCH v2 3/6] Nested VMX: Add interface to update vPPR

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

Add vlapic_set_ppr() to allow update vPPR which in virtual apic page.
vPPR updating is required when trying to emulate the APIC-v behavior of
delivery virtual interrupt to guest.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vlapic.c        |    8 ++++++++
 xen/include/asm-x86/hvm/vlapic.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index f1530fd..99c5700 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -168,6 +168,14 @@ static uint32_t vlapic_get_ppr(struct vlapic *vlapic)
     return ppr;
 }
 
+uint32_t vlapic_set_ppr(struct vlapic *vlapic)
+{
+   uint32_t ppr = vlapic_get_ppr(vlapic);
+
+   vlapic_set_reg(vlapic, APIC_PROCPRI, ppr);
+   return ppr;
+}
+
 static int vlapic_match_logical_addr(struct vlapic *vlapic, uint8_t mda)
 {
     int result = 0;
diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index 021a5f2..29ba7d9 100644
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -108,6 +108,7 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
 uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 
 int vlapic_accept_pic_intr(struct vcpu *v);
+uint32_t vlapic_set_ppr(struct vlapic *vlapic);
 
 void vlapic_adjust_i8259_target(struct domain *d);
 
-- 
1.7.1

[PATCH v2 4/6] Nested VMX: Check whether interrupt is blocked by TPR

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

If interrupt is blocked by L1's TPR, L2 should not see it and keep
running. Adding the check before L2 to retrive interrupt.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/intr.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index cb120f2..45942ab 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -165,6 +165,11 @@ static int nvmx_intr_intercept(struct vcpu *v, struct hvm_intack intack)
 {
     u32 ctrl;
 
+    /* If blocked by L1's tpr, then nothing to do. */
+    if ( nestedhvm_vcpu_in_guestmode(v) &&
+         hvm_interrupt_blocked(v, intack) == hvm_intblk_tpr )
+        return 1;
+
     if ( nvmx_intr_blocked(v) != hvm_intblk_none )
     {
         enable_intr_window(v, intack);
-- 
1.7.1

[PATCH v2 5/6] Nested VMX: Update APIC-v(RVI/SVI) when vmexit to L1

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

If enabling APIC-v, all interrupts to L1 are delivered through APIC-v.
But when L2 is running, external interrupt will casue L1 vmexit with
reason external interrupt. Then L1 will pick up the interrupt through
vmcs12. when L1 ack the interrupt, since the APIC-v is enabled when
L1 is running, so APIC-v hardware still will do vEOI updating. The problem
is that the interrupt is delivered not through APIC-v hardware, this means
SVI/RVI/vPPR are not setting, but hardware required them when doing vEOI
updating. The solution is that, when L1 tried to pick up the interrupt
from vmcs12, then hypervisor will help to update the SVI/RVI/vPPR to make
sure the following vEOI updating and vPPR updating corrently.

Also, since interrupt is delivered through vmcs12, so APIC-v hardware will
not cleare vIRR and hypervisor need to clear it before L1 running.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/irq.c           |    2 +-
 xen/arch/x86/hvm/vlapic.c        |   12 ++++++------
 xen/arch/x86/hvm/vmx/vvmx.c      |   33 +++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/vlapic.h |    2 +-
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index 9eae5de..6a6fb68 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -437,7 +437,7 @@ struct hvm_intack hvm_vcpu_ack_pending_irq(
             intack.vector = (uint8_t)vector;
         break;
     case hvm_intsrc_lapic:
-        if ( !vlapic_ack_pending_irq(v, intack.vector) )
+        if ( !vlapic_ack_pending_irq(v, intack.vector, 0) )
             intack = hvm_intack_none;
         break;
     case hvm_intsrc_vector:
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 99c5700..d21a6c9 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1058,15 +1058,15 @@ int vlapic_has_pending_irq(struct vcpu *v)
     return irr;
 }
 
-int vlapic_ack_pending_irq(struct vcpu *v, int vector)
+int vlapic_ack_pending_irq(struct vcpu *v, int vector, int force_ack)
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
 
-    if ( vlapic_virtual_intr_delivery_enabled() )
-        return 1;
-
-    vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
-    vlapic_clear_irr(vector, vlapic);
+    if ( force_ack || !vlapic_virtual_intr_delivery_enabled() )
+    {
+        vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
+        vlapic_clear_irr(vector, vlapic);
+    }
 
     return 1;
 }
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 5dfbc54..c197ad5 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1283,6 +1283,36 @@ static void sync_exception_state(struct vcpu *v)
     }
 }
 
+static void nvmx_update_apicv(struct vcpu *v)
+{
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+    struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
+    unsigned long reason = __get_vvmcs(nvcpu->nv_vvmcx, VM_EXIT_REASON);
+    uint32_t intr_info = __get_vvmcs(nvcpu->nv_vvmcx, VM_EXIT_INTR_INFO);
+
+    if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
+         nvmx->intr.source == hvm_intsrc_lapic &&
+         (intr_info & INTR_INFO_VALID_MASK) )
+    {
+        uint16_t status;
+        uint32_t rvi, ppr;
+        uint32_t vector = intr_info & 0xff;
+        struct vlapic *vlapic = vcpu_vlapic(v);
+        
+        vlapic_ack_pending_irq(v, vector, 1);
+
+        ppr = vlapic_set_ppr(vlapic);
+        WARN_ON( (ppr & 0xf0) != (vector & 0xf0) );
+
+        status = vector << 8;
+        rvi = vlapic_has_pending_irq(v);
+        if ( rvi != -1 )
+            status |= rvi & 0xff;
+
+        __vmwrite(GUEST_INTR_STATUS, status);
+    }
+}
+
 static void virtual_vmexit(struct cpu_user_regs *regs)
 {
     struct vcpu *v = current;
@@ -1328,6 +1358,9 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
     /* updating host cr0 to sync TS bit */
     __vmwrite(HOST_CR0, v->arch.hvm_vmx.host_cr0);
 
+    if ( cpu_has_vmx_virtual_intr_delivery )
+        nvmx_update_apicv(v);
+
     vmreturn(regs, VMSUCCEED);
 }
 
diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index 29ba7d9..6109137 100644
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -96,7 +96,7 @@ bool_t is_vlapic_lvtpc_enabled(struct vlapic *vlapic);
 void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig);
 
 int vlapic_has_pending_irq(struct vcpu *v);
-int vlapic_ack_pending_irq(struct vcpu *v, int vector);
+int vlapic_ack_pending_irq(struct vcpu *v, int vector, int force_ack);
 
 int  vlapic_init(struct vcpu *v);
 void vlapic_destroy(struct vcpu *v);
-- 
1.7.1

[PATCH v2 6/6] Nested VMX: Clear APIC-v control bit in vmcs02

[Yang Zhang]

From: Yang Zhang <yang.z.zhang@Intel.com>

There is no vAPIC-v supporting, so mask APIC-v control bit when
constructing vmcs02.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index c197ad5..2a14ed8 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -613,8 +613,15 @@ void nvmx_update_secondary_exec_control(struct vcpu *v,
     u32 shadow_cntrl;
     struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+    u32 apicv_bit = SECONDARY_EXEC_APIC_REGISTER_VIRT |
+                    SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY;
 
+    host_cntrl &= ~apicv_bit;
     shadow_cntrl = __get_vvmcs(nvcpu->nv_vvmcx, SECONDARY_VM_EXEC_CONTROL);
+
+    /* No vAPIC-v support, so it shouldn't be set in vmcs12. */
+    ASSERT( !(shadow_cntrl & apicv_bit) );
+
     nvmx->ept.enabled = !!(shadow_cntrl & SECONDARY_EXEC_ENABLE_EPT);
     shadow_cntrl |= host_cntrl;
     __vmwrite(SECONDARY_VM_EXEC_CONTROL, shadow_cntrl);
@@ -625,7 +632,12 @@ static void nvmx_update_pin_control(struct vcpu *v, unsigned long host_cntrl)
     u32 shadow_cntrl;
     struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
 
+    host_cntrl &= ~PIN_BASED_POSTED_INTERRUPT;
     shadow_cntrl = __get_vvmcs(nvcpu->nv_vvmcx, PIN_BASED_VM_EXEC_CONTROL);
+
+    /* No vAPIC-v support, so it shouldn't be set in vmcs12. */
+    ASSERT( !(shadow_cntrl & PIN_BASED_POSTED_INTERRUPT) );
+
     shadow_cntrl |= host_cntrl;
     __vmwrite(PIN_BASED_VM_EXEC_CONTROL, shadow_cntrl);
 }
-- 
1.7.1

Re: [PATCH v2 0/6] Nested VMX: APIC-v related bug fixing

[Jan Beulich]

>>> On 16.08.13 at 07:43, Yang Zhang <yang.z.zhang@intel.com> wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> The following patches fix the issue that fail to boot L2 guest on APIC-v
> available machine. The main problem is that with APIC-v, virtual interrupt 
> inject
> L1 is totally through APIC-v. But if virtual interrupt is arrived when L2 is 
> running,
> L1 will detect interrupt through vmexit with reason external interrupt. If 
> this happens,
> we should update RVI/SVI to make APIC-v working accordingly.
> 
> Changes from v1:
> * rebase on top of Xen.
> * Minor adjustment according the comments.

Many of the comments I made on v1 still apply. I'm not going to
repeat them.

> Yang Zhang (6):
>   Nested VMX: Introduce interrupt source supporting
>   Nested VMX: Force check ISR when L2 is running
>   Nested VMX: Add interface to update vPPR
>   Nested VMX: Check whether interrupt is blocked by TPR
>   Nested VMX: Update APIC-v(RVI/SVI) when vmexit to L1
>   Nested VMX: Clear APIC-v control bit in vmcs02

Patches 2, 4, and 6 appear the be standalone - if you can confirm
this, then applying them but not 1, 3, and 5 might be a first step
(provided the necessary ack-s trickle in). And it would also point
out that the series is at least badly ordered (if not - as I continue
to think - badly broken up).

Jan

Re: [PATCH v2 0/6] Nested VMX: APIC-v related bug fixing

[Zhang, Yang Z]

Jan Beulich wrote on 2013-08-16:
>>>> On 16.08.13 at 07:43, Yang Zhang <yang.z.zhang@intel.com> wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> The following patches fix the issue that fail to boot L2 guest on
>> APIC-v available machine. The main problem is that with APIC-v,
>> virtual interrupt inject
>> L1 is totally through APIC-v. But if virtual interrupt is arrived
>> when
>> L2 is running,
>> L1 will detect interrupt through vmexit with reason external
>> interrupt. If this happens, we should update RVI/SVI to make APIC-v
>> working accordingly.
>> 
>> Changes from v1:
>> * rebase on top of Xen.
>> * Minor adjustment according the comments.
> 
> Many of the comments I made on v1 still apply. I'm not going to repeat them.
Sorry, I forget to CC the maintainer. Except this, I don't see any other missing comments. Please point out them if I am missing. Thanks.

> 
>> Yang Zhang (6):
>>   Nested VMX: Introduce interrupt source supporting
>>   Nested VMX: Force check ISR when L2 is running
>>   Nested VMX: Add interface to update vPPR
>>   Nested VMX: Check whether interrupt is blocked by TPR
>>   Nested VMX: Update APIC-v(RVI/SVI) when vmexit to L1
>>   Nested VMX: Clear APIC-v control bit in vmcs02
> 
> Patches 2, 4, and 6 appear the be standalone - if you can confirm
Sure. Please apply them if you can.

> this, then applying them but not 1, 3, and 5 might be a first step
> (provided the necessary ack-s trickle in). And it would also point out
> that the series is at least badly ordered (if not - as I continue to think - badly broken up).
Ok, I will resend the 1, 3 and 5 with the necessary ack from the Nested maintainer.

> 
> Jan


Best regards,
Yang

Re: [PATCH v2 0/6] Nested VMX: APIC-v related bug fixing

[Jan Beulich]

>>> "Zhang, Yang Z" <yang.z.zhang@intel.com> 08/19/13 5:13 AM >>>
>Jan Beulich wrote on 2013-08-16:
>>>>> On 16.08.13 at 07:43, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>> Changes from v1:
>>> * rebase on top of Xen.
>>> * Minor adjustment according the comments.
>> 
>> Many of the comments I made on v1 still apply. I'm not going to repeat them.
>Sorry, I forget to CC the maintainer. Except this, I don't see any other missing comments. Please point out them if I am missing. Thanks.

The main fact being the break up of the patches, with (iirc) 1 and 3 introducing things
not needed until a later (and not even immediately subsequent) patch. As said before -
such break up is desirable when a truly huge patch otherwise becomes unreviewable,
but that doesn't appear to be the case here. In your consideration here you need to -
namely when fixing bugs - include the aspect of backporting: Needing to pull in
prerequisite patches that aren't obviously prerequisites causes extra attention/work.

Jan

Re: [PATCH v2 0/6] Nested VMX: APIC-v related bug fixing

[Zhang, Yang Z]

Jan Beulich wrote on 2013-08-19:
>>>> "Zhang, Yang Z" <yang.z.zhang@intel.com> 08/19/13 5:13 AM >>>
>> Jan Beulich wrote on 2013-08-16:
>>>>>> On 16.08.13 at 07:43, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>>> Changes from v1:
>>>> * rebase on top of Xen.
>>>> * Minor adjustment according the comments.
>>> 
>>> Many of the comments I made on v1 still apply. I'm not going to
>>> repeat
> them.
>> Sorry, I forget to CC the maintainer. Except this, I don't see any
>> other missing
> comments. Please point out them if I am missing. Thanks.
> 
> The main fact being the break up of the patches, with (iirc) 1 and 3
> introducing things not needed until a later (and not even immediately subsequent) patch.
> As said before - such break up is desirable when a truly huge patch
> otherwise becomes unreviewable, but that doesn't appear to be the case
> here. In your consideration here you need to - namely when fixing bugs
> - include the aspect of backporting: Needing to pull in prerequisite
> patches that aren't obviously prerequisites causes extra attention/work.
Ok. I will put 1 and 3 to where they were called.

Best regards,
Yang