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

[Keir Fraser <keir.xen@gmail.com>]

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
>      {