| Message ID | 1362811730-6716-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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. > if (kvm_irqchip_in_kernel()) { > return 0; > } > @@ -1872,10 +1888,6 @@ int kvm_arch_process_async_events(CPUState *cs) > (env->interrupt_request & CPU_INTERRUPT_NMI)) { > env->halted = 0; > } > - if (env->interrupt_request & CPU_INTERRUPT_INIT) { > - kvm_cpu_synchronize_state(env); > - do_cpu_init(cpu); > - } > if (env->interrupt_request & CPU_INTERRUPT_SIPI) { > kvm_cpu_synchronize_state(env); > do_cpu_sipi(cpu); > -- > 1.8.1.4 -- Gleb.
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. Paolo >> if (kvm_irqchip_in_kernel()) { >> return 0; >> } >> @@ -1872,10 +1888,6 @@ int kvm_arch_process_async_events(CPUState *cs) >> (env->interrupt_request & CPU_INTERRUPT_NMI)) { >> env->halted = 0; >> } >> - if (env->interrupt_request & CPU_INTERRUPT_INIT) { >> - kvm_cpu_synchronize_state(env); >> - do_cpu_init(cpu); >> - } >> if (env->interrupt_request & CPU_INTERRUPT_SIPI) { >> kvm_cpu_synchronize_state(env); >> do_cpu_sipi(cpu); >> -- >> 1.8.1.4 > > > -- > Gleb. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
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.
Il 10/03/2013 15:55, Gleb Natapov ha scritto: > > > 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. Here is where we are resetting the processor. After clearing CPU_INTERRUPT_INIT, no matter what else we do (such as resetting the APIC and CPU), we need to set the mp_state to KVM_MP_STATE_INIT_RECEIVED. Or if we go with your simpler hypervisor patch, we need to go to either KVM_MP_STATE_INIT_RECEIVED for APs (wait for SIPI) or KVM_MP_STATE_SIPI_RECEIVED for the BSP (restart running from the reset vector). > Should AP be able to get SIPI without INIT after trigger of INIT# line? Yes, the effect is the same for an INIT interrupt and the triggering of INIT#. Paolo
On Sun, Mar 10, 2013 at 04:04:39PM +0100, Paolo Bonzini wrote: > Il 10/03/2013 15:55, Gleb Natapov ha scritto: > > > > 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. > > Here is where we are resetting the processor. After clearing > CPU_INTERRUPT_INIT, no matter what else we do (such as resetting the > APIC and CPU), we need to set the mp_state to KVM_MP_STATE_INIT_RECEIVED. > > Or if we go with your simpler hypervisor patch, we need to go to either > KVM_MP_STATE_INIT_RECEIVED for APs (wait for SIPI) or > KVM_MP_STATE_SIPI_RECEIVED for the BSP (restart running from the reset > vector). > No need for KVM_MP_STATE_SIPI_RECEIVED. Just make it RUNNING. This is similar to system_reset path, not? UNINIT for AP, RUNNING for BSP. > > Should AP be able to get SIPI without INIT after trigger of INIT# line? > > Yes, the effect is the same for an INIT interrupt and the triggering of > INIT#. > Can you give me SDM pointer? -- Gleb.
Il 10/03/2013 16:24, Gleb Natapov ha scritto: > On Sun, Mar 10, 2013 at 04:04:39PM +0100, Paolo Bonzini wrote: >> Il 10/03/2013 15:55, Gleb Natapov ha scritto: >>>>> 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. >> >> Here is where we are resetting the processor. After clearing >> CPU_INTERRUPT_INIT, no matter what else we do (such as resetting the >> APIC and CPU), we need to set the mp_state to KVM_MP_STATE_INIT_RECEIVED. >> >> Or if we go with your simpler hypervisor patch, we need to go to either >> KVM_MP_STATE_INIT_RECEIVED for APs (wait for SIPI) or >> KVM_MP_STATE_SIPI_RECEIVED for the BSP (restart running from the reset >> vector). >> > No need for KVM_MP_STATE_SIPI_RECEIVED. Just make it RUNNING. This is similar to > system_reset path, not? UNINIT for AP, RUNNING for BSP. > >>> Should AP be able to get SIPI without INIT after trigger of INIT# line? >> >> Yes, the effect is the same for an INIT interrupt and the triggering of >> INIT#. >> > Can you give me SDM pointer? 10.4.7.3 Local APIC State After an INIT Reset (“Wait-for-SIPI” State) An INIT reset of the processor can be initiated in either of two ways: • By asserting the processor’s INIT# pin. • By sending the processor an INIT IPI (an IPI with the delivery mode set to INIT). Upon receiving an INIT through either of these mechanisms, the processor responds by beginning the initialization process of the processor core and the local APIC. The state of the local APIC following an INIT reset is the same as it is after a power-up or hardware RESET, except that the APIC ID and arbitration ID registers are not affected. This state is also referred to at the “wait-for-SIPI” state (see also: Section 8.4.2, “MP Initialization Protocol Requirements and Restrictions”). Paolo
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); + } + } + if (kvm_irqchip_in_kernel()) { return 0; } @@ -1872,10 +1888,6 @@ int kvm_arch_process_async_events(CPUState *cs) (env->interrupt_request & CPU_INTERRUPT_NMI)) { env->halted = 0; } - if (env->interrupt_request & CPU_INTERRUPT_INIT) { - kvm_cpu_synchronize_state(env); - do_cpu_init(cpu); - } if (env->interrupt_request & CPU_INTERRUPT_SIPI) { kvm_cpu_synchronize_state(env); do_cpu_sipi(cpu);
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(-)