On Mon, Jan 31, 2011 at 05:41:24PM +0100, Jan Kiszka wrote: > On 2011-01-31 17:38, Gleb Natapov wrote: > > On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote: > >> On 2011-01-31 14:04, Jan Kiszka wrote: > >>> On 2011-01-31 12:36, Jan Kiszka wrote: > >>>> On 2011-01-31 11:08, Avi Kivity wrote: > >>>>> On 01/27/2011 03:10 PM, Jan Kiszka wrote: > >>>>>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run > >>>>>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change > >>>>>> this service processes will first cause an exit from kvm_cpu_exec > >>>>>> anyway. And we will have to reenter the kernel on IO exits > >>>>>> unconditionally, something that the current logic prevents. > >>>>>> > >>>>>> Signed-off-by: Jan Kiszka<jan.kiszka@xxxxxxxxxxx> > >>>>>> --- > >>>>>> kvm-all.c | 11 ++++++----- > >>>>>> 1 files changed, 6 insertions(+), 5 deletions(-) > >>>>>> > >>>>>> diff --git a/kvm-all.c b/kvm-all.c > >>>>>> index 5bfa8c0..46ecc1c 100644 > >>>>>> --- a/kvm-all.c > >>>>>> +++ b/kvm-all.c > >>>>>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env) > >>>>>> > >>>>>> DPRINTF("kvm_cpu_exec()\n"); > >>>>>> > >>>>>> + if (kvm_arch_process_irqchip_events(env)) { > >>>>>> + env->exit_request = 0; > >>>>>> + env->exception_index = EXCP_HLT; > >>>>>> + return 0; > >>>>>> + } > >>>>>> + > >>>>>> do { > >>>>>> #ifndef CONFIG_IOTHREAD > >>>>>> if (env->exit_request) { > >>>>>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env) > >>>>>> } > >>>>> > >>>>> We check for ->exit_request here > >>>>> > >>>>>> #endif > >>>>>> > >>>>>> - if (kvm_arch_process_irqchip_events(env)) { > >>>>>> - ret = 0; > >>>>>> - break; > >>>>>> - } > >>>>>> - > >>>>> > >>>>> But this checks for ->interrupt_request. What ensures that we exit when > >>>>> ->interrupt_request is set? > >>>> > >>>> Good question, need to check again. But if that turns out to be an > >>>> issue, qemu-kvm would be broken as well. I'm just aligning the code here. > >>>> > >>> > >>> The only thing we miss by moving process_irqchip_events is a self-INIT > >>> of an AP - if such thing exists in real life. In that case, the AP would > >>> cause a reset of itself, followed by a transition to HALT state. > >> > >> I checked again with the Intel spec, and a self-INIT is invalid (at > >> least when specified via shorthand). So I'm under the impression now > >> that we can safely ignore this case and leave the patch as is. > >> > >> Any different views? > >> > > IIRC if you don't use shorthand you can send INIT to self. > > We didn't care so far (in qemu-kvm), do you think we should? > Doesn't kernel lapic emulation support this? -- Gleb. -- 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