[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



> From: Gao, Chao
> Sent: Wednesday, February 08, 2017 8:12 AM
> 
> On Tue, Feb 07, 2017 at 09:18:56AM -0700, Jan Beulich wrote:
> >>>> On 07.02.17 at 08:28, <chao.gao@xxxxxxxxx> wrote:
> >> On Tue, Feb 07, 2017 at 06:46:16AM -0700, Jan Beulich wrote:
> >>>>>> On 07.02.17 at 07:32, <chao.gao@xxxxxxxxx> wrote:
> >>>> On Tue, Feb 07, 2017 at 03:04:32AM -0700, Jan Beulich wrote:
> >>>>>>>> On 07.02.17 at 07:48, <xuquan8@xxxxxxxxxx> wrote:
> >>>>>> 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..
> >>>>>
> >>>>>I think this is unrelated: If another interrupt came in, the highest
> >>>>>set bit in vIRR can only increase. Plus pt_update_irq(), before
> >>>>>returning, calls vlapic_set_irq(), which ought to result in pt_vector's
> >>>>>vIRR bit to be set (either directly or via setting its PIR bit). I.e. the
> >>>>>highest priority interrupt should still have a vector >= pt_vector.
> >>>>
> >>>> I have noticed that pt_vector was 0x38 and the hightest vector was 0x30
> >>>> when the assertion failed. In the process of debugging pt_update_irq()
> >>>> when I was working on another feature, I noticed that 0x30 is always
> >>>> the vector of IRQ2. I suspect that the source of the periodic timer
> >> interrupt
> >>>> is not lapic. If that, pt_update_irq() reads the vioapic entry twice,
> >>>> one in hvm_isa_irq_assert() and the other in pt_irq_vector().
> >>>> If guest changed the content in viopaic entry between the two read
> >>>> (ie. change the vector from 0x30 to 0x38), the assertion would fail.
> >>>> Do you think it is a reasonable explanation?
> >>>
> >>>IRQ2? If this was going via the PIC, that would be impossible, as
> >>>that's the cascade pin. IRQ0, otoh, normally arrives at pin2 of the
> >>>IO-APIC (and we follow this model - see tools/libacpi/build.c). How
> >>
> >> Sorry, I meaned IRQ0. I was confused with IRQ0 and pin2 of the IO-APIC.
> >>
> >>>did you determine that 0x30 is the vector of IRQ2?
> >>
> >> By monitoring the write operation to IO-APIC redirection entry.
> >> From experimental data, linux kernel always writes vector 0x30 to
> >> redirection entry of pin2 of IO-APIC.
> >
> >But if it goes through the IO-APIC, it'll also go through the LAPIC
> >(and in the ExtInt case you wouldn't be able to read a valid vector
> >from the RTE).
> 
> Yes. If a pt interrupt goes through IO-APIC, pt_update_irq() will read
> the RTE twice. One is in hvm_isa_irq_assert() to determine which bit
> in vIRR should be set. The other is in pt_irq_vector() to get the bit
> we have set and return it. Is it possible that the return vector is
> different from the bit set in vIRR of LAPIC, since guest has changed
> RTE of IO-APIC pin2.
> 

In theory it could happen. There is no lock to prevent vIOAPIC RTE
being modified between above two operations. But I'm not sure
whether it's the reason for this bug. My feeling is that vector for IRQ0
should be fixed one. Did you capture such vector change in your side?

Thanks
Kevin

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.