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.kis...@siemens.com>
>>>> ---
>>>>   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?

Jan

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

Reply via email to