On 10/6/11, Jan Kiszka <jan.kiszka@xxxxxx> wrote: > On 2011-10-06 03:13, liu ping fan wrote: >> On Wed, Oct 5, 2011 at 7:01 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote: >>> On 2011-10-05 12:26, liu ping fan wrote: >>>>> > And make the creation of apic as part of cpu initialization, so >>>>>> apic's state has been ready, before setting kvm_apic. >>>>> >>>>> There is no kvm-apic upstream yet, so it's hard to judge why we need >>>>> this here. If we do, this has to be a separate patch. But I seriously >>>>> doubt we need it (my hack worked without it, and that was not because >>>>> of >>>>> its hack nature). >>>>> >>>>> Sorry, I did not explain it clearly. What I mean is that >>>>> “env->apic_state” >>>> must be prepared >>>> before qemu_kvm_cpu_thread_fn() -> ... -> kvm_put_sregs(), where we get >>>> apic_base by >>>> “ sregs.apic_base = cpu_get_apic_base(env->apic_state);” >>>> and then call “kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);” which will >>>> finally affect the >>>> kvm_apic structure in kernel. >>>> >>>> But as current code, in pc_new_cpu(), we call apic_init() to initialize >>>> apic_state, after cpu_init(), >>>> so we can not guarantee the order of apic_state initializaion and the >>>> setting to kernel. >>>> >>>> Because LAPIC is part of x86 chip, I want to move it into >>>> cpu_x86_init(), >>>> and ensure apic_init() >>>> called before thread “qemu_kvm_cpu_thread_fn()” creation. >>> >>> The LAPIC is part of the CPU, the classic APIC was a dedicated chip. >> Sorry, a little puzzle. I think x86 interrupt system consists of two >> parts: IOAPIC/LAPIC. >> For we have "hw/ioapic.c" to simulate IOAPIC, I think "hw/apic.c" >> takes the role as LAPIC, >> especially that we create an APICState instance for each CPUX86State, >> just like each LAPIC >> for x86 CPU in real machine. >> So we can consider apic_init() to create a LAPIC instance, rather than >> create a "classic APIC"? >> >> I guess If there is lack of something in IOAPIC/LAPIC bus topology, >> that will be the arbitrator of ICC bus, right? >> So "the classic APIC was a dedicated chip" what you said, play this >> role, right? >> Could you tell me a sample chipset of APIC, and I can increase my >> knowledge about it, thanks. > > The 82489DX was used as a discrete APIC on 486 SMP systems. > >> >>> >>> For various reasons, a safer approach for creating a new CPU is to stop >>> the machine, add the new device models, run cpu_synchronize_post_init on >>> that new cpu (looks like you missed that) and then resume everything. >>> See >>> http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=be8f21c6b54eac82f7add7ee9d4ecf9cb8ebb320 >>> >> Great job. And I am interesting about it. Could you give some sample >> reason about why we need to stop >> the machine, or list all of the reasons, so we can resolve it one by >> one. I can not figure out such scenes by myself. >> From my view, especially for KVM, the creation of vcpu are protected >> well by lock mechanism from other >> vcpu threads in kernel, so we need not to stop all of the threads. > > Maybe I was seeing ghosts: I thought that there is a race window between > VCPU_CREATE and the last initialization IOCTL when we allow other VCPUs > to interact with the new one already. However, I do not find the > scenario again ATM. > > But if you want to move the VCPU resource initialization completely over > the VCPU thread, you also have to handle env->halted in that context. > See [1] for this topic and associated todos. > > And don't forget the cpu_synchronize_post_init. Running this after each > VCPU creation directly should also obsolete cpu_synchronize_all_post_init. Thanks, Jan. I will dig into this and follow the thread to see what to do in next step Regards, ping fan > > Jan > > [1] http://thread.gmane.org/gmane.comp.emulators.qemu/100806 > > -- 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
