Re: [Xen-devel] [PATCH] x86/hvm: implement save/restore for posted interrupts

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Zhang, Yang Z
> Sent: Thursday, July 17, 2014 2:30 AM
> 
> Olaf Hering wrote on 2014-07-17:
> > On Wed, Jul 16, Tian, Kevin wrote:
> >
> >> could you figure out the reason? irr->pir propagation should happen
> >> automatically when injecting a pending irr, which is supposed to be
> >> triggered in the restore path. Otherwise I don't understand how it's
> >> handled even w/o pir concept...
> >
> > I can probably do that. And I think initially I just changed
> > lapic_save_regs, which was not enough. I think in the end the reason
> > is that with posted irqs the code paths are different and just having IRR 
> > set is
> not enough.
> 
> It doesn't make sense. PIR is more like a tmp_IRR. All interrupts pending in 
> PIR
> will finally sync to IRR by hardware. In your case, you already record it in 
> IRR,
> it should be enough.
> The problem I am seeing is that the eoi_exit_bitmap need to be setup correct
> after migration, please try attached patch to see whether it solves your
> problem:

Hi, Yang,

I'm wondering whether eoi_exit_bitmap really matters here (though necessary),
because it is not about injection?

Olaf, could you help with one experiment, to check any pending IRR at restore
when disabling posted interrupt? I'm thinking whether this would be a more
general problem (though unlikely)...

btw, did you run any workload cross save/restore? does it only happen with
xen-platform-pci device?

Thanks
Kevin

> 
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index cd7e872..d9368a7 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -131,7 +131,7 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec,
> uint8_t trig)
>          vlapic_set_vector(vec, &vlapic->regs->data[APIC_TMR]);
> 
>      if ( hvm_funcs.update_eoi_exit_bitmap )
> -        hvm_funcs.update_eoi_exit_bitmap(target, vec, trig);
> +        hvm_funcs.update_eoi_exit_bitmap(target, vec, trig, 0);
> 
>      if ( hvm_funcs.deliver_posted_intr )
>          hvm_funcs.deliver_posted_intr(target, vec);
> @@ -1179,6 +1179,9 @@ static int lapic_save_regs(struct domain *d,
> hvm_domain_context_t *h)
> 
>      for_each_vcpu ( d, v )
>      {
> +        if ( hvm_funcs.sync_pir_to_irr )
> +            hvm_funcs.sync_pir_to_irr(v);
> +
>          s = vcpu_vlapic(v);
>          if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs)) !=
> 0 )
>              break;
> @@ -1230,6 +1233,9 @@ static int lapic_load_regs(struct domain *d,
> hvm_domain_context_t *h)
>      if ( hvm_load_entry(LAPIC_REGS, h, s->regs) != 0 )
>          return -EINVAL;
> 
> +    if ( hvm_funcs.update_eoi_exit_bitmap )
> +        hvm_funcs.update_eoi_exit_bitmap(v, 0, 0, 1);
> +
>      if ( hvm_funcs.process_isr )
>          hvm_funcs.process_isr(vlapic_find_highest_isr(s), v);
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2caa04a..9a2dfd6 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1567,12 +1567,28 @@ static void vmx_set_info_guest(struct vcpu *v)
>      vmx_vmcs_exit(v);
>  }
> 
> -static void vmx_update_eoi_exit_bitmap(struct vcpu *v, u8 vector, u8 trig)
> +static void vmx_rebuild_eoi_exit_bitmap(struct vcpu *v)
>  {
> -    if ( trig )
> -        vmx_set_eoi_exit_bitmap(v, vector);
> +    struct vlapic *vlapic = vcpu_vlapic(v);
> +    unsigned int vector;
> +
> +    for ( vector = 0; vector < NR_VECTORS; vector++ )
> +        if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_IRR]) )
> +            set_bit(vector, v->arch.hvm_vmx.eoi_exit_bitmap);
> +}
> +
> +static void vmx_update_eoi_exit_bitmap(struct vcpu *v, u8 vector,
> +                                       u8 trig, bool_t rebuild)
> +{
> +    if ( rebuild )
> +        vmx_rebuild_eoi_exit_bitmap(v);
>      else
> -        vmx_clear_eoi_exit_bitmap(v, vector);
> +    {
> +        if ( trig )
> +            vmx_set_eoi_exit_bitmap(v, vector);
> +        else
> +            vmx_clear_eoi_exit_bitmap(v, vector);
> +    }
>  }
> 
>  static int vmx_virtual_intr_delivery_enabled(void)
> diff --git a/xen/include/asm-x86/hvm/hvm.h
> b/xen/include/asm-x86/hvm/hvm.h
> index 0ebd478..3e2e365 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -189,7 +189,8 @@ struct hvm_function_table {
>      void (*nhvm_domain_relinquish_resources)(struct domain *d);
> 
>      /* Virtual interrupt delivery */
> -    void (*update_eoi_exit_bitmap)(struct vcpu *v, u8 vector, u8 trig);
> +    void (*update_eoi_exit_bitmap)(struct vcpu *v, u8 vector,
> +                                   u8 trig, bool_t rebuild);
>      int (*virtual_intr_delivery_enabled)(void);
>      void (*process_isr)(int isr, struct vcpu *v);
>      void (*deliver_posted_intr)(struct vcpu *v, u8 vector);
> diff --git a/xen/include/asm-x86/hvm/vlapic.h
> b/xen/include/asm-x86/hvm/vlapic.h
> index 66f0aff..2a83a1e 100644
> --- a/xen/include/asm-x86/hvm/vlapic.h
> +++ b/xen/include/asm-x86/hvm/vlapic.h
> @@ -58,6 +58,8 @@
> 
>  #define VEC_POS(v) ((v) % 32)
>  #define REG_POS(v) (((v) / 32) * 0x10)
> +#define vlapic_test_vector(vec, bitmap)                         \
> +    test_bit(VEC_POS(vec), (uint32_t *)((bitmap) + REG_POS(vec)))
>  #define vlapic_test_and_set_vector(vec, bitmap)
> \
>      test_and_set_bit(VEC_POS(vec), (uint32_t *)((bitmap) + REG_POS(vec)))
>  #define vlapic_test_and_clear_vector(vec, bitmap)
> 
> Best regards,
> Yang
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel