Re: [PATCH 08/13] kvm: x86: Inject pending MCE events on state writeback

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

On 2011-02-17 18:55, Marcelo Tosatti wrote:
>>>> @@ -1375,10 +1413,25 @@ static int kvm_put_vcpu_events(CPUState *env, int level)
>>>>          return 0;
>>>>      }
>>>>  
>>>> -    events.exception.injected = (env->exception_injected >= 0);
>>>> -    events.exception.nr = env->exception_injected;
>>>> -    events.exception.has_error_code = env->has_error_code;
>>>> -    events.exception.error_code = env->error_code;
>>>> +    if (env->interrupt_request & CPU_INTERRUPT_MCE) {
>>>> +        /* We must not raise CPU_INTERRUPT_MCE if it's not supported. */
>>>> +        assert(env->mcg_cap);
>>>> +
>>>> +        env->interrupt_request &= ~CPU_INTERRUPT_MCE;
>>>> +        if (env->exception_injected == EXCP08_DBLE) {
>>>> +            /* this means triple fault */
>>>> +            qemu_system_reset_request();
>>>> +            env->exit_request = 1;
>>>> +        }
>>>> +        events.exception.injected = 1;
>>>> +        events.exception.nr = EXCP12_MCHK;
>>>> +        events.exception.has_error_code = 0;
>>>> +    } else {
>>>> +        events.exception.injected = (env->exception_injected >= 0);
>>>> +        events.exception.nr = env->exception_injected;
>>>> +        events.exception.has_error_code = env->has_error_code;
>>>> +        events.exception.error_code = env->error_code;
>>>> +    }
>>>
>>> IMO it is important to maintain a scope for kvm_put_vcpu_events /
>>> kvm_get_vcpu_events: they synchronize state to/from the kernel. Not more
>>> than that. Whatever you're trying to do here should be higher in the
>>> vcpu loop code.
>>
>> We pick up CPU_INTERRUPT_MCE and translate it into the right exception
>> that put_vcpu_events is about to sync to the kernel. What should be done
>> earlier of those steps? Calculating env->exception_injected?
> 
> Everything but writeback. Update env->exception_injected/nr in
> process_irqchip_events, or in a separate kvm_arch_update_exceptions.
> 

OK, will rework this.

>>>>          return ret;
>>>> @@ -1678,10 +1736,17 @@ void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
>>>>  int kvm_arch_process_irqchip_events(CPUState *env)
>>>>  {
>>>>      if (kvm_irqchip_in_kernel()) {
>>>> +        if (env->interrupt_request & CPU_INTERRUPT_MCE) {
>>>> +            kvm_cpu_synchronize_state(env);
>>>> +            if (env->mp_state == KVM_MP_STATE_HALTED) {
>>>> +                env->mp_state = KVM_MP_STATE_RUNNABLE;
>>>> +            }
>>>> +        }
>>>
>>> Should not manipulate mp_state of a running vcpu (should only do that
>>> for migration when vcpu is stopped), since its managed by the kernel,
>>> for irqchip case.
>>
>> Not for asynchronously injected MCEs. The target CPU would simply
>> oversleep them. MCEs are not in the scope of the in-kernel irqchip.
> 
> Pending MCE exception could break out of in-kernel halt emulation.

Can't follow. What do you mean? That the kernel already takes care? I
didn't find a trace, so I added that code.

Jan

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