[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop

[Gleb Natapov]

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<address@hidden>
> >>>>>> ---
> >>>>>>   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.