Re: [Qemu-devel] [PATCH uq/master 2/2] kvm: forward INIT signals coming from the chipset

[Gleb Natapov <gleb@redhat.com>]

On Sun, Mar 10, 2013 at 03:28:06PM +0100, Paolo Bonzini wrote:
> Il 10/03/2013 12:54, Gleb Natapov ha scritto:
> > On Sat, Mar 09, 2013 at 07:48:50AM +0100, Paolo Bonzini wrote:
> >> CPU_INTERRUPT_INIT can also be generated if you have an internal APIC,
> >> since the keyboard controller and the southbridge can also pulse the
> >> CPU's INIT# pin.
> >>
> >> Exit the VCPU is one is received, and process it by changing the
> >> mp_state to KVM_MP_STATE_INIT_RECEIVED.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>  target-i386/kvm.c | 34 +++++++++++++++++++++++-----------
> >>  1 file changed, 23 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 0cf413d..56de77c 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -1772,14 +1772,15 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
> >>          }
> >>      }
> >>  
> >> -    if (!kvm_irqchip_in_kernel()) {
> >> -        /* Force the VCPU out of its inner loop to process any INIT requests
> >> -         * or pending TPR access reports. */
> >> -        if (env->interrupt_request &
> >> -            (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
> >> -            cpu->exit_request = 1;
> >> -        }
> >> +    /* Force the VCPU out of its inner loop to process any INIT requests
> >> +     * or (for userspace APIC, but it is cheap to combine the checks here)
> >> +     * pending TPR access reports.
> >> +     */
> >> +    if (env->interrupt_request & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
> >> +        cpu->exit_request = 1;
> >> +    }
> >>  
> >> +    if (!kvm_irqchip_in_kernel()) {
> >>          /* Try to inject an interrupt if the guest can accept it */
> >>          if (run->ready_for_interrupt_injection &&
> >>              (env->interrupt_request & CPU_INTERRUPT_HARD) &&
> >> @@ -1835,6 +1836,7 @@ int kvm_arch_process_async_events(CPUState *cs)
> >>  {
> >>      X86CPU *cpu = X86_CPU(cs);
> >>      CPUX86State *env = &cpu->env;
> >> +    int ret;
> >>  
> >>      if (env->interrupt_request & CPU_INTERRUPT_MCE) {
> >>          /* We must not raise CPU_INTERRUPT_MCE if it's not supported. */
> >> @@ -1859,6 +1861,20 @@ int kvm_arch_process_async_events(CPUState *cs)
> >>          }
> >>      }
> >>  
> >> +    if (env->interrupt_request & CPU_INTERRUPT_INIT) {
> >> +        kvm_cpu_synchronize_state(env);
> >> +        if (kvm_irqchip_in_kernel()) {
> >> +            env->mp_state = KVM_MP_STATE_INIT_RECEIVED;
> >> +            env->interrupt_request = 0;
> >> +            ret = kvm_put_mp_state(cpu);
> >> +            if (ret < 0) {
> >> +                return ret;
> >> +            }
> >> +        } else {
> >> +            do_cpu_init(cpu);
> >> +        }
> >> +    }
> >> +
> > Why not move INIT case from below as is? Vcpu is reset to correct sate
> > by QEMU just like during system_reset.
> 
> APs would not be able to receive SIPIs after executing do_cpu_init,
> because they would stay in KVM_MP_STATE_RUNNABLE state.
> 
If APs are in runnable state after reset with in kernel irq chip we
have a bug somewhere. Should AP be able to get SIPI without INIT after
trigger of INIT# line?

--
			Gleb.