Re: qemu-kvm requires apic initialized before vcpu main loop

[Glauber Costa <glommer@redhat.com>]

On Wed, Dec 09, 2009 at 06:21:38PM -0200, Marcelo Tosatti wrote:
> On Wed, Dec 09, 2009 at 08:00:41PM +0100, Jan Kiszka wrote:
> > Glauber Costa wrote:
> > > On Wed, Dec 09, 2009 at 03:46:54PM -0200, Marcelo Tosatti wrote:
> > >> Otherwise a zero apic base is loaded into KVM, which results
> > >> in interrupts being lost until a proper apic base with enabled 
> > >> bit set is loaded.
> > >>
> > >> Fixes WinXP migration in qemu-kvm origin/next.
> > >>
> > >> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > >>
> > >> diff --git a/hw/apic.c b/hw/apic.c
> > >> index 627ff98..45a4d2b 100644
> > >> --- a/hw/apic.c
> > >> +++ b/hw/apic.c
> > >> @@ -1131,6 +1131,11 @@ int apic_init(CPUState *env)
> > >>      vmstate_register(s->idx, &vmstate_apic, s);
> > >>      qemu_register_reset(apic_reset, s);
> > >>  
> > >> +    /* apic_reset must be called before the vcpu threads are initialized and load 
> > >> +     * registers, in qemu-kvm.
> > >> +     */
> > >> +    apic_reset(s);
> > >> +
> > > But by doing this, the system-wide reset will re-reset the apic, possibly losing
> > > some other information.
> > > 
> > > Also, system_reset happens before we signal system_ready (or at least should).
> > > This means the vcpus should not be running and producing anything useful yet.
> > > So how does it happen, in the first place?
> > 
> > There is
> > 
> > 	kvm_arch_load_regs(env);
> > 
> > before qemu_cond_wait in ap_main_loop. Probably part of the reason. Why
> > is it there?
> 
> Hum ... see how qemu_kvm_load_lapic depends on kvm_vcpu_inited.
> 
> kvm_vcpu_ioctl_set_lapic -> kvm_apic_post_state_restore relies on
> proper apicbase set (maybe other reasons too).

Have you tried getting rid of kvm_vcpu_inited()? Now that we are doing reset after vcpu creation,
it is quite possible that it won't be needed anymore.

Btw, the whole point of this exercise is to try diminishing oportunities
for nasty things like that.