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. Jan [1] http://thread.gmane.org/gmane.comp.emulators.qemu/100806
Attachment:
signature.asc
Description: OpenPGP digital signature
