Re: [Xen-devel] [PATCH] X86/vmx: Dump PIR and vIRR before ASSERT()

["Xuquan (Quan Xu)" <xuquan8@xxxxxxxxxx>]

On February 07, 2017 7:33 AM, Chao Gao wrote:
>Commit c7bdecae42 ("x86/apicv: fix RTC periodic timer and apicv issue")
>has added a assertion that intack.vector is the highest priority vector. But
>according to the osstest, the assertion failed sometimes. More discussion
>can be found in the thread
>(https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01019.ht
>ml).
>
>The assertion failure is hard to reproduce. In order to root cause issue, this
>patch is to add logs to dump PIR and vIRR when failure takes place. It
>should be reverted once the root cause is found.
>
>Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>---
> xen/arch/x86/hvm/vmx/intr.c | 43
>+++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
>diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
>index 24e4505..5e5b37a 100644
>--- a/xen/arch/x86/hvm/vmx/intr.c
>+++ b/xen/arch/x86/hvm/vmx/intr.c
>@@ -23,6 +23,7 @@
> #include <xen/errno.h>
> #include <xen/trace.h>
> #include <xen/event.h>
>+#include <asm/apicdef.h>
> #include <asm/current.h>
> #include <asm/cpufeature.h>
> #include <asm/processor.h>
>@@ -318,6 +319,48 @@ void vmx_intr_assist(void)
>         */
>         if ( pt_vector != -1 )
>         {
>+            /*
>+             * We assert that intack.vector is the highest priority vector
>for
>+             * only an interrupt from vlapic can reach this point and
>the
>+             * highest vector is chosen in hvm_vcpu_has_pending_irq().
>+             * But, in fact, the assertion failed sometimes. It is
>suspected
>+             * that PIR is not synced to vIRR which makes pt_vector is
>left in
>+             * PIR. In order to verify this suspicion, dump some
>information
>+             * when the assertion fails.
>+             */

Some comment from QEMU/KVM code, in <linux-kernel>/arch/x86/kvm/lapic.c,

int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
{
        /* This may race with setting of irr in __apic_accept_irq() and
         * value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq
         * will cause vmexit immediately and the value will be recalculated
         * on the next vmentry.
         */
...
}

I am afraid, there may be a similar race with setting of vIRR..

Quan




>+            if ( unlikely(intack.vector < pt_vector) )
>+            {
>+                struct vlapic *vlapic;
>+                struct pi_desc *pi_desc;
>+                uint32_t *word;
>+                int i;
>+
>+                printk("Assertion failed on %pv. Some info are
>below.\n",
>+                       current);
>+                printk("intack info: vector 0x%2x, source %x\n",
>intack.vector,
>+                       intack.source);
>+                printk("pt_vector info: vector 0x%2x\n", pt_vector);
>+
>+                vlapic = vcpu_vlapic(v);
>+                if ( vlapic && vlapic->regs->data )
>+                {
>+                    word = (void *)&vlapic->regs->data[APIC_IRR];
>+                    printk("vIRR:");
>+                    for ( i = NR_VECTORS / 32 - 1; i >= 0 ; i-- )
>+                        printk(" %08x", word[i*4]);
>+                    printk("\n");
>+                }
>+
>+                pi_desc = &v->arch.hvm_vmx.pi_desc;
>+                if ( pi_desc && pi_desc->pir )
>+                {
>+                    word = (void *)&pi_desc->pir;
>+                    printk(" PIR:");
>+                    for ( i = NR_VECTORS / 32 - 1; i >= 0 ; i-- )
>+                        printk(" %08x", word[i]);
>+                    printk("\n");
>+                }
>+            }
>             ASSERT(intack.vector >= pt_vector);
>             vmx_set_eoi_exit_bitmap(v, intack.vector);
>         }
>--
>1.8.3.1


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