From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] qemu-kvm: fix guest resetting Date: Fri, 09 May 2008 10:13:33 +0200 Message-ID: <482407AD.5080900@siemens.com> References: <4822B9EC.3020907@siemens.com> <20080508233528.GB6429@dmt> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel To: Marcelo Tosatti Return-path: In-Reply-To: <20080508233528.GB6429@dmt> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces@lists.sourceforge.net Errors-To: kvm-devel-bounces@lists.sourceforge.net List-Id: kvm.vger.kernel.org Marcelo Tosatti wrote: > Hi Jan, > > On Thu, May 08, 2008 at 10:29:32AM +0200, Jan Kiszka wrote: >> Resetting guests used to be racy, deadlock-prone, or simply broken (for >> SMP). This patch fixes the issues - at least for me on x86 (tested on >> Intel SMP host, UP and SMP guest, in-kernel und user space irqchip, >> guest- and monitor-issued resets). Note that ia64 and powerpc may need >> to look into the SMP thing as well (=>kvm_arch_cpu_reset). >> >> At this chance, the patch also cleans up some unneeded reset fragments. >> >> Signed-off-by: Jan Kiszka >> --- >> @@ -317,11 +317,18 @@ void qemu_kvm_system_reset_request(void) >> { >> int i; >> >> + pause_all_threads(); >> + >> + qemu_system_reset(); >> + >> + for (i = 0; i < smp_cpus; ++i) >> + kvm_arch_cpu_reset(vcpu_info[i].env); >> + >> for (i = 0; i < smp_cpus; ++i) { >> - vcpu_info[i].reload_regs = 1; >> + vcpu_info[i].stop = 0; >> + vcpu_info[i].stopped = 0; >> pthread_kill(vcpu_info[i].thread, SIG_IPI); >> } >> - qemu_system_reset(); > > Why don't you signal the IO thread to pause all vcpu's and place their > registers and "run state" in the proper condition if the reset request > comes from the guest? It should simplify things a lot (and avoid any > changes to vl.c). > > After signalling the vcpu should stop instead of returning to guest > mode. Hmm, need to think a bit more about it as I don't see the benefit yet (code suggestions are welcome in the meantime :)!). The changes to vl.c are actually cleanups, as pause_all_threads() is now context-agnostic and we no longer need to go through the qemu way of raising reset. This new property of pause_all_threads() is something we should desire for simplicity and robustness reasons anyway (e.g. to simplify guest debugging later on). And the way qemu_kvm_system_reset_request() is implemented would not change, we need this serialization to avoid races between IO and VCPU threads. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone