Re: [RFC v1 12/15] vmx: Properly handle notification event when vCPU is running

["Zhang, Yang Z" <yang.z.zhang@intel.com>]

Wu, Feng wrote on 2015-03-27:
> 
> 
> Zhang, Yang Z wrote on 2015-03-25:
>> when vCPU is running
>> 
>> Wu, Feng wrote on 2015-03-25:
>>> When a vCPU is running in Root mode and a notification event has
>>> been injected to it. we need to set VCPU_KICK_SOFTIRQ for the
>>> current cpu, so the pending interrupt in PIRR will be synced to
>>> vIRR before
> VM-Exit in time.
>> 
>> Shouldn't the pending interrupt be synced unconditionally before next
>> vmentry? What happens if we didn't set the softirq?
> 
> If we didn't set the softirq in the notification handler, the
> interrupts happened exactly before VM-entry cannot be delivered to
> guest at this time. Please see the following code fragments from
> xen/arch/x86/hvm/vmx/entry.S: (pls pay attention to the comments)
> 
> .Lvmx_do_vmentry
> 
> ......
> 		/* If Vt-d engine issues a notification event here,
>          * it cannot be delivered to guest during this VM-entry
>          * without raising the softirq in notification handler. */
>         cmp  %ecx,(%rdx,%rax,1)
>         jnz  .Lvmx_process_softirqs
> ......
> 
>         je   .Lvmx_launch
> ......
> 
> 
> .Lvmx_process_softirqs:
>         sti
>         call do_softirq
>         jmp  .Lvmx_do_vmentry

You are right! This helps me to recall why raise the softirq when delivering the PI.

> Thanks,
> Feng
> 
>> 
>>> 
>>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>>> ---
>>>  xen/arch/x86/hvm/vmx/vmx.c        | 24 +++++++++++++++++++++++-
>>>  xen/include/asm-x86/hvm/vmx/vmx.h |  1 +
>>>  2 files changed, 24 insertions(+), 1 deletion(-) diff --git
>>> a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index
>>> b2b4c26..b30392c 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++
>>> b/xen/arch/x86/hvm/vmx/vmx.c @@ -1838,7 +1838,7 @@ const struct
>>> hvm_function_table * __init start_vmx(void)
>>> 
>>>      if ( cpu_has_vmx_posted_intr_processing )
>>>      {
>>> -        alloc_direct_apic_vector(&posted_intr_vector,
>>> event_check_interrupt); +
>>> alloc_direct_apic_vector(&posted_intr_vector, +
>>> pi_notification_interrupt);
>>> 
>>>          if ( iommu_intpost )
>>>              alloc_direct_apic_vector(&pi_wakeup_vector,
>>> pi_wakeup_interrupt); @@ -3288,6 +3288,28 @@ void
>>> pi_wakeup_interrupt(struct cpu_user_regs *regs)  }
>>> 
>>>  /*
>>> + * Handle VT-d posted-interrupt when VCPU is running. + */ +
>>> + +void
>>> pi_notification_interrupt(struct cpu_user_regs *regs) { +    /* +     *
>>> We get here because a vCPU is running in Root mode +     * and a
>>> notification event has been injected to it. +     * +     * we need to
>>> set VCPU_KICK_SOFTIRQ for the current +     * cpu, just like
>>> __vmx_deliver_posted_interrupt(). +     * +     * So the pending
>>> interrupt in PIRR will be synced to +     * vIRR before VM-Exit in time.
>>> +     */ +    set_bit(VCPU_KICK_SOFTIRQ,
>>> &softirq_pending(smp_processor_id())); + +    ack_APIC_irq(); +
>>> this_cpu(irq_count)++; +} + +/*
>>>   * Local variables:
>>>   * mode: C
>>>   * c-file-style: "BSD"
>>> diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h
>>> b/xen/include/asm-x86/hvm/vmx/vmx.h index f4296ab..e53275b 100644 ---
>>> a/xen/include/asm-x86/hvm/vmx/vmx.h +++
>>> b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -576,6 +576,7 @@ void
>>> free_p2m_hap_data(struct p2m_domain *p2m); void
>>> p2m_init_hap_data(struct p2m_domain *p2m);
>>> 
>>>  void pi_wakeup_interrupt(struct cpu_user_regs *regs);
>>> +void pi_notification_interrupt(struct cpu_user_regs *regs);
>>> 
>>>  /* EPT violation qualifications definitions */
>>>  #define _EPT_READ_VIOLATION         0
>> 
>> 
>> Best regards,
>> Yang
>>


Best regards,
Yang