[PATCH 0/4] Add posted interrupt supporting

[PATCH 0/4] Add posted interrupt supporting

[Yang Zhang]

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

The follwoing patches are adding the Posted Interrupt supporting to Xen:
Posted Interrupt allows vAPIC interrupts to inject into guest directly
without any vmexit.

- When delivering a interrupt to guest, if target vcpu is running,
  update Posted-interrupt requests bitmap and send a notification event
  to the vcpu. Then the vcpu will handle this interrupt automatically,
  without any software involvemnt.

- If target vcpu is not running or there already a notification event
  pending in the vcpu, do nothing. The interrupt will be handled by
  next vm entry

Refer to Intel SDM vol3, 29.6  to get more information.

Yang Zhang (4):
  VMX: Detect posted interrupt capability
  VMX: Turn on posted interrupt bit in vmcs
  VMX: Add posted interrupt supporting
  VMX: Use posted interrupt to deliver virutal interrupt

 xen/arch/x86/hvm/vioapic.c                     |    4 +-
 xen/arch/x86/hvm/vlapic.c                      |   19 +++++-
 xen/arch/x86/hvm/vmsi.c                        |    5 +-
 xen/arch/x86/hvm/vmx/vmcs.c                    |   18 +++++-
 xen/arch/x86/hvm/vmx/vmx.c                     |   81 ++++++++++++++++++++++++
 xen/arch/x86/hvm/vmx/vpmu_core2.c              |    5 +-
 xen/arch/x86/hvm/vpt.c                         |   10 ++-
 xen/include/asm-x86/hvm/hvm.h                  |    2 +
 xen/include/asm-x86/hvm/vlapic.h               |    1 +
 xen/include/asm-x86/hvm/vmx/vmcs.h             |   13 ++++
 xen/include/asm-x86/hvm/vmx/vmx.h              |   22 +++++++
 xen/include/asm-x86/mach-default/irq_vectors.h |    3 +-
 12 files changed, 172 insertions(+), 11 deletions(-)

[PATCH 1/4] VMX: Detect posted interrupt capability

[Yang Zhang]

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

Check whether the Hardware supports posted interrupt capability.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/hvm/vmx/vmcs.c        |   12 +++++++++++-
 xen/include/asm-x86/hvm/vmx/vmcs.h |    6 ++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 9926ffb..a88a548 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -92,6 +92,7 @@ static void __init vmx_display_features(void)
     P(cpu_has_vmx_unrestricted_guest, "Unrestricted Guest");
     P(cpu_has_vmx_apic_reg_virt, "APIC Register Virtualization");
     P(cpu_has_vmx_virtual_intr_delivery, "Virtual Interrupt Delivery");
+    P(cpu_has_vmx_posted_intr_processing, "Posted Interrupt Processing");
     P(cpu_has_vmx_vmcs_shadowing, "VMCS shadowing");
 #undef P
 
@@ -143,7 +144,8 @@ static int vmx_init_vmcs_config(void)
 
     min = (PIN_BASED_EXT_INTR_MASK |
            PIN_BASED_NMI_EXITING);
-    opt = PIN_BASED_VIRTUAL_NMIS;
+    opt = (PIN_BASED_VIRTUAL_NMIS |
+            PIN_BASED_POSTED_INTERRUPT);
     _vmx_pin_based_exec_control = adjust_vmx_controls(
         "Pin-Based Exec Control", min, opt,
         MSR_IA32_VMX_PINBASED_CTLS, &mismatch);
@@ -269,6 +271,14 @@ static int vmx_init_vmcs_config(void)
     _vmx_vmexit_control = adjust_vmx_controls(
         "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS, &mismatch);
 
+    /*
+     * "Process posted interrupt" can be set only when "virtual-interrupt
+     * delivery" and "acknowledge interrupt on exit" is set
+     */
+    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
+          || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
+        _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
+
     min = 0;
     opt = VM_ENTRY_LOAD_GUEST_PAT;
     _vmx_vmentry_control = adjust_vmx_controls(
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 37e6734..3a5c91a 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -165,6 +165,7 @@ extern u32 vmx_cpu_based_exec_control;
 #define PIN_BASED_NMI_EXITING           0x00000008
 #define PIN_BASED_VIRTUAL_NMIS          0x00000020
 #define PIN_BASED_PREEMPT_TIMER         0x00000040
+#define PIN_BASED_POSTED_INTERRUPT      0x00000080
 extern u32 vmx_pin_based_exec_control;
 
 #define VM_EXIT_SAVE_DEBUG_CNTRLS       0x00000004
@@ -256,6 +257,8 @@ extern bool_t cpu_has_vmx_ins_outs_instr_info;
     (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
 #define cpu_has_vmx_virtualize_x2apic_mode \
     (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
+#define cpu_has_vmx_posted_intr_processing \
+    (vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT)
 #define cpu_has_vmx_vmcs_shadowing \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VMCS_SHADOWING)
 
@@ -280,6 +283,7 @@ extern bool_t cpu_has_vmx_ins_outs_instr_info;
 /* VMCS field encodings. */
 enum vmcs_field {
     VIRTUAL_PROCESSOR_ID            = 0x00000000,
+    POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002,
     GUEST_ES_SELECTOR               = 0x00000800,
     GUEST_CS_SELECTOR               = 0x00000802,
     GUEST_SS_SELECTOR               = 0x00000804,
@@ -314,6 +318,8 @@ enum vmcs_field {
     VIRTUAL_APIC_PAGE_ADDR_HIGH     = 0x00002013,
     APIC_ACCESS_ADDR                = 0x00002014,
     APIC_ACCESS_ADDR_HIGH           = 0x00002015,
+    PI_DESC_ADDR                    = 0x00002016,
+    PI_DESC_ADDR_HIGH               = 0x00002017,
     EPT_POINTER                     = 0x0000201a,
     EPT_POINTER_HIGH                = 0x0000201b,
     EOI_EXIT_BITMAP0                = 0x0000201c,
-- 
1.7.1

[PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Yang Zhang]

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

Turn on posted interrupt for vcpu if posted interrupt is avaliable.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/hvm/vmx/vmcs.c                    |    6 ++++++
 xen/arch/x86/hvm/vmx/vmx.c                     |   11 +++++++++++
 xen/include/asm-x86/hvm/vmx/vmcs.h             |    7 +++++++
 xen/include/asm-x86/mach-default/irq_vectors.h |    3 ++-
 4 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index a88a548..fb1be8d 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -926,6 +926,12 @@ static int construct_vmcs(struct vcpu *v)
         __vmwrite(GUEST_INTR_STATUS, 0);
     }
 
+    if ( cpu_has_vmx_posted_intr_processing )
+    {
+        __vmwrite(PI_DESC_ADDR, virt_to_maddr(&v->arch.hvm_vmx.pi_desc));
+        __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR, POSTED_INTERRUPT_VECTOR);
+    }
+
     /* Host data selectors. */
     __vmwrite(HOST_SS_SELECTOR, __HYPERVISOR_DS);
     __vmwrite(HOST_DS_SELECTOR, __HYPERVISOR_DS);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index a869ed4..fba9998 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1500,6 +1500,13 @@ static struct hvm_function_table __read_mostly vmx_function_table = {
     .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m,
 };
 
+static void posted_interrupt_handler(struct cpu_user_regs *regs)
+{
+    ack_APIC_irq();
+    perfc_incr(ipis);
+    this_cpu(irq_count)++;
+}
+
 struct hvm_function_table * __init start_vmx(void)
 {
     set_in_cr4(X86_CR4_VMXE);
@@ -1524,6 +1531,10 @@ struct hvm_function_table * __init start_vmx(void)
         setup_ept_dump();
     }
 
+    if ( cpu_has_vmx_posted_intr_processing )
+        set_direct_apic_vector(POSTED_INTERRUPT_VECTOR,
+                posted_interrupt_handler);
+
     setup_vmcs_dump();
 
     return &vmx_function_table;
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 3a5c91a..1825a19 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -73,6 +73,12 @@ struct vmx_domain {
     unsigned long apic_access_mfn;
 };
 
+struct pi_desc {
+    u32 pir[8];
+    u32 control;
+    u32 rsvd[7];
+} __attribute__ ((packed, aligned (64)));
+
 #define ept_get_wl(ept)   ((ept)->ept_wl)
 #define ept_get_asr(ept)  ((ept)->asr)
 #define ept_get_eptp(ept) ((ept)->eptp)
@@ -113,6 +119,7 @@ struct arch_vmx_struct {
 
     uint32_t             eoi_exitmap_changed;
     uint64_t             eoi_exit_bitmap[4];
+    struct pi_desc  	 pi_desc;
 
     unsigned long        host_cr0;
 
diff --git a/xen/include/asm-x86/mach-default/irq_vectors.h b/xen/include/asm-x86/mach-default/irq_vectors.h
index 992e00c..34c2f44 100644
--- a/xen/include/asm-x86/mach-default/irq_vectors.h
+++ b/xen/include/asm-x86/mach-default/irq_vectors.h
@@ -9,12 +9,13 @@
 #define CALL_FUNCTION_VECTOR	0xfb
 #define LOCAL_TIMER_VECTOR	0xfa
 #define PMU_APIC_VECTOR 	0xf9
+#define POSTED_INTERRUPT_VECTOR	0xf8
 /*
  * High-priority dynamically-allocated vectors. For interrupts that
  * must be higher priority than any guest-bound interrupt.
  */
 #define FIRST_HIPRIORITY_VECTOR	0xf1
-#define LAST_HIPRIORITY_VECTOR  0xf8
+#define LAST_HIPRIORITY_VECTOR  0xf7
 /* IRQ0 (timer) is statically allocated but must be high priority. */
 #define IRQ0_VECTOR             0xf0
 
-- 
1.7.1

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Jan Beulich]

>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
> @@ -73,6 +73,12 @@ struct vmx_domain {
>      unsigned long apic_access_mfn;
>  };
>  
> +struct pi_desc {
> +    u32 pir[8];
> +    u32 control;
> +    u32 rsvd[7];
> +} __attribute__ ((packed, aligned (64)));

The "packed" part is pointless here afaict.

> --- a/xen/include/asm-x86/mach-default/irq_vectors.h
> +++ b/xen/include/asm-x86/mach-default/irq_vectors.h
> @@ -9,12 +9,13 @@
>  #define CALL_FUNCTION_VECTOR	0xfb
>  #define LOCAL_TIMER_VECTOR	0xfa
>  #define PMU_APIC_VECTOR 	0xf9
> +#define POSTED_INTERRUPT_VECTOR	0xf8

Is it really necessary to use a static, high priority vector here?

Jan

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Zhang, Yang Z]

Jan Beulich wrote on 2013-04-09:
>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h
>> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
>> @@ -73,6 +73,12 @@ struct vmx_domain {
>>      unsigned long apic_access_mfn;
>>  };
>> +struct pi_desc {
>> +    u32 pir[8];
>> +    u32 control;
>> +    u32 rsvd[7];
>> +} __attribute__ ((packed, aligned (64)));
> 
> The "packed" part is pointless here afaict.
Right. I used bit field to define control and then I found it is not necessary. So I removed the bit field but forget to remove packed.

>> --- a/xen/include/asm-x86/mach-default/irq_vectors.h
>> +++ b/xen/include/asm-x86/mach-default/irq_vectors.h
>> @@ -9,12 +9,13 @@
>>  #define CALL_FUNCTION_VECTOR	0xfb
>>  #define LOCAL_TIMER_VECTOR	0xfa
>>  #define PMU_APIC_VECTOR 	0xf9
>> +#define POSTED_INTERRUPT_VECTOR	0xf8
> 
> Is it really necessary to use a static, high priority vector here?
There is an corner case. During vmenty, cpu will not respond external interrupt. And it is possible that posted interrupt and another interrupt are pending in IRR. We hope posted interrupt have high priority and it can be consumed immediately after vmentry finished. Or else, there may two separate interrupt arrived in hypervisor.

Best regards,
Yang

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Jan Beulich]

>>> On 09.04.13 at 10:30, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Jan Beulich wrote on 2013-04-09:
>>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>> --- a/xen/include/asm-x86/mach-default/irq_vectors.h
>>> +++ b/xen/include/asm-x86/mach-default/irq_vectors.h
>>> @@ -9,12 +9,13 @@
>>>  #define CALL_FUNCTION_VECTOR	0xfb
>>>  #define LOCAL_TIMER_VECTOR	0xfa
>>>  #define PMU_APIC_VECTOR 	0xf9
>>> +#define POSTED_INTERRUPT_VECTOR	0xf8
>> 
>> Is it really necessary to use a static, high priority vector here?
> There is an corner case. During vmenty, cpu will not respond external 
> interrupt. And it is possible that posted interrupt and another interrupt are 
> pending in IRR. We hope posted interrupt have high priority and it can be 
> consumed immediately after vmentry finished. Or else, there may two separate 
> interrupt arrived in hypervisor.

If there is a window, using a static, high priority vector only shrinks
its size. If what you describe is an actual problem (and not just a
latency issue), then it needs to be fixed properly rather just lowering
its likelihood to occur. And if it isn't, I think you ought to demonstrate
that using a static vector indeed provides meaningful benefit over
a dynamically allocated one.

Jan

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Zhang, Yang Z]

Jan Beulich wrote on 2013-04-09:
>>>> On 09.04.13 at 10:30, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> Jan Beulich wrote on 2013-04-09:
>>>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>>> --- a/xen/include/asm-x86/mach-default/irq_vectors.h
>>>> +++ b/xen/include/asm-x86/mach-default/irq_vectors.h
>>>> @@ -9,12 +9,13 @@
>>>>  #define CALL_FUNCTION_VECTOR	0xfb
>>>>  #define LOCAL_TIMER_VECTOR	0xfa
>>>>  #define PMU_APIC_VECTOR 	0xf9
>>>> +#define POSTED_INTERRUPT_VECTOR	0xf8
>>> 
>>> Is it really necessary to use a static, high priority vector here?
>> There is an corner case. During vmenty, cpu will not respond external
>> interrupt. And it is possible that posted interrupt and another
>> interrupt are pending in IRR. We hope posted interrupt have high
>> priority and it can be consumed immediately after vmentry finished. Or
>> else, there may two separate interrupt arrived in hypervisor.
> 
> If there is a window, using a static, high priority vector only shrinks
> its size. If what you describe is an actual problem (and not just a
> latency issue), then it needs to be fixed properly rather just lowering
It is not a problem, it is really a latency issue. We expect to eliminate unnecessary interrupt as possible as we can.

> its likelihood to occur. And if it isn't, I think you ought to demonstrate
> that using a static vector indeed provides meaningful benefit over
> a dynamically allocated one.
It do happen. But hard to catch.

Also, posted interrupt have the same meaning with EVENT_CHECK_VECTOR if hypervisor received it. I think it is reasonable to use similar vector as event_check_vector did.

Best regards,
Yang

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Jan Beulich]

>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
> +static void posted_interrupt_handler(struct cpu_user_regs *regs)
> +{
> +    ack_APIC_irq();
> +    perfc_incr(ipis);
> +    this_cpu(irq_count)++;
> +}

If I'm not mistaken, there's no further code being added to this
interrupt handler in the subsequent patches. What's the point
for the separate interrupt then? I.e. is there any reason not to
reuse event_check_interrupt() and EVENT_CHECK_VECTOR?

Jan

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Keir Fraser]

On 09/04/2013 09:23, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>> +static void posted_interrupt_handler(struct cpu_user_regs *regs)
>> +{
>> +    ack_APIC_irq();
>> +    perfc_incr(ipis);
>> +    this_cpu(irq_count)++;
>> +}
> 
> If I'm not mistaken, there's no further code being added to this
> interrupt handler in the subsequent patches. What's the point
> for the separate interrupt then? I.e. is there any reason not to
> reuse event_check_interrupt() and EVENT_CHECK_VECTOR?

The new POSTED_INTERRUPT_VECTOR has special behaviour when delivered in vmx
non-root mode, so it has to be distinct from the EVENT_CHECK_VECTOR. Still,
it could share event_check_interrupt() -- there is no need at all for
posted_interrupt_handler() to exist.

 -- Keir

> Jan
> 

Re: [PATCH 2/4] VMX: Turn on posted interrupt bit in vmcs

[Zhang, Yang Z]

Keir Fraser wrote on 2013-04-09:
> On 09/04/2013 09:23, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>> +static void posted_interrupt_handler(struct cpu_user_regs *regs)
>>> +{
>>> +    ack_APIC_irq();
>>> +    perfc_incr(ipis);
>>> +    this_cpu(irq_count)++;
>>> +}
>> 
>> If I'm not mistaken, there's no further code being added to this
>> interrupt handler in the subsequent patches. What's the point
>> for the separate interrupt then? I.e. is there any reason not to
>> reuse event_check_interrupt() and EVENT_CHECK_VECTOR?
> 
> The new POSTED_INTERRUPT_VECTOR has special behaviour when delivered in
> vmx non-root mode, so it has to be distinct from the EVENT_CHECK_VECTOR.
> Still, it could share event_check_interrupt() -- there is no need at all
> for posted_interrupt_handler() to exist.
Yes, we can share event_check_interrupt() but cannot reuse EVENT_CHECK_VECTOR since they have different meanings.

Best regards,
Yang

[PATCH 3/4] VMX: Add posted interrupt supporting

[Yang Zhang]

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

Add the supporting of using posted interrupt to deliver interrupt.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/hvm/vlapic.c         |    6 +++
 xen/arch/x86/hvm/vmx/vmx.c        |   70 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/hvm.h     |    2 +
 xen/include/asm-x86/hvm/vlapic.h  |    1 +
 xen/include/asm-x86/hvm/vmx/vmx.h |   22 +++++++++++
 5 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 4b25cc8..128745c 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1050,6 +1050,12 @@ int vlapic_ack_pending_irq(struct vcpu *v, int vector)
     return 1;
 }
 
+void vlapic_set_tmr(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
+{
+    if ( trig )
+        vlapic_set_vector(vec, &vlapic->regs->data[APIC_TMR]);
+}
+
 bool_t is_vlapic_lvtpc_enabled(struct vlapic *vlapic)
 {
     return (vlapic_enabled(vlapic) &&
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index fba9998..85da8cb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -55,6 +55,7 @@
 #include <asm/debugger.h>
 #include <asm/apic.h>
 #include <asm/hvm/nestedhvm.h>
+#include <asm/event.h>
 
 enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
 
@@ -1447,6 +1448,68 @@ static void vmx_process_isr(int isr, struct vcpu *v)
     vmx_vmcs_exit(v);
 }
 
+static void __vmx_deliver_posted_interrupt(struct vcpu *v)
+{
+    bool_t running;
+
+    running = v->is_running;
+    vcpu_unblock(v);
+    if ( running && (in_irq() || (v != current)) )
+    {
+        unsigned int cpu = v->processor;
+
+        if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu))
+                && (cpu != smp_processor_id()) )
+            send_IPI_mask(cpumask_of(cpu), POSTED_INTERRUPT_VECTOR);
+    }
+}
+
+static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector, u8 trig_mode)
+{
+    vlapic_set_tmr(vcpu_vlapic(v), vector, trig_mode);
+
+    vmx_update_eoi_exit_bitmap(v, vector, trig_mode);
+
+    if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
+        return;
+
+    if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) )
+    {
+        /* If EOI exitbitmap needs to changed or notification vector
+         * can't be allocated, interrupt will not be injected till
+         * VMEntry as it used to be
+         */
+        pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);
+        goto out;
+    }
+
+    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
+    {
+        __vmx_deliver_posted_interrupt(v);
+        return;
+    }
+
+out:
+    vcpu_kick(v);
+}
+
+static void vmx_sync_pir_to_irr(struct vcpu *v)
+{
+    struct vlapic *vlapic = vcpu_vlapic(v);
+    u32 val;
+    int offset, group;
+
+    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
+        return;
+
+    for (group = 0; group < 8; group++ )
+    {
+        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
+        offset = APIC_IRR + 0x10 * group;
+        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;
+    }
+}
+
 static struct hvm_function_table __read_mostly vmx_function_table = {
     .name                 = "VMX",
     .cpu_up_prepare       = vmx_cpu_up_prepare,
@@ -1497,6 +1560,8 @@ static struct hvm_function_table __read_mostly vmx_function_table = {
     .update_eoi_exit_bitmap = vmx_update_eoi_exit_bitmap,
     .virtual_intr_delivery_enabled = vmx_virtual_intr_delivery_enabled,
     .process_isr          = vmx_process_isr,
+    .deliver_posted_intr  = vmx_deliver_posted_intr,
+    .sync_pir_to_irr      = vmx_sync_pir_to_irr,
     .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m,
 };
 
@@ -1534,6 +1599,11 @@ struct hvm_function_table * __init start_vmx(void)
     if ( cpu_has_vmx_posted_intr_processing )
         set_direct_apic_vector(POSTED_INTERRUPT_VECTOR,
                 posted_interrupt_handler);
+    else
+    {
+        hvm_funcs.deliver_posted_intr = NULL;
+        hvm_funcs.sync_pir_to_irr = NULL;
+    }
 
     setup_vmcs_dump();
 
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 2fa2ea5..493cffa 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -184,6 +184,8 @@ struct hvm_function_table {
     void (*update_eoi_exit_bitmap)(struct vcpu *v, u8 vector, u8 trig);
     int (*virtual_intr_delivery_enabled)(void);
     void (*process_isr)(int isr, struct vcpu *v);
+    void (*deliver_posted_intr)(struct vcpu *v, u8 vector, u8 trig_mode);
+    void (*sync_pir_to_irr)(struct vcpu *v);
 
     /*Walk nested p2m  */
     int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa,
diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index 101ef57..b212134 100644
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -104,6 +104,7 @@ void vlapic_handle_EOI_induced_exit(struct vlapic *vlapic, int vector);
 void vlapic_ipi(struct vlapic *vlapic, uint32_t icr_low, uint32_t icr_high);
 
 int vlapic_apicv_write(struct vcpu *v, unsigned int offset);
+void vlapic_set_tmr(struct vlapic *vlapic, uint8_t vec, uint8_t trig);
 
 struct vlapic *vlapic_lowest_prio(
     struct domain *d, struct vlapic *source,
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index d4d6feb..ed814ce 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -99,6 +99,28 @@ void vmx_update_exception_bitmap(struct vcpu *v);
 void vmx_update_cpu_exec_control(struct vcpu *v);
 void vmx_update_secondary_exec_control(struct vcpu *v);
 
+#define POSTED_INTR_ON  0
+static inline int pi_test_and_set_pir(int vector, struct pi_desc *pi_desc)
+{
+    return test_and_set_bit(vector, (unsigned long *)pi_desc->pir);
+}
+
+static inline int pi_test_and_set_on(struct pi_desc *pi_desc)
+{
+    return test_and_set_bit(POSTED_INTR_ON,
+            (unsigned long *)&pi_desc->control);
+}
+
+static inline int pi_test_and_clear_on(struct pi_desc *pi_desc)
+{
+    return test_and_clear_bit(POSTED_INTR_ON,
+            (unsigned long *)&pi_desc->control);
+}
+
+static inline u32 pi_get_pir(struct pi_desc *pi_desc, int group)
+{
+    return xchg(&pi_desc->pir[group], 0);
+}
 
 /*
  * Exit Reasons
-- 
1.7.1

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Keir Fraser]

On 09/04/2013 07:01, "Yang Zhang" <yang.z.zhang@intel.com> wrote:

> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Add the supporting of using posted interrupt to deliver interrupt.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
> ---

...

> +static void __vmx_deliver_posted_interrupt(struct vcpu *v)
> +{
> +    bool_t running;
> +
> +    running = v->is_running;
> +    vcpu_unblock(v);
> +    if ( running && (in_irq() || (v != current)) )
> +    {
> +        unsigned int cpu = v->processor;
> +
> +        if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu))
> +                && (cpu != smp_processor_id()) )
> +            send_IPI_mask(cpumask_of(cpu), POSTED_INTERRUPT_VECTOR);

I don't think you need to tickle VCPU_KICK_SOFTIRQ here? You aren't
synchronising with vmx_intr_assist() here, only notifying the processor
itself of pending virtual interrupts. All that requires is an IPI of
POSTED_INTERRUPT_VECTOR in some cases.

I suggest:
    if ( running )
    {
        unsigned int cpu = v->processor;
        if ( cpu != smp_processor_id() )
            send_IPI_mask(...);
    }

That would just work, right? And avoids needing to duplicate some of the
trickier logic in vcpu_kick().

A code comment to say that this is a simplified form of vcpu_kick() is
probably worthwhile too. There are useful code comments in vcpu_kick()
regarding why things are ordered as they are.

 -- Keir


> +    }
> +}
> +
> +static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector, u8 trig_mode)
> +{
> +    vlapic_set_tmr(vcpu_vlapic(v), vector, trig_mode);
> +
> +    vmx_update_eoi_exit_bitmap(v, vector, trig_mode);
> +
> +    if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
> +        return;
> +
> +    if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) )
> +    {
> +        /* If EOI exitbitmap needs to changed or notification vector
> +         * can't be allocated, interrupt will not be injected till
> +         * VMEntry as it used to be
> +         */
> +        pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);
> +        goto out;
> +    }
> +
> +    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
> +    {
> +        __vmx_deliver_posted_interrupt(v);
> +        return;
> +    }
> +
> +out:
> +    vcpu_kick(v);
> +}
> +
> +static void vmx_sync_pir_to_irr(struct vcpu *v)
> +{
> +    struct vlapic *vlapic = vcpu_vlapic(v);
> +    u32 val;
> +    int offset, group;
> +
> +    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
> +        return;
> +
> +    for (group = 0; group < 8; group++ )
> +    {
> +        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
> +        offset = APIC_IRR + 0x10 * group;
> +        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;
> +    }
> +}
> +
>  static struct hvm_function_table __read_mostly vmx_function_table = {
>      .name                 = "VMX",
>      .cpu_up_prepare       = vmx_cpu_up_prepare,
> @@ -1497,6 +1560,8 @@ static struct hvm_function_table __read_mostly
> vmx_function_table = {
>      .update_eoi_exit_bitmap = vmx_update_eoi_exit_bitmap,
>      .virtual_intr_delivery_enabled = vmx_virtual_intr_delivery_enabled,
>      .process_isr          = vmx_process_isr,
> +    .deliver_posted_intr  = vmx_deliver_posted_intr,
> +    .sync_pir_to_irr      = vmx_sync_pir_to_irr,
>      .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m,
>  };
>  
> @@ -1534,6 +1599,11 @@ struct hvm_function_table * __init start_vmx(void)
>      if ( cpu_has_vmx_posted_intr_processing )
>          set_direct_apic_vector(POSTED_INTERRUPT_VECTOR,
>                  posted_interrupt_handler);
> +    else
> +    {
> +        hvm_funcs.deliver_posted_intr = NULL;
> +        hvm_funcs.sync_pir_to_irr = NULL;
> +    }
>  
>      setup_vmcs_dump();
>  
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 2fa2ea5..493cffa 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -184,6 +184,8 @@ struct hvm_function_table {
>      void (*update_eoi_exit_bitmap)(struct vcpu *v, u8 vector, u8 trig);
>      int (*virtual_intr_delivery_enabled)(void);
>      void (*process_isr)(int isr, struct vcpu *v);
> +    void (*deliver_posted_intr)(struct vcpu *v, u8 vector, u8 trig_mode);
> +    void (*sync_pir_to_irr)(struct vcpu *v);
>  
>      /*Walk nested p2m  */
>      int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa,
> diff --git a/xen/include/asm-x86/hvm/vlapic.h
> b/xen/include/asm-x86/hvm/vlapic.h
> index 101ef57..b212134 100644
> --- a/xen/include/asm-x86/hvm/vlapic.h
> +++ b/xen/include/asm-x86/hvm/vlapic.h
> @@ -104,6 +104,7 @@ void vlapic_handle_EOI_induced_exit(struct vlapic *vlapic,
> int vector);
>  void vlapic_ipi(struct vlapic *vlapic, uint32_t icr_low, uint32_t icr_high);
>  
>  int vlapic_apicv_write(struct vcpu *v, unsigned int offset);
> +void vlapic_set_tmr(struct vlapic *vlapic, uint8_t vec, uint8_t trig);
>  
>  struct vlapic *vlapic_lowest_prio(
>      struct domain *d, struct vlapic *source,
> diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h
> b/xen/include/asm-x86/hvm/vmx/vmx.h
> index d4d6feb..ed814ce 100644
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -99,6 +99,28 @@ void vmx_update_exception_bitmap(struct vcpu *v);
>  void vmx_update_cpu_exec_control(struct vcpu *v);
>  void vmx_update_secondary_exec_control(struct vcpu *v);
>  
> +#define POSTED_INTR_ON  0
> +static inline int pi_test_and_set_pir(int vector, struct pi_desc *pi_desc)
> +{
> +    return test_and_set_bit(vector, (unsigned long *)pi_desc->pir);
> +}
> +
> +static inline int pi_test_and_set_on(struct pi_desc *pi_desc)
> +{
> +    return test_and_set_bit(POSTED_INTR_ON,
> +            (unsigned long *)&pi_desc->control);
> +}
> +
> +static inline int pi_test_and_clear_on(struct pi_desc *pi_desc)
> +{
> +    return test_and_clear_bit(POSTED_INTR_ON,
> +            (unsigned long *)&pi_desc->control);
> +}
> +
> +static inline u32 pi_get_pir(struct pi_desc *pi_desc, int group)
> +{
> +    return xchg(&pi_desc->pir[group], 0);
> +}
>  
>  /*
>   * Exit Reasons

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Zhang, Yang Z]

Keir Fraser wrote on 2013-04-09:
> On 09/04/2013 07:01, "Yang Zhang" <yang.z.zhang@intel.com> wrote:
> 
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Add the supporting of using posted interrupt to deliver interrupt.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
>> ---
> 
> ...
> 
>> +static void __vmx_deliver_posted_interrupt(struct vcpu *v)
>> +{
>> +    bool_t running;
>> +
>> +    running = v->is_running;
>> +    vcpu_unblock(v);
>> +    if ( running && (in_irq() || (v != current)) )
>> +    {
>> +        unsigned int cpu = v->processor;
>> +
>> +        if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu))
>> +                && (cpu != smp_processor_id()) )
>> +            send_IPI_mask(cpumask_of(cpu),
> POSTED_INTERRUPT_VECTOR);
> 
> I don't think you need to tickle VCPU_KICK_SOFTIRQ here? You aren't
> synchronising with vmx_intr_assist() here, only notifying the processor
> itself of pending virtual interrupts. All that requires is an IPI of
> POSTED_INTERRUPT_VECTOR in some cases.
> 
> I suggest:
>     if ( running )
>     {
>         unsigned int cpu = v->processor;
>         if ( cpu != smp_processor_id() )
>             send_IPI_mask(...);
>     }
> That would just work, right? And avoids needing to duplicate some of the
> trickier logic in vcpu_kick().
If posted interrupt arrived after vmx_intr_assit() and before vmentry, then the interrupt cannot be inject to guest in time.
vmx_asm_do_vmentry:
        call vmx_intr_assist
        call nvmx_switch_guest
        ASSERT_NOT_IN_ATOMIC
                              <---------if posted interrupt arrived here and we don't set softirq, then the interrupt will be delay until next vmentry.
        GET_CURRENT(%rbx)
        cli

        mov  VCPU_processor(%rbx),%eax
        shl  $IRQSTAT_shift,%eax
        lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
        cmpl $0,(%rdx,%rax,1)
        jnz  .Lvmx_process_softirqs

Actually, the posted interrupt is same with a normal vcpu kick IPI, except that if cpu received it in non-root mode, it will not cause vmext. So we must follow the vcpu_kick logic.

> 
> A code comment to say that this is a simplified form of vcpu_kick() is
> probably worthwhile too. There are useful code comments in vcpu_kick()
> regarding why things are ordered as they are.


Best regards,
Yang

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Keir Fraser]

On 09/04/2013 08:58, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:

> If posted interrupt arrived after vmx_intr_assit() and before vmentry, then
> the interrupt cannot be inject to guest in time.

But pending virtual interrupts are picked up during vmentry aren't they? So
if the IPI occurs before vmentry, why is there a race? Isn't the only
purpose of __vmx_deliver_posted_interrupt() to ensure that the pending
virtual interrupts of the given vcpu get scanned/handled by the appropriate
processor?

 -- Keir

> vmx_asm_do_vmentry:
>         call vmx_intr_assist
>         call nvmx_switch_guest
>         ASSERT_NOT_IN_ATOMIC
>                               <---------if posted interrupt arrived here and
> we don't set softirq, then the interrupt will be delay until next vmentry.
>         GET_CURRENT(%rbx)
>         cli
> 
>         mov  VCPU_processor(%rbx),%eax
>         shl  $IRQSTAT_shift,%eax
>         lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
>         cmpl $0,(%rdx,%rax,1)
>         jnz  .Lvmx_process_softirqs
> 
> Actually, the posted interrupt is same with a normal vcpu kick IPI, except
> that if cpu received it in non-root mode, it will not cause vmext. So we must
> follow the vcpu_kick logic.

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Zhang, Yang Z]

Keir Fraser wrote on 2013-04-09:
> On 09/04/2013 08:58, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> 
>> If posted interrupt arrived after vmx_intr_assit() and before vmentry, then
>> the interrupt cannot be inject to guest in time.
> 
> But pending virtual interrupts are picked up during vmentry aren't they?
Yes. But in this case, the interrupt still in PIR not in vIRR, and vmentry cannot pick up interrupt from PIR. So we must sync PIR to vIRR before vmentry.

> So if the IPI occurs before vmentry, why is there a race? Isn't the only
> purpose of __vmx_deliver_posted_interrupt() to ensure that the pending
> virtual interrupts of the given vcpu get scanned/handled by the
> appropriate processor?
> 
>  -- Keir
>  
>> vmx_asm_do_vmentry:
>>         call vmx_intr_assist
>>         call nvmx_switch_guest
>>         ASSERT_NOT_IN_ATOMIC
>>                               <---------if posted interrupt arrived here and
>> we don't set softirq, then the interrupt will be delay until next vmentry.
>>         GET_CURRENT(%rbx)
>>         cli
>>         
>>         mov  VCPU_processor(%rbx),%eax
>>         shl  $IRQSTAT_shift,%eax
>>         lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
>>         cmpl $0,(%rdx,%rax,1)
>>         jnz  .Lvmx_process_softirqs
>> Actually, the posted interrupt is same with a normal vcpu kick IPI, except
>> that if cpu received it in non-root mode, it will not cause vmext. So we must
>> follow the vcpu_kick logic.
>


Best regards,
Yang

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Keir Fraser]

On 09/04/2013 09:23, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:

> Keir Fraser wrote on 2013-04-09:
>> On 09/04/2013 08:58, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> 
>>> If posted interrupt arrived after vmx_intr_assit() and before vmentry, then
>>> the interrupt cannot be inject to guest in time.
>> 
>> But pending virtual interrupts are picked up during vmentry aren't they?
> Yes. But in this case, the interrupt still in PIR not in vIRR, and vmentry
> cannot pick up interrupt from PIR. So we must sync PIR to vIRR before vmentry.

Ah, I see. That makes sense.

I think I would like more code sharing of vcpu_kick(), but I know how to
make that change myself easier than explaining it. So I'm happy for this
part of the patch to go in as-is, and I'd clean up after.

 -- Keir

>> So if the IPI occurs before vmentry, why is there a race? Isn't the only
>> purpose of __vmx_deliver_posted_interrupt() to ensure that the pending
>> virtual interrupts of the given vcpu get scanned/handled by the
>> appropriate processor?
>> 
>>  -- Keir
>>  
>>> vmx_asm_do_vmentry:
>>>         call vmx_intr_assist
>>>         call nvmx_switch_guest
>>>         ASSERT_NOT_IN_ATOMIC
>>>                               <---------if posted interrupt arrived here and
>>> we don't set softirq, then the interrupt will be delay until next vmentry.
>>>         GET_CURRENT(%rbx)
>>>         cli
>>>         
>>>         mov  VCPU_processor(%rbx),%eax
>>>         shl  $IRQSTAT_shift,%eax
>>>         lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
>>>         cmpl $0,(%rdx,%rax,1)
>>>         jnz  .Lvmx_process_softirqs
>>> Actually, the posted interrupt is same with a normal vcpu kick IPI, except
>>> that if cpu received it in non-root mode, it will not cause vmext. So we
>>> must
>>> follow the vcpu_kick logic.
>> 
> 
> 
> Best regards,
> Yang
> 
> 

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Jan Beulich]

>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
> +static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector, u8 trig_mode)
> +{
> +    vlapic_set_tmr(vcpu_vlapic(v), vector, trig_mode);
> +
> +    vmx_update_eoi_exit_bitmap(v, vector, trig_mode);
> +
> +    if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
> +        return;
> +
> +    if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) )
> +    {
> +        /* If EOI exitbitmap needs to changed or notification vector
> +         * can't be allocated, interrupt will not be injected till
> +         * VMEntry as it used to be
> +         */
> +        pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);

Why is this a test-and-set when the result is unused?

> +        goto out;
> +    }
> +
> +    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )

And if it's just because you don't want to have a simple "set" only
operation, then the two instances should be folded, and the result
stored in a local variable.

> +static void vmx_sync_pir_to_irr(struct vcpu *v)
> +{
> +    struct vlapic *vlapic = vcpu_vlapic(v);
> +    u32 val;
> +    int offset, group;

unsigned.

> +
> +    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
> +        return;
> +
> +    for (group = 0; group < 8; group++ )

Formatting.

> +    {
> +        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
> +        offset = APIC_IRR + 0x10 * group;
> +        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;

Can't you use vlapic_set_vector() here (even if that means
looping over vectors individually rather than groups), to add the
necessary atomicity (I don't see how you avoid races with other
updates) and to avoid the ugly cast?

Jan

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Zhang, Yang Z]

Jan Beulich wrote on 2013-04-09:
>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>> +static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector, u8 trig_mode)
>> +{
>> +    vlapic_set_tmr(vcpu_vlapic(v), vector, trig_mode);
>> +
>> +    vmx_update_eoi_exit_bitmap(v, vector, trig_mode);
>> +
>> +    if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
>> +        return;
>> +
>> +    if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) )
>> +    {
>> +        /* If EOI exitbitmap needs to changed or notification vector
>> +         * can't be allocated, interrupt will not be injected till
>> +         * VMEntry as it used to be
>> +         */
>> +        pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);
> 
> Why is this a test-and-set when the result is unused?
>
> 
>> +        goto out;
>> +    }
>> +
>> +    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
> 
> And if it's just because you don't want to have a simple "set" only
> operation, then the two instances should be folded, and the result
> stored in a local variable.
Yes. Only want to set on bit. Perhaps it is better to define a separate function pi_set_on() for this to make the logic more clear.

> 
>> +static void vmx_sync_pir_to_irr(struct vcpu *v)
>> +{
>> +    struct vlapic *vlapic = vcpu_vlapic(v);
>> +    u32 val;
>> +    int offset, group;
> 
> unsigned.
Sure.
 
>> +
>> +    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
>> +        return;
>> +
>> +    for (group = 0; group < 8; group++ )
> 
> Formatting.
Sure.

> 
>> +    {
>> +        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
>> +        offset = APIC_IRR + 0x10 * group;
>> +        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;
> 
> Can't you use vlapic_set_vector() here (even if that means
> looping over vectors individually rather than groups), to add the
> necessary atomicity (I don't see how you avoid races with other
> updates) and to avoid the ugly cast?
> 
> Jan


Best regards,
Yang

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Keir Fraser]

On 09/04/2013 09:40, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:

>>> +    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
>> 
>> And if it's just because you don't want to have a simple "set" only
>> operation, then the two instances should be folded, and the result
>> stored in a local variable.
> Yes. Only want to set on bit. Perhaps it is better to define a separate
> function pi_set_on() for this to make the logic more clear.

Yes!

 -- Keir

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Zhang, Yang Z]

Zhang, Yang Z wrote on 2013-04-09:
> Jan Beulich wrote on 2013-04-09:
>>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>> +static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector, u8 trig_mode)
>>> +{
>>> +    vlapic_set_tmr(vcpu_vlapic(v), vector, trig_mode);
>>> +
>>> +    vmx_update_eoi_exit_bitmap(v, vector, trig_mode);
>>> +
>>> +    if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
>>> +        return;
>>> +
>>> +    if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) )
>>> +    {
>>> +        /* If EOI exitbitmap needs to changed or notification vector
>>> +         * can't be allocated, interrupt will not be injected till
>>> +         * VMEntry as it used to be
>>> +         */
>>> +        pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);
>> 
>> Why is this a test-and-set when the result is unused?
>> 
>> 
>>> +        goto out;
>>> +    }
>>> +
>>> +    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
>> 
>> And if it's just because you don't want to have a simple "set" only
>> operation, then the two instances should be folded, and the result
>> stored in a local variable.
> Yes. Only want to set on bit. Perhaps it is better to define a separate function
> pi_set_on() for this to make the logic more clear.
> 
>> 
>>> +static void vmx_sync_pir_to_irr(struct vcpu *v)
>>> +{
>>> +    struct vlapic *vlapic = vcpu_vlapic(v);
>>> +    u32 val;
>>> +    int offset, group;
>> 
>> unsigned.
> Sure.
> 
>>> +
>>> +    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
>>> +        return;
>>> +
>>> +    for (group = 0; group < 8; group++ )
>> 
>> Formatting.
> Sure.
> 
>> 
>>> +    {
>>> +        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
>>> +        offset = APIC_IRR + 0x10 * group;
>>> +        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;
>> 
>> Can't you use vlapic_set_vector() here (even if that means
>> looping over vectors individually rather than groups), to add the
>> necessary atomicity (I don't see how you avoid races with other
>> updates) and to avoid the ugly cast?
Which races?

Best regards,
Yang

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Jan Beulich]

>>> On 09.04.13 at 10:53, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Zhang, Yang Z wrote on 2013-04-09:
>> Jan Beulich wrote on 2013-04-09:
>>>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@intel.com> wrote:
>>>> +    {
>>>> +        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
>>>> +        offset = APIC_IRR + 0x10 * group;
>>>> +        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;
>>> 
>>> Can't you use vlapic_set_vector() here (even if that means
>>> looping over vectors individually rather than groups), to add the
>>> necessary atomicity (I don't see how you avoid races with other
>>> updates) and to avoid the ugly cast?
> Which races?

vlapic.c also updates the APIC_IRR bit array, and hence (unless
you can guarantee that now and forever such updates only
happen when the subject vCPU is current, which I don't think is
the case) your non-atomic read-modify-write operation here can
discard an update done in vlapic.c. And am I wrong in recalling
that the CPU may actually also on its own update that bit array?

Jan

Re: [PATCH 3/4] VMX: Add posted interrupt supporting

[Keir Fraser]

On 09/04/2013 10:00, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> Can't you use vlapic_set_vector() here (even if that means
>>>> looping over vectors individually rather than groups), to add the
>>>> necessary atomicity (I don't see how you avoid races with other
>>>> updates) and to avoid the ugly cast?
>> Which races?
> 
> vlapic.c also updates the APIC_IRR bit array, and hence (unless
> you can guarantee that now and forever such updates only
> happen when the subject vCPU is current, which I don't think is
> the case)

It's not the case. Which is one reason the IRR updates (and I think all
bitmap updates) in vlapic.c are atomic.

 -- Keir

> your non-atomic read-modify-write operation here can
> discard an update done in vlapic.c. And am I wrong in recalling
> that the CPU may actually also on its own update that bit array?

[PATCH 4/4] VMX: Use posted interrupt to deliver virutal interrupt

[Yang Zhang]

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

Deliver virtual interrupt through posted way if posted interrupt
is enabled.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/hvm/vioapic.c        |    4 +++-
 xen/arch/x86/hvm/vlapic.c         |   13 ++++++++++---
 xen/arch/x86/hvm/vmsi.c           |    5 ++++-
 xen/arch/x86/hvm/vmx/vpmu_core2.c |    5 ++++-
 xen/arch/x86/hvm/vpt.c            |   10 +++++++---
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
index d3de695..b543e55 100644
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -263,7 +263,9 @@ static void ioapic_inj_irq(
     ASSERT((delivery_mode == dest_Fixed) ||
            (delivery_mode == dest_LowestPrio));
 
-    if ( vlapic_set_irq(target, vector, trig_mode) )
+    if ( hvm_funcs.deliver_posted_intr )
+        hvm_funcs.deliver_posted_intr(vlapic_vcpu(target), vector, trig_mode);
+    else if ( vlapic_set_irq(target, vector, trig_mode) )
         vcpu_kick(vlapic_vcpu(target));
 }
 
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 128745c..a8af47a 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -137,6 +137,9 @@ static void vlapic_clear_irr(int vector, struct vlapic *vlapic)
 
 static int vlapic_find_highest_irr(struct vlapic *vlapic)
 {
+    if ( hvm_funcs.sync_pir_to_irr )
+        hvm_funcs.sync_pir_to_irr(vlapic_vcpu(vlapic));
+
     return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
 }
 
@@ -315,9 +318,13 @@ static void vlapic_accept_irq(struct vcpu *v, uint32_t icr_low)
     {
     case APIC_DM_FIXED:
     case APIC_DM_LOWEST:
-        if ( vlapic_enabled(vlapic) &&
-             !vlapic_test_and_set_irr(vector, vlapic) )
-            vcpu_kick(v);
+        if ( vlapic_enabled(vlapic) )
+        {
+            if ( hvm_funcs.deliver_posted_intr )
+                hvm_funcs.deliver_posted_intr(v, vector, 0);
+            else if ( !vlapic_test_and_set_irr(vector, vlapic) )
+                vcpu_kick(v);
+        }
         break;
 
     case APIC_DM_REMRD:
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index cfc7c80..0ac14d1 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -57,7 +57,10 @@ static void vmsi_inj_irq(
     {
     case dest_Fixed:
     case dest_LowestPrio:
-        if ( vlapic_set_irq(target, vector, trig_mode) )
+        if ( hvm_funcs.deliver_posted_intr )
+            hvm_funcs.deliver_posted_intr(vlapic_vcpu(target), vector,
+                                        trig_mode);
+        else if ( vlapic_set_irq(target, vector, trig_mode) )
             vcpu_kick(vlapic_vcpu(target));
         break;
     default:
diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c b/xen/arch/x86/hvm/vmx/vpmu_core2.c
index 2313e39..dc152f7 100644
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -737,7 +737,10 @@ static int core2_vpmu_do_interrupt(struct cpu_user_regs *regs)
     int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
     vlapic_set_reg(vlapic, APIC_LVTPC, vlapic_lvtpc | APIC_LVT_MASKED);
     if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
-        vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
+        if ( hvm_funcs.deliver_posted_intr )
+            hvm_funcs.deliver_posted_intr(v, int_vec, 0);
+        else 
+            vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
     else
         v->nmi_pending = 1;
     return 1;
diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
index 46d3ec6..7c3f2ba 100644
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -257,9 +257,13 @@ int pt_update_irq(struct vcpu *v)
 
     spin_unlock(&v->arch.hvm_vcpu.tm_lock);
 
-    if ( is_lapic )
-        vlapic_set_irq(vcpu_vlapic(v), irq, 0);
-    else if ( irq == RTC_IRQ && pt_priv )
+    if ( is_lapic ) 
+    {
+        if ( hvm_funcs.deliver_posted_intr )
+            hvm_funcs.deliver_posted_intr(v, irq, 0);
+        else
+            vlapic_set_irq(vcpu_vlapic(v), irq, 0);
+    } else if ( irq == RTC_IRQ && pt_priv )
         rtc_periodic_interrupt(pt_priv);
     else
     {
-- 
1.7.1

Re: [PATCH 4/4] VMX: Use posted interrupt to deliver virutal interrupt

[Keir Fraser]

On 09/04/2013 07:01, "Yang Zhang" <yang.z.zhang@intel.com> wrote:

> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Deliver virtual interrupt through posted way if posted interrupt
> is enabled.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>

You modify many but not all callers of vlapic_set_irq() -- is that
intentional? Would it be better to modify vlapic_set_irq(), or add a new
wrapper function for most/all current callers of vlapic_set_irq()?

Just seems that the method presented here is a bit uglier and more fragile
than perhaps it needs to be.

 -- Keir

> ---
>  xen/arch/x86/hvm/vioapic.c        |    4 +++-
>  xen/arch/x86/hvm/vlapic.c         |   13 ++++++++++---
>  xen/arch/x86/hvm/vmsi.c           |    5 ++++-
>  xen/arch/x86/hvm/vmx/vpmu_core2.c |    5 ++++-
>  xen/arch/x86/hvm/vpt.c            |   10 +++++++---
>  5 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
> index d3de695..b543e55 100644
> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -263,7 +263,9 @@ static void ioapic_inj_irq(
>      ASSERT((delivery_mode == dest_Fixed) ||
>             (delivery_mode == dest_LowestPrio));
>  
> -    if ( vlapic_set_irq(target, vector, trig_mode) )
> +    if ( hvm_funcs.deliver_posted_intr )
> +        hvm_funcs.deliver_posted_intr(vlapic_vcpu(target), vector,
> trig_mode);
> +    else if ( vlapic_set_irq(target, vector, trig_mode) )
>          vcpu_kick(vlapic_vcpu(target));
>  }
>  
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 128745c..a8af47a 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -137,6 +137,9 @@ static void vlapic_clear_irr(int vector, struct vlapic
> *vlapic)
>  
>  static int vlapic_find_highest_irr(struct vlapic *vlapic)
>  {
> +    if ( hvm_funcs.sync_pir_to_irr )
> +        hvm_funcs.sync_pir_to_irr(vlapic_vcpu(vlapic));
> +
>      return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
>  }
>  
> @@ -315,9 +318,13 @@ static void vlapic_accept_irq(struct vcpu *v, uint32_t
> icr_low)
>      {
>      case APIC_DM_FIXED:
>      case APIC_DM_LOWEST:
> -        if ( vlapic_enabled(vlapic) &&
> -             !vlapic_test_and_set_irr(vector, vlapic) )
> -            vcpu_kick(v);
> +        if ( vlapic_enabled(vlapic) )
> +        {
> +            if ( hvm_funcs.deliver_posted_intr )
> +                hvm_funcs.deliver_posted_intr(v, vector, 0);
> +            else if ( !vlapic_test_and_set_irr(vector, vlapic) )
> +                vcpu_kick(v);
> +        }
>          break;
>  
>      case APIC_DM_REMRD:
> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> index cfc7c80..0ac14d1 100644
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -57,7 +57,10 @@ static void vmsi_inj_irq(
>      {
>      case dest_Fixed:
>      case dest_LowestPrio:
> -        if ( vlapic_set_irq(target, vector, trig_mode) )
> +        if ( hvm_funcs.deliver_posted_intr )
> +            hvm_funcs.deliver_posted_intr(vlapic_vcpu(target), vector,
> +                                        trig_mode);
> +        else if ( vlapic_set_irq(target, vector, trig_mode) )
>              vcpu_kick(vlapic_vcpu(target));
>          break;
>      default:
> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> index 2313e39..dc152f7 100644
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -737,7 +737,10 @@ static int core2_vpmu_do_interrupt(struct cpu_user_regs
> *regs)
>      int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
>      vlapic_set_reg(vlapic, APIC_LVTPC, vlapic_lvtpc | APIC_LVT_MASKED);
>      if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
> -        vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
> +        if ( hvm_funcs.deliver_posted_intr )
> +            hvm_funcs.deliver_posted_intr(v, int_vec, 0);
> +        else 
> +            vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
>      else
>          v->nmi_pending = 1;
>      return 1;
> diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
> index 46d3ec6..7c3f2ba 100644
> --- a/xen/arch/x86/hvm/vpt.c
> +++ b/xen/arch/x86/hvm/vpt.c
> @@ -257,9 +257,13 @@ int pt_update_irq(struct vcpu *v)
>  
>      spin_unlock(&v->arch.hvm_vcpu.tm_lock);
>  
> -    if ( is_lapic )
> -        vlapic_set_irq(vcpu_vlapic(v), irq, 0);
> -    else if ( irq == RTC_IRQ && pt_priv )
> +    if ( is_lapic )
> +    {
> +        if ( hvm_funcs.deliver_posted_intr )
> +            hvm_funcs.deliver_posted_intr(v, irq, 0);
> +        else
> +            vlapic_set_irq(vcpu_vlapic(v), irq, 0);
> +    } else if ( irq == RTC_IRQ && pt_priv )
>          rtc_periodic_interrupt(pt_priv);
>      else
>      {

Re: [PATCH 4/4] VMX: Use posted interrupt to deliver virutal interrupt

[Zhang, Yang Z]

Keir Fraser wrote on 2013-04-09:
> On 09/04/2013 07:01, "Yang Zhang" <yang.z.zhang@intel.com> wrote:
> 
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Deliver virtual interrupt through posted way if posted interrupt
>> is enabled.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com>
> 
> You modify many but not all callers of vlapic_set_irq() -- is that
> intentional? Would it be better to modify vlapic_set_irq(), or add a new
> wrapper function for most/all current callers of vlapic_set_irq()?
You are right. It should be better to use a new wrapper function for this.

> 
> Just seems that the method presented here is a bit uglier and more fragile
> than perhaps it needs to be.
> 
>  -- Keir
>  
>> ---
>>  xen/arch/x86/hvm/vioapic.c        |    4 +++-
>>  xen/arch/x86/hvm/vlapic.c         |   13 ++++++++++---
>>  xen/arch/x86/hvm/vmsi.c           |    5 ++++-
>>  xen/arch/x86/hvm/vmx/vpmu_core2.c |    5 ++++-
>>  xen/arch/x86/hvm/vpt.c            |   10 +++++++---
>>  5 files changed, 28 insertions(+), 9 deletions(-)
>> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
>> index d3de695..b543e55 100644
>> --- a/xen/arch/x86/hvm/vioapic.c
>> +++ b/xen/arch/x86/hvm/vioapic.c
>> @@ -263,7 +263,9 @@ static void ioapic_inj_irq(
>>      ASSERT((delivery_mode == dest_Fixed) ||
>>             (delivery_mode == dest_LowestPrio));
>> -    if ( vlapic_set_irq(target, vector, trig_mode) )
>> +    if ( hvm_funcs.deliver_posted_intr )
>> +        hvm_funcs.deliver_posted_intr(vlapic_vcpu(target), vector,
>> trig_mode);
>> +    else if ( vlapic_set_irq(target, vector, trig_mode) )
>>          vcpu_kick(vlapic_vcpu(target));
>>  }
>> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
>> index 128745c..a8af47a 100644
>> --- a/xen/arch/x86/hvm/vlapic.c
>> +++ b/xen/arch/x86/hvm/vlapic.c
>> @@ -137,6 +137,9 @@ static void vlapic_clear_irr(int vector, struct vlapic
>> *vlapic)
>> 
>>  static int vlapic_find_highest_irr(struct vlapic *vlapic)
>>  {
>> +    if ( hvm_funcs.sync_pir_to_irr )
>> +        hvm_funcs.sync_pir_to_irr(vlapic_vcpu(vlapic));
>> +
>>      return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
>>  }
>> @@ -315,9 +318,13 @@ static void vlapic_accept_irq(struct vcpu *v, uint32_t
>> icr_low)
>>      {
>>      case APIC_DM_FIXED:
>>      case APIC_DM_LOWEST:
>> -        if ( vlapic_enabled(vlapic) &&
>> -             !vlapic_test_and_set_irr(vector, vlapic) )
>> -            vcpu_kick(v);
>> +        if ( vlapic_enabled(vlapic) )
>> +        {
>> +            if ( hvm_funcs.deliver_posted_intr )
>> +                hvm_funcs.deliver_posted_intr(v, vector, 0);
>> +            else if ( !vlapic_test_and_set_irr(vector, vlapic) )
>> +                vcpu_kick(v);
>> +        }
>>          break;
>>      case APIC_DM_REMRD:
>> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
>> index cfc7c80..0ac14d1 100644
>> --- a/xen/arch/x86/hvm/vmsi.c
>> +++ b/xen/arch/x86/hvm/vmsi.c
>> @@ -57,7 +57,10 @@ static void vmsi_inj_irq(
>>      {
>>      case dest_Fixed:
>>      case dest_LowestPrio:
>> -        if ( vlapic_set_irq(target, vector, trig_mode) )
>> +        if ( hvm_funcs.deliver_posted_intr )
>> +            hvm_funcs.deliver_posted_intr(vlapic_vcpu(target), vector,
>> +                                        trig_mode);
>> +        else if ( vlapic_set_irq(target, vector, trig_mode) )
>>              vcpu_kick(vlapic_vcpu(target));
>>          break;
>>      default:
>> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
>> b/xen/arch/x86/hvm/vmx/vpmu_core2.c index 2313e39..dc152f7 100644 ---
>> a/xen/arch/x86/hvm/vmx/vpmu_core2.c +++
>> b/xen/arch/x86/hvm/vmx/vpmu_core2.c @@ -737,7 +737,10 @@ static int
>> core2_vpmu_do_interrupt(struct cpu_user_regs *regs)
>>      int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
>>      vlapic_set_reg(vlapic, APIC_LVTPC, vlapic_lvtpc | APIC_LVT_MASKED);
>>      if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
>> -        vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
>> +        if ( hvm_funcs.deliver_posted_intr )
>> +            hvm_funcs.deliver_posted_intr(v, int_vec, 0);
>> +        else
>> +            vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
>>      else
>>          v->nmi_pending = 1;
>>      return 1;
>> diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
>> index 46d3ec6..7c3f2ba 100644
>> --- a/xen/arch/x86/hvm/vpt.c
>> +++ b/xen/arch/x86/hvm/vpt.c
>> @@ -257,9 +257,13 @@ int pt_update_irq(struct vcpu *v)
>> 
>>      spin_unlock(&v->arch.hvm_vcpu.tm_lock);
>> -    if ( is_lapic )
>> -        vlapic_set_irq(vcpu_vlapic(v), irq, 0);
>> -    else if ( irq == RTC_IRQ && pt_priv )
>> +    if ( is_lapic )
>> +    {
>> +        if ( hvm_funcs.deliver_posted_intr )
>> +            hvm_funcs.deliver_posted_intr(v, irq, 0);
>> +        else
>> +            vlapic_set_irq(vcpu_vlapic(v), irq, 0);
>> +    } else if ( irq == RTC_IRQ && pt_priv )
>>          rtc_periodic_interrupt(pt_priv);
>>      else
>>      {
>


Best regards,
Yang

Re: [PATCH 0/4] Add posted interrupt supporting

[Konrad Rzeszutek Wilk]

On Tue, Apr 09, 2013 at 02:01:27PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> The follwoing patches are adding the Posted Interrupt supporting to Xen:
> Posted Interrupt allows vAPIC interrupts to inject into guest directly
> without any vmexit.

Is there a mechanism for the guest to figure out whether this is exported?

Right now the Linux PVHVM guests will use the event channel mechanism
(via the 0xf3 callback vector). This means that if we want to do
IPIs (RESCHEDULE_VECTOR or CALL_FUNCTION_*) we end up doing a vmexit -
but with this we should be able to do these inside the guest right?

If so, is there a mechanism inside the guest to detect this and
use the HVM mechanisms for IPI?

It is mostly just the matter of not calling 'xen_hvm_smp_init' in the
Linux kernel.

> 
> - When delivering a interrupt to guest, if target vcpu is running,
>   update Posted-interrupt requests bitmap and send a notification event
>   to the vcpu. Then the vcpu will handle this interrupt automatically,
>   without any software involvemnt.
> 
> - If target vcpu is not running or there already a notification event
>   pending in the vcpu, do nothing. The interrupt will be handled by
>   next vm entry
> 
> Refer to Intel SDM vol3, 29.6  to get more information.
> 
> Yang Zhang (4):
>   VMX: Detect posted interrupt capability
>   VMX: Turn on posted interrupt bit in vmcs
>   VMX: Add posted interrupt supporting
>   VMX: Use posted interrupt to deliver virutal interrupt
> 
>  xen/arch/x86/hvm/vioapic.c                     |    4 +-
>  xen/arch/x86/hvm/vlapic.c                      |   19 +++++-
>  xen/arch/x86/hvm/vmsi.c                        |    5 +-
>  xen/arch/x86/hvm/vmx/vmcs.c                    |   18 +++++-
>  xen/arch/x86/hvm/vmx/vmx.c                     |   81 ++++++++++++++++++++++++
>  xen/arch/x86/hvm/vmx/vpmu_core2.c              |    5 +-
>  xen/arch/x86/hvm/vpt.c                         |   10 ++-
>  xen/include/asm-x86/hvm/hvm.h                  |    2 +
>  xen/include/asm-x86/hvm/vlapic.h               |    1 +
>  xen/include/asm-x86/hvm/vmx/vmcs.h             |   13 ++++
>  xen/include/asm-x86/hvm/vmx/vmx.h              |   22 +++++++
>  xen/include/asm-x86/mach-default/irq_vectors.h |    3 +-
>  12 files changed, 172 insertions(+), 11 deletions(-)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH 0/4] Add posted interrupt supporting

[Zhang, Yang Z]

Konrad Rzeszutek Wilk wrote on 2013-04-10:
> On Tue, Apr 09, 2013 at 02:01:27PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> The follwoing patches are adding the Posted Interrupt supporting to Xen:
>> Posted Interrupt allows vAPIC interrupts to inject into guest directly
>> without any vmexit.
> 
> Is there a mechanism for the guest to figure out whether this is exported?
Expose it to guest? No.

> 
> Right now the Linux PVHVM guests will use the event channel mechanism
> (via the 0xf3 callback vector). This means that if we want to do
> IPIs (RESCHEDULE_VECTOR or CALL_FUNCTION_*) we end up doing a vmexit -
> but with this we should be able to do these inside the guest right?
I am not familiar with PV staffs. Will event channel mechanism touch vAPIC? If no, perhaps the answer is not.

> 
> If so, is there a mechanism inside the guest to detect this and
> use the HVM mechanisms for IPI?
Why guest should aware of this? Current implementation already covers the vIPI.

> 
> It is mostly just the matter of not calling 'xen_hvm_smp_init' in the
> Linux kernel.
> 
>> 
>> - When delivering a interrupt to guest, if target vcpu is running,
>>   update Posted-interrupt requests bitmap and send a notification event
>>   to the vcpu. Then the vcpu will handle this interrupt automatically,
>>   without any software involvemnt.
>> - If target vcpu is not running or there already a notification event
>>   pending in the vcpu, do nothing. The interrupt will be handled by
>>   next vm entry
>> Refer to Intel SDM vol3, 29.6  to get more information.
>> 
>> Yang Zhang (4):
>>   VMX: Detect posted interrupt capability
>>   VMX: Turn on posted interrupt bit in vmcs
>>   VMX: Add posted interrupt supporting
>>   VMX: Use posted interrupt to deliver virutal interrupt
>>  xen/arch/x86/hvm/vioapic.c                     |    4 +-
>>  xen/arch/x86/hvm/vlapic.c                      |   19 +++++-
>>  xen/arch/x86/hvm/vmsi.c                        |    5 +-
>>  xen/arch/x86/hvm/vmx/vmcs.c                    |   18 +++++-
>>  xen/arch/x86/hvm/vmx/vmx.c                     |   81
>>  ++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vpmu_core2.c            
>>   |    5 +- xen/arch/x86/hvm/vpt.c                         |   10 ++-
>>  xen/include/asm-x86/hvm/hvm.h                  |    2 +
>>  xen/include/asm-x86/hvm/vlapic.h               |    1 +
>>  xen/include/asm-x86/hvm/vmx/vmcs.h             |   13 ++++
>>  xen/include/asm-x86/hvm/vmx/vmx.h              |   22 +++++++
>>  xen/include/asm-x86/mach-default/irq_vectors.h |    3 +- 12 files
>>  changed, 172 insertions(+), 11 deletions(-)
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>


Best regards,
Yang

Re: [PATCH 0/4] Add posted interrupt supporting

[Konrad Rzeszutek Wilk]

On Wed, Apr 10, 2013 at 02:51:41AM +0000, Zhang, Yang Z wrote:
> Konrad Rzeszutek Wilk wrote on 2013-04-10:
> > On Tue, Apr 09, 2013 at 02:01:27PM +0800, Yang Zhang wrote:
> >> From: Yang Zhang <yang.z.zhang@Intel.com>
> >> 
> >> The follwoing patches are adding the Posted Interrupt supporting to Xen:
> >> Posted Interrupt allows vAPIC interrupts to inject into guest directly
> >> without any vmexit.
> > 
> > Is there a mechanism for the guest to figure out whether this is exported?
> Expose it to guest? No.
> 
> > 
> > Right now the Linux PVHVM guests will use the event channel mechanism
> > (via the 0xf3 callback vector). This means that if we want to do
> > IPIs (RESCHEDULE_VECTOR or CALL_FUNCTION_*) we end up doing a vmexit -
> > but with this we should be able to do these inside the guest right?
> I am not familiar with PV staffs. Will event channel mechanism touch vAPIC? If no, perhaps the answer is not.

No it won't. But any any inter-OS vectors (such as IPIs) would benefit
from vAPIC right? As in posting those vectors (assuming the destination
VCPU is running) would not cause an VMEXIT?
> 
> > 
> > If so, is there a mechanism inside the guest to detect this and
> > use the HVM mechanisms for IPI?
> Why guest should aware of this? Current implementation already covers the vIPI.
 
Not sure I understand you. What current implementation?

> 
> > 
> > It is mostly just the matter of not calling 'xen_hvm_smp_init' in the
> > Linux kernel.
> > 
> >> 
> >> - When delivering a interrupt to guest, if target vcpu is running,
> >>   update Posted-interrupt requests bitmap and send a notification event
> >>   to the vcpu. Then the vcpu will handle this interrupt automatically,
> >>   without any software involvemnt.
> >> - If target vcpu is not running or there already a notification event
> >>   pending in the vcpu, do nothing. The interrupt will be handled by
> >>   next vm entry
> >> Refer to Intel SDM vol3, 29.6  to get more information.
> >> 
> >> Yang Zhang (4):
> >>   VMX: Detect posted interrupt capability
> >>   VMX: Turn on posted interrupt bit in vmcs
> >>   VMX: Add posted interrupt supporting
> >>   VMX: Use posted interrupt to deliver virutal interrupt
> >>  xen/arch/x86/hvm/vioapic.c                     |    4 +-
> >>  xen/arch/x86/hvm/vlapic.c                      |   19 +++++-
> >>  xen/arch/x86/hvm/vmsi.c                        |    5 +-
> >>  xen/arch/x86/hvm/vmx/vmcs.c                    |   18 +++++-
> >>  xen/arch/x86/hvm/vmx/vmx.c                     |   81
> >>  ++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vpmu_core2.c            
> >>   |    5 +- xen/arch/x86/hvm/vpt.c                         |   10 ++-
> >>  xen/include/asm-x86/hvm/hvm.h                  |    2 +
> >>  xen/include/asm-x86/hvm/vlapic.h               |    1 +
> >>  xen/include/asm-x86/hvm/vmx/vmcs.h             |   13 ++++
> >>  xen/include/asm-x86/hvm/vmx/vmx.h              |   22 +++++++
> >>  xen/include/asm-x86/mach-default/irq_vectors.h |    3 +- 12 files
> >>  changed, 172 insertions(+), 11 deletions(-)
> >> 
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> http://lists.xen.org/xen-devel
> >>
> 
> 
> Best regards,
> Yang
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH 0/4] Add posted interrupt supporting

[Zhang, Yang Z]

Konrad Rzeszutek Wilk wrote on 2013-04-10:
> On Wed, Apr 10, 2013 at 02:51:41AM +0000, Zhang, Yang Z wrote:
>> Konrad Rzeszutek Wilk wrote on 2013-04-10:
>>> On Tue, Apr 09, 2013 at 02:01:27PM +0800, Yang Zhang wrote:
>>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>> 
>>>> The follwoing patches are adding the Posted Interrupt supporting to Xen:
>>>> Posted Interrupt allows vAPIC interrupts to inject into guest directly
>>>> without any vmexit.
>>> 
>>> Is there a mechanism for the guest to figure out whether this is exported?
>> Expose it to guest? No.
>> 
>>> 
>>> Right now the Linux PVHVM guests will use the event channel mechanism
>>> (via the 0xf3 callback vector). This means that if we want to do IPIs
>>> (RESCHEDULE_VECTOR or CALL_FUNCTION_*) we end up doing a vmexit - but
>>> with this we should be able to do these inside the guest right?
>> I am not familiar with PV staffs. Will event channel mechanism touch vAPIC? If
> no, perhaps the answer is not.
> 
> No it won't. But any any inter-OS vectors (such as IPIs) would benefit
> from vAPIC right? As in posting those vectors (assuming the destination
> VCPU is running) would not cause an VMEXIT?
Yes.

>> 
>>> 
>>> If so, is there a mechanism inside the guest to detect this and
>>> use the HVM mechanisms for IPI?
>> Why guest should aware of this? Current implementation already covers the
> vIPI.
> 
> Not sure I understand you. What current implementation?
I thought you mean the vIPI. Obviously, I am wrong. :(
What does " HVM mechanisms for IPI " mean? 

> 
>> 
>>> 
>>> It is mostly just the matter of not calling 'xen_hvm_smp_init' in the
>>> Linux kernel.
>>> 
>>>> 
>>>> - When delivering a interrupt to guest, if target vcpu is running,
>>>>   update Posted-interrupt requests bitmap and send a notification
>>>>   event to the vcpu. Then the vcpu will handle this interrupt
>>>>   automatically, without any software involvemnt. - If target vcpu is
>>>>   not running or there already a notification event pending in the
>>>>   vcpu, do nothing. The interrupt will be handled by next vm entry
>>>> Refer to Intel SDM vol3, 29.6  to get more information.
>>>> 
>>>> Yang Zhang (4):
>>>>   VMX: Detect posted interrupt capability
>>>>   VMX: Turn on posted interrupt bit in vmcs
>>>>   VMX: Add posted interrupt supporting
>>>>   VMX: Use posted interrupt to deliver virutal interrupt
>>>>  xen/arch/x86/hvm/vioapic.c                     |    4 +-
>>>>  xen/arch/x86/hvm/vlapic.c                      |   19 +++++-
>>>>  xen/arch/x86/hvm/vmsi.c                        |    5 +-
>>>>  xen/arch/x86/hvm/vmx/vmcs.c                    |   18 +++++-
>>>>  xen/arch/x86/hvm/vmx/vmx.c                     |   81
>>>>  ++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vpmu_core2.c
>>>>   |    5 +- xen/arch/x86/hvm/vpt.c                         |   10
> ++-
>>>>  xen/include/asm-x86/hvm/hvm.h                  |    2 +
>>>>  xen/include/asm-x86/hvm/vlapic.h               |    1 +
>>>>  xen/include/asm-x86/hvm/vmx/vmcs.h             |   13 ++++
>>>>  xen/include/asm-x86/hvm/vmx/vmx.h              |   22 +++++++
>>>>  xen/include/asm-x86/mach-default/irq_vectors.h |    3 +- 12 files
>>>>  changed, 172 insertions(+), 11 deletions(-)
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xen.org
>>>> http://lists.xen.org/xen-devel
>>>> 
>> 
>> 
>> Best regards,
>> Yang
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>


Best regards,
Yang

Re: [PATCH 0/4] Add posted interrupt supporting

[Konrad Rzeszutek Wilk]

On Wed, Apr 10, 2013 at 01:32:41PM +0000, Zhang, Yang Z wrote:
> Konrad Rzeszutek Wilk wrote on 2013-04-10:
> > On Wed, Apr 10, 2013 at 02:51:41AM +0000, Zhang, Yang Z wrote:
> >> Konrad Rzeszutek Wilk wrote on 2013-04-10:
> >>> On Tue, Apr 09, 2013 at 02:01:27PM +0800, Yang Zhang wrote:
> >>>> From: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> 
> >>>> The follwoing patches are adding the Posted Interrupt supporting to Xen:
> >>>> Posted Interrupt allows vAPIC interrupts to inject into guest directly
> >>>> without any vmexit.
> >>> 
> >>> Is there a mechanism for the guest to figure out whether this is exported?
> >> Expose it to guest? No.
> >> 
> >>> 
> >>> Right now the Linux PVHVM guests will use the event channel mechanism
> >>> (via the 0xf3 callback vector). This means that if we want to do IPIs
> >>> (RESCHEDULE_VECTOR or CALL_FUNCTION_*) we end up doing a vmexit - but
> >>> with this we should be able to do these inside the guest right?
> >> I am not familiar with PV staffs. Will event channel mechanism touch vAPIC? If
> > no, perhaps the answer is not.
> > 
> > No it won't. But any any inter-OS vectors (such as IPIs) would benefit
> > from vAPIC right? As in posting those vectors (assuming the destination
> > VCPU is running) would not cause an VMEXIT?
> Yes.
> 
> >> 
> >>> 
> >>> If so, is there a mechanism inside the guest to detect this and
> >>> use the HVM mechanisms for IPI?
> >> Why guest should aware of this? Current implementation already covers the
> > vIPI.
> > 
> > Not sure I understand you. What current implementation?
> I thought you mean the vIPI. Obviously, I am wrong. :(
> What does " HVM mechanisms for IPI " mean? 

'__default_send_IPI_dest_field' in the Linux code, which is
writting in the ICR2 field in the APIC.

Re: [PATCH 0/4] Add posted interrupt supporting

[Stefano Stabellini]

On Wed, 10 Apr 2013, Konrad Rzeszutek Wilk wrote:
> On Wed, Apr 10, 2013 at 01:32:41PM +0000, Zhang, Yang Z wrote:
> > Konrad Rzeszutek Wilk wrote on 2013-04-10:
> > > On Wed, Apr 10, 2013 at 02:51:41AM +0000, Zhang, Yang Z wrote:
> > >> Konrad Rzeszutek Wilk wrote on 2013-04-10:
> > >>> On Tue, Apr 09, 2013 at 02:01:27PM +0800, Yang Zhang wrote:
> > >>>> From: Yang Zhang <yang.z.zhang@Intel.com>
> > >>>> 
> > >>>> The follwoing patches are adding the Posted Interrupt supporting to Xen:
> > >>>> Posted Interrupt allows vAPIC interrupts to inject into guest directly
> > >>>> without any vmexit.
> > >>> 
> > >>> Is there a mechanism for the guest to figure out whether this is exported?
> > >> Expose it to guest? No.
> > >> 
> > >>> 
> > >>> Right now the Linux PVHVM guests will use the event channel mechanism
> > >>> (via the 0xf3 callback vector). This means that if we want to do IPIs
> > >>> (RESCHEDULE_VECTOR or CALL_FUNCTION_*) we end up doing a vmexit - but
> > >>> with this we should be able to do these inside the guest right?
> > >> I am not familiar with PV staffs. Will event channel mechanism touch vAPIC? If
> > > no, perhaps the answer is not.
> > > 
> > > No it won't. But any any inter-OS vectors (such as IPIs) would benefit
> > > from vAPIC right? As in posting those vectors (assuming the destination
> > > VCPU is running) would not cause an VMEXIT?
> > Yes.
> > 
> > >> 
> > >>> 
> > >>> If so, is there a mechanism inside the guest to detect this and
> > >>> use the HVM mechanisms for IPI?
> > >> Why guest should aware of this? Current implementation already covers the
> > > vIPI.
> > > 
> > > Not sure I understand you. What current implementation?
> > I thought you mean the vIPI. Obviously, I am wrong. :(
> > What does " HVM mechanisms for IPI " mean? 
> 
> '__default_send_IPI_dest_field' in the Linux code, which is
> writting in the ICR2 field in the APIC.

I am also interested in this question and I cannot find a clear answer:
when a guest vcpu wants to issue an IPI to another vcpu, will it trap
into Xen? Or will everything work automagically?

Re: [PATCH 0/4] Add posted interrupt supporting

[Jan Beulich]

>>> On 19.04.13 at 13:49, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> I am also interested in this question and I cannot find a clear answer:
> when a guest vcpu wants to issue an IPI to another vcpu, will it trap
> into Xen? Or will everything work automagically?

For PV, obviously, this is done by a hypercall.

For HVM this ought to depend on the level of hardware assistance
in LAPIC virtualization: Traditionally, the involved LAPIC register
accesses would trap into the hypervisor. But with full APIC
virtualization, this should not be happening anymore.

Jan

Re: [PATCH 0/4] Add posted interrupt supporting

[Stefano Stabellini]

On Fri, 19 Apr 2013, Jan Beulich wrote:
> >>> On 19.04.13 at 13:49, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > I am also interested in this question and I cannot find a clear answer:
> > when a guest vcpu wants to issue an IPI to another vcpu, will it trap
> > into Xen? Or will everything work automagically?
> 
> For PV, obviously, this is done by a hypercall.
> 
> For HVM this ought to depend on the level of hardware assistance
> in LAPIC virtualization: Traditionally, the involved LAPIC register
> accesses would trap into the hypervisor.

Right.

> But with full APIC virtualization, this should not be happening anymore.

Is this feature detectable on the guest side?
If so, we could probe it and only if it is available we could drop PV
IPIs in favor of the native IPI ops.

Re: [PATCH 0/4] Add posted interrupt supporting

[Jan Beulich]

>>> On 19.04.13 at 16:16, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> On Fri, 19 Apr 2013, Jan Beulich wrote:
>> But with full APIC virtualization, this should not be happening anymore.
> 
> Is this feature detectable on the guest side?
> If so, we could probe it and only if it is available we could drop PV
> IPIs in favor of the native IPI ops.

Not at the moment afaict.

Jan

Re: [PATCH 0/4] Add posted interrupt supporting

[Zhang, Yang Z]

Jan Beulich wrote on 2013-04-19:
>>>> On 19.04.13 at 13:49, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
>> I am also interested in this question and I cannot find a clear answer:
>> when a guest vcpu wants to issue an IPI to another vcpu, will it trap
>> into Xen? Or will everything work automagically?
> 
> For PV, obviously, this is done by a hypercall.
> 
> For HVM this ought to depend on the level of hardware assistance
> in LAPIC virtualization: Traditionally, the involved LAPIC register
> accesses would trap into the hypervisor. But with full APIC
> virtualization, this should not be happening anymore.
No. Actually, only self IPI will handled by hardware directly. All other vIPIs still cause APIC write vmexit.

Best regards,
Yang

Re: [PATCH 0/4] Add posted interrupt supporting

[Stefano Stabellini]

On Sat, 20 Apr 2013, Zhang, Yang Z wrote:
> Jan Beulich wrote on 2013-04-19:
> >>>> On 19.04.13 at 13:49, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > wrote:
> >> I am also interested in this question and I cannot find a clear answer:
> >> when a guest vcpu wants to issue an IPI to another vcpu, will it trap
> >> into Xen? Or will everything work automagically?
> > 
> > For PV, obviously, this is done by a hypercall.
> > 
> > For HVM this ought to depend on the level of hardware assistance
> > in LAPIC virtualization: Traditionally, the involved LAPIC register
> > accesses would trap into the hypervisor. But with full APIC
> > virtualization, this should not be happening anymore.
> No. Actually, only self IPI will handled by hardware directly. All other vIPIs still cause APIC write vmexit.

Thanks for the clarification, that answers my question.

Re: [PATCH 0/4] Add posted interrupt supporting

[Zhang, Yang Z]

Zhang, Yang Z wrote on 2013-04-09:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> The follwoing patches are adding the Posted Interrupt supporting to Xen:
> Posted Interrupt allows vAPIC interrupts to inject into guest directly
> without any vmexit.
> 
> - When delivering a interrupt to guest, if target vcpu is running,
>   update Posted-interrupt requests bitmap and send a notification event
>   to the vcpu. Then the vcpu will handle this interrupt automatically,
>   without any software involvemnt.
> - If target vcpu is not running or there already a notification event
>   pending in the vcpu, do nothing. The interrupt will be handled by
>   next vm entry
> Refer to Intel SDM vol3, 29.6  to get more information.
Hi Jan,

Are there any additional comments with this patch set? Also, what's your point of using static vector?
If no, I will rebase the patch according to current comments.

 
> Yang Zhang (4):
>   VMX: Detect posted interrupt capability
>   VMX: Turn on posted interrupt bit in vmcs
>   VMX: Add posted interrupt supporting
>   VMX: Use posted interrupt to deliver virutal interrupt
>  xen/arch/x86/hvm/vioapic.c                     |    4 +-
>  xen/arch/x86/hvm/vlapic.c                      |   19 +++++-
>  xen/arch/x86/hvm/vmsi.c                        |    5 +-
>  xen/arch/x86/hvm/vmx/vmcs.c                    |   18 +++++-
>  xen/arch/x86/hvm/vmx/vmx.c                     |   81
>  ++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vpmu_core2.c             
>  |    5 +- xen/arch/x86/hvm/vpt.c                         |   10 ++-
>  xen/include/asm-x86/hvm/hvm.h                  |    2 +
>  xen/include/asm-x86/hvm/vlapic.h               |    1 +
>  xen/include/asm-x86/hvm/vmx/vmcs.h             |   13 ++++
>  xen/include/asm-x86/hvm/vmx/vmx.h              |   22 +++++++
>  xen/include/asm-x86/mach-default/irq_vectors.h |    3 +- 12 files
>  changed, 172 insertions(+), 11 deletions(-)


Best regards,
Yang

Re: [PATCH 0/4] Add posted interrupt supporting

[Jan Beulich]

>>> On 10.04.13 at 15:58, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Are there any additional comments with this patch set? Also, what's your 
> point of using static vector?
> If no, I will rebase the patch according to current comments.

As said before, I'd prefer use of a dynamically allocated vector
unless you can prove that this won't perform well. Keir may
override my vote here though...

Jan

Re: [PATCH 0/4] Add posted interrupt supporting

[Keir Fraser]

On 10/04/2013 15:34, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 10.04.13 at 15:58, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> Are there any additional comments with this patch set? Also, what's your
>> point of using static vector?
>> If no, I will rebase the patch according to current comments.
> 
> As said before, I'd prefer use of a dynamically allocated vector
> unless you can prove that this won't perform well. Keir may
> override my vote here though...

Oh, yes, the vector should be allocated and initialised with
alloc_direct_apic_vector(). That's probably true for a number of the
existing statically-defined vectors too, but at least we should not create
any more of them!

 -- Keir

> Jan
> 

Re: [PATCH 0/4] Add posted interrupt supporting

[Zhang, Yang Z]

Keir Fraser wrote on 2013-04-11:
> On 10/04/2013 15:34, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>>>> On 10.04.13 at 15:58, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>>> Are there any additional comments with this patch set? Also, what's your
>>> point of using static vector?
>>> If no, I will rebase the patch according to current comments.
>> 
>> As said before, I'd prefer use of a dynamically allocated vector
>> unless you can prove that this won't perform well. Keir may
>> override my vote here though...
> 
> Oh, yes, the vector should be allocated and initialised with
> alloc_direct_apic_vector(). That's probably true for a number of the
> existing statically-defined vectors too, but at least we should not create
> any more of them!

Sure. 

Best regards,
Yang