On 2011-02-02 16:09, Avi Kivity wrote: > On 02/02/2011 04:52 PM, Jan Kiszka wrote: >> On 2011-02-02 15:43, Jan Kiszka wrote: >>> On 2011-02-02 15:35, Avi Kivity wrote: >>>> On 02/02/2011 04:30 PM, Jan Kiszka wrote: >>>>> On 2011-02-02 14:05, Avi Kivity wrote: >>>>>> On 02/02/2011 02:50 PM, Jan Kiszka wrote: >>>>>>>>> >>>>>>>> Opps, -smp 1. With -smp 2 it boot almost completely and then hangs. >>>>>>> >>>>>>> Ah, good (or not good). With Windows 2003 Server, I actually get a Blue >>>>>>> Screen (Stop 0x000000b8). >>>>>> >>>>>> Userspace APIC is broken since it may run with an outdated cr8, does >>>>>> reverting 27a4f7976d5 help? >>>>> >>>>> Can you elaborate on what is broken? The way hw/apic.c maintains the >>>>> tpr? Would it make sense to compare this against the in-kernel model? Or >>>>> do you mean something else? >>>> >>>> The problem, IIRC, was that we look up the TPR but it may already have >>>> been changed by the running vcpu. Not 100% sure. >>>> >>>> If that is indeed the problem then the fix would be to process the APIC >>>> in vcpu context (which is what the kernel does - we set a bit in the IRR >>>> and all further processing is synchronous). >>> >>> You mean: user space changes the tpr value while the vcpu is in KVM_RUN, >>> then we return from the kernel and overwrite the tpr in the apic with >>> the vcpu's view, right? >> >> Hmm, probably rather that there is a discrepancy between tpr and irr. >> The latter is changed asynchronously /wrt to the vcpu, the former /wrt >> the user space device model. > > And yet, both are synchronized via qemu_mutex. So we're still missing > something in this picture. > >> Run apic_set_irq on the vcpu? > > static void apic_set_irq(APICState *s, int vector_num, int trigger_mode) > { > apic_irq_delivered += !get_bit(s->irr, vector_num); > > trace_apic_set_irq(apic_irq_delivered); > > set_bit(s->irr, vector_num); > > This is even more async with kernel irqchip > > if (trigger_mode) > set_bit(s->tmr, vector_num); > else > reset_bit(s->tmr, vector_num); > > This is protected by qemu_mutex > > apic_update_irq(s); > > This will be run the next time the vcpu exits, via apic_get_interrupt(). The decision to pend an IRQ (and potentially kick the vcpu) takes place immediately in acip_update_irq. And it is based on current irr as well as tpr. But we update again when user space returns with a new value. > > } > > Did you check whether reverting that commit helps? > Just did so, and I can no longer reproduce the problem. Hmm... Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html