On Fri, Oct 17, 2008 at 07:35:01PM +0200, Jan Kiszka wrote:
> Sheng Yang wrote:
> > On Fri, Oct 17, 2008 at 10:10:54AM +0200, Jan Kiszka wrote:
> >> Sheng Yang wrote:
> >>> On Wednesday 15 October 2008 22:27:49 Jan Kiszka wrote:
> >>>> Aligning in-kernel kvm_apic_accept_pic_intr with its user space mate,
> >>>> this patch relaxes the conditions under which PIC IRQs are accepted
> >>>> by LVT0. This reflects reality and allows to reuse the service for the
> >>>> NMI watchdog use case.
> >>>>
> >>>> Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]>
> >>>> ---
> >>>> arch/x86/kvm/lapic.c | 13 ++++---------
> >>>> 1 file changed, 4 insertions(+), 9 deletions(-)
> >>>>
> >>>> Index: b/arch/x86/kvm/lapic.c
> >>>> ===================================================================
> >>>> --- a/arch/x86/kvm/lapic.c
> >>>> +++ b/arch/x86/kvm/lapic.c
> >>>> @@ -1072,16 +1072,11 @@ int kvm_apic_has_interrupt(struct kvm_vc
> >>>> int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu)
> >>>> {
> >>>> u32 lvt0 = apic_get_reg(vcpu->arch.apic, APIC_LVT0);
> >>>> - int r = 0;
> >>>>
> >>>> - if (vcpu->vcpu_id == 0) {
> >>>> - if (!apic_hw_enabled(vcpu->arch.apic))
> >>>> - r = 1;
> >>>> - if ((lvt0 & APIC_LVT_MASKED) == 0 &&
> >>>> - GET_APIC_DELIVERY_MODE(lvt0) == APIC_MODE_EXTINT)
> >>>> - r = 1;
> >>>> - }
> >>>> - return r;
> >>>> + if (!apic_hw_enabled(vcpu->arch.apic) ||
> >>>> + (lvt0 & APIC_LVT_MASKED) == 0)
> >>>> + return 1;
> >>>> + return 0;
> >>>> }
> >>>>
> >>>> void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
> >>>>
> >>> (sorry for late review...)
> >>>
> >>> Thanks to find out the root cause of BSOD!
> >>>
> >>> But I am a little concern about this change. As you know, PIC only
> >>> connect to
> >>> cpu0. So I think it's not proper to make it generic.
> >> I don't think so - and if it were true, qemu would have a bug then, see
> >> its corresponding code.
> >
> > You can refer to Intel MP spec, virtual wire mode. Google
> > "MP spec" can find it.
>
> Ah, good reference.
>
> >
> > Normally PIC is only used in BSP boot up for SMP guest(PIC can't afford SMP,
> > otherwise we won't need IOAPIC/LAPIC). After that, it should be disabled.
> > And virtual wire mode works with APIC_MODE_EXTINT on LVT0 of BSP lapic, so
> > that's why you see
> >
> > GET_APIC_DELIVERY_MODE(lvt0) == APIC_MODE_EXTINT
> >
> > KVM follow virtual wire mode exactly.
>
> According to my understanding of the spec, the virtual wire mode means
> that the PIC signal is delivered via LVT0, and thus can be received by
> _all_ CPUs in the system. However, only the BSP usually enables LVT0,
> thus is receiving the IRQ. When Linux switches to NMI watchdog mode 1,
> it also unmasks the other CPUs (and reprograms all to deliver NMIs
> instead of EXTINTs).
>
> Then there is also the "PIC Mode", ie. direct delivery to the BSP, and
> only the latter. That mode is obviously target by the current
> kvm_apic_accept_pic_intr implementation. But I find no indication in the
> spec yet that both modes cannot exists in the same system. But I also
> fail to understand how one could switch between both modes (via software).
No. If so, we don't need to check LVT0.
--
regards
Yang, Sheng
>
> >
> > For QEmu, it just check if lapic LVT0 is masked, and don't check vcpu0.
> > That's indeed a little problematic, for it's not that sufficient to
> > determine if it's programmed as virtual wire mode and used for deliver
> > interrupts from PIC. Well, in most condition, it can work. But maybe
> > it's not clean in logic.
> >
> > For NMI watchdog here, we use a little more tricky way other than normal
> > PIC/LAPIC interaction. IIRC, NMI watchdog don't mask PIC after enable
> > IOAPIC, it also don't mask LVT0 of every LAPIC. It use physical connection
> > of PIT to PIC then to LAPIC LVT0 to send NMI. Program LVT0 to NMI, then
> > every PIT interrupt would go through PIC, arrive at LVT0, trig a NMI.
> >
> > So I think the key problem for Windows is, they don't need it, but we send
> > the NMIs. We send the NMI when LVT0 is masked. Base on this, I think your
> > optimize patch also can resolve this issue? It's already including necessary
> > judgment. We will try it next week.
>
> The key problem for Windows was most probably not NMI, but the fact that
> we forwarded _any_ PIC IRQ (emulating virtual wire mode) without
> checking for the LAPICs' mask state.
>
> OK, this requires a few more thoughts and a bit more reading.
>
> Jan
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
