Re: [Xen-devel] [PATCH for 4.5 v2] vmx, apicv: fix save/restore issue with apicv

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

> From: Zhang, Yang Z
> Sent: Tuesday, October 21, 2014 1:33 PM
> 
> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> 
> This patch fixes two issues:
> 
> 1. Interrupts on PIR are lost during save/restore. Syncing the PIR
> into IRR during save will fix it.
> 
> 2. EOI exit bitmap doesn't set up correctly after restore. Here we
> will construct the eoi exit bitmap via (IRR | ISR). Though it may cause
> unnecessary eoi exit of the interrupts that pending in IRR or ISR during
> save/restore, each pending interrupt only causes one vmexit. The
> subsequent interrupts will adjust the eoi exit bitmap correctly. So
> the performance hurt can be ignored.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> ---
> Hi Olaf and Malcolm, please help to retest this version.
> 
>  xen/arch/x86/hvm/vlapic.c        |    3 +++
>  xen/arch/x86/hvm/vmx/vmx.c       |   24 ++++++++++++++++++++++++
>  xen/include/asm-x86/hvm/vlapic.h |    2 ++
>  3 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 089d13f..6691e50 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -1293,6 +1293,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;
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 304aeea..df4737d 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1584,6 +1584,8 @@ static void vmx_process_isr(int isr, struct vcpu *v)
>  {
>      unsigned long status;
>      u8 old;
> +    unsigned int i, vector;
> +    struct vlapic *vlapic = vcpu_vlapic(v);
> 
>      if ( isr < 0 )
>          isr = 0;
> @@ -1597,6 +1599,28 @@ static void vmx_process_isr(int isr, struct vcpu *v)
>          status |= isr << VMX_GUEST_INTR_STATUS_SVI_OFFSET;
>          __vmwrite(GUEST_INTR_STATUS, status);
>      }
> +
> +    /*
> +     * Theoretically, only level triggered interrupts can have their
> +     * corresponding bits set in the eoi exit bitmap. That is, the bits
> +     * set in the eoi exit bitmap should also be set in TMR. But a periodic
> +     * timer interrupt does not follow the rule: it is edge triggered, but
> +     * requires its corresponding bit be set in the eoi exit bitmap. So we
> +     * should not construct the eoi exit bitmap based on TMR.
> +     * Here we will construct the eoi exit bitmap via (IRR | ISR). This
> +     * means that EOIs to the interrupts that are set in the IRR or ISR will
> +     * cause VM exits after restoring, regardless of the trigger modes. It
> +     * is acceptable because the subsequent interrupts will set up the eoi
> +     * bitmap correctly.
> +     */
> +    for ( vector = 0x10; vector < NR_VECTORS; vector++ )
> +        if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_IRR]) ||
> +             vlapic_test_vector(vector, &vlapic->regs->data[APIC_ISR]) )
> +            set_bit(vector,  v->arch.hvm_vmx.eoi_exit_bitmap);
> +
> +    for ( i = 0; i < 4; i++ )
> +        __vmwrite(EOI_EXIT_BITMAP(i),
> v->arch.hvm_vmx.eoi_exit_bitmap[i]);
> +
>      vmx_vmcs_exit(v);
>  }
> 
> diff --git a/xen/include/asm-x86/hvm/vlapic.h
> b/xen/include/asm-x86/hvm/vlapic.h
> index 16752b5..384b59a 100644
> --- a/xen/include/asm-x86/hvm/vlapic.h
> +++ b/xen/include/asm-x86/hvm/vlapic.h
> @@ -61,6 +61,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)
> \
> --
> 1.7.6.3


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