From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 08/13] kvm: x86: Inject pending MCE events on state writeback Date: Thu, 17 Feb 2011 19:04:51 +0100 Message-ID: <4D5D6343.6060508@siemens.com> References: <56bf7460c65f7fc5c89fa9673b880a556b99e0fc.1297758211.git.jan.kiszka@siemens.com> <20110217163534.GB10918@amt.cnet> <4D5D558B.10406@siemens.com> <20110217175559.GA12113@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , Huang Ying , Hidetoshi Seto , Jin Dongming To: Marcelo Tosatti Return-path: Received: from thoth.sbs.de ([192.35.17.2]:30368 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757505Ab1BQSF1 (ORCPT ); Thu, 17 Feb 2011 13:05:27 -0500 In-Reply-To: <20110217175559.GA12113@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: 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