Re: [PATCH v3 04/10] qemu-kvm: Clean up mpstate synchronization

[Jan Kiszka <jan.kiszka@siemens.com>]

Jan Kiszka wrote:
> Marcelo Tosatti wrote:
>> On Wed, Feb 24, 2010 at 03:17:52PM +0100, Jan Kiszka wrote:
>>> Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86,
>>> properly synchronize with halted in the accessor functions.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> @@ -1290,6 +1318,7 @@ int kvm_arch_init_vcpu(CPUState *cenv)
>>>  #ifdef KVM_EXIT_TPR_ACCESS
>>>      kvm_tpr_vcpu_start(cenv);
>>>  #endif
>>> +    kvm_reset_mpstate(cenv);
>>>      return 0;
>>>  }
>>>  
>>> @@ -1363,15 +1392,10 @@ void kvm_arch_cpu_reset(CPUState *env)
>>>  {
>>>      kvm_arch_reset_vcpu(env);
>>>      kvm_put_vcpu_events(env);
>>> -    if (!cpu_is_bsp(env)) {
>>> -	if (kvm_irqchip_in_kernel()) {
>>> -#ifdef KVM_CAP_MP_STATE
>>> -	    kvm_reset_mpstate(env);
>>> -#endif
>>> -	} else {
>>> -	    env->interrupt_request &= ~CPU_INTERRUPT_HARD;
>>> -	    env->halted = 1;
>>> -	}
>>> +    kvm_reset_mpstate(env);
>>> +    if (!cpu_is_bsp(env) && !kvm_irqchip_in_kernel()) {
>>> +        env->interrupt_request &= ~CPU_INTERRUPT_HARD;
>>> +        env->halted = 1;
>>>      }
>>>  }
>> Why are these two needed? Now that initialization of mp_state 
>> happens via synchronize_state(init/reset) -> arch_load_regs?
> 
> Maybe correct. env->halted is also reset on load, not sure about the
> interrupt_request reset impact yet. This is (or at least was) hairy
> stuff, /me has to sleep about it again.

Regarding the !irqchip bits: env->halted is actually already reset in
apic_init_reset and can be dropped. I still wonder why I (was surprised
to find this out) once introduced the interrupt_request reset,
specifically why we may need it with KVM not not with TCG. Maybe we do,
and this should better be addressed generically.

But did you also ask about the need for kvm_reset_mpstate? That function
solely brings env->mp_state into the proper state after reset, something
that does not happen elsewhere.

Jan

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