Re: [Xen-devel] [PATCH RFC] VMX: fix vmx_handle_eoi()

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

+Chao to help take a look.

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, October 12, 2018 5:33 PM
> To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Nakajima, Jun <jun.nakajima@xxxxxxxxx>; Tian,
> Kevin <kevin.tian@xxxxxxxxx>
> Subject: [PATCH RFC] VMX: fix vmx_handle_eoi()
> 
> In commit 303066fdb1e ("VMX: fix interaction of APIC-V and Viridian
> emulation") I screwed up quite heavily: Instead of clearing SVI, RVI was
> cleared. Furthermore, unconditional clearing of SVI is wrong too - other
> ISR bits should be taken into account.
> 
> Introduce a new helper set_svi(), split out of vmx_process_isr(), and
> use it also from vmx_handle_eoi().
> 
> Following the problems in vmx_intr_assist() (see the still present big
> block of debugging code there) also warn (once) if EOI'd vector and
> original SVI don't match.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> RFC: Untested, as I didn't see any of the possibly resulting problems
>      in any of my environments. But perhaps this is (part of) the
>      reason of lost interrupts XenServer folks have been observing.
> 
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -448,7 +448,7 @@ void vlapic_EOI_set(struct vlapic *vlapi
>      vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
> 
>      if ( hvm_funcs.handle_eoi )
> -        hvm_funcs.handle_eoi(vector);
> +        hvm_funcs.handle_eoi(vector, vlapic_find_highest_isr(vlapic));
> 
>      vlapic_handle_EOI(vlapic, vector);
> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1941,17 +1941,14 @@ static void vmx_update_eoi_exit_bitmap(s
>          vmx_clear_eoi_exit_bitmap(v, vector);
>  }
> 
> -static void vmx_process_isr(int isr, struct vcpu *v)
> +static u8 set_svi(int isr)
>  {
>      unsigned long status;
>      u8 old;
> -    unsigned int i;
> -    const struct vlapic *vlapic = vcpu_vlapic(v);
> 
>      if ( isr < 0 )
>          isr = 0;
> 
> -    vmx_vmcs_enter(v);
>      __vmread(GUEST_INTR_STATUS, &status);
>      old = status >> VMX_GUEST_INTR_STATUS_SVI_OFFSET;
>      if ( isr != old )
> @@ -1961,6 +1958,18 @@ static void vmx_process_isr(int isr, str
>          __vmwrite(GUEST_INTR_STATUS, status);
>      }
> 
> +    return old;
> +}
> +
> +static void vmx_process_isr(int isr, struct vcpu *v)
> +{
> +    unsigned int i;
> +    const struct vlapic *vlapic = vcpu_vlapic(v);
> +
> +    vmx_vmcs_enter(v);
> +
> +    set_svi(isr);
> +
>      /*
>       * Theoretically, only level triggered interrupts can have their
>       * corresponding bits set in the eoi exit bitmap. That is, the bits
> @@ -2111,14 +2120,13 @@ static bool vmx_test_pir(const struct vc
>      return pi_test_pir(vec, &v->arch.hvm.vmx.pi_desc);
>  }
> 
> -static void vmx_handle_eoi(u8 vector)
> +static void vmx_handle_eoi(uint8_t vector, int isr)
>  {
> -    unsigned long status;
> +    u8 old_svi = set_svi(isr);
> +    static bool warned;
> 
> -    /* We need to clear the SVI field. */
> -    __vmread(GUEST_INTR_STATUS, &status);
> -    status &= VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
> -    __vmwrite(GUEST_INTR_STATUS, status);
> +    if ( vector != old_svi && !test_and_set_bool(warned) )
> +        printk(XENLOG_WARNING "EOI for %02x but SVI=%02x\n", vector,
> old_svi);
>  }
> 
>  static void vmx_enable_msr_interception(struct domain *d, uint32_t msr)
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -201,7 +201,7 @@ struct hvm_function_table {
>      void (*deliver_posted_intr)(struct vcpu *v, u8 vector);
>      void (*sync_pir_to_irr)(struct vcpu *v);
>      bool (*test_pir)(const struct vcpu *v, uint8_t vector);
> -    void (*handle_eoi)(u8 vector);
> +    void (*handle_eoi)(uint8_t vector, int isr);
> 
>      /*Walk nested p2m  */
>      int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa,
> 
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel