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

[Jan Kiszka]

On 2011-01-31 17:41, 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?

...and the kernel model should have barked "INIT on a runnable vcpu" in
such cases (BTW, that's user triggerable and should likely be rate limited).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux