From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] Activate Virtualization On Demand v2 Date: Wed, 05 Nov 2008 11:53:30 +0100 Message-ID: <49117B2A.5050903@suse.de> References: <1225874896-13186-1-git-send-email-agraf@suse.de> <49117015.7040902@redhat.com> <49117548.8030601@suse.de> <4911795A.6020807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, kraxel@redhat.com, anthony@codemonkey.ws, Sander.Vanleeuwen@sun.com, zach@vmware.com, brogers@novell.com To: Avi Kivity Return-path: Received: from mx1.suse.de ([195.135.220.2]:60463 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754276AbYKEKxd (ORCPT ); Wed, 5 Nov 2008 05:53:33 -0500 In-Reply-To: <4911795A.6020807@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > Alexander Graf wrote: > > > >>> We'll be in a nice fix if we can only enable virtualization on some >>> processors; that's the reason hardware_enable() was originally >>> specified as returning void. >>> >>> I don't see an easy way out, but it's hardly a likely event. >>> >> >> I don't think there's any way we can circumvent that. >> > > No. We can live with it though. > >> What I've wanted to ask for some time already: How does suspend/resume >> work? > > The question is important, even without the first word. > >> I only see one suspend/resume hook that disables virt on the >> currently running CPU. Why don't we have to loop through the CPUs to >> enable/disable all of them? >> At least for suspend-to-disk this sounds pretty necessary. >> >> > > Suspend first offlines all other cpus. Ah, ok. >>>> { >>>> - hardware_enable(NULL); >>>> + if (atomic_read(&kvm_usage_count)) >>>> + hardware_enable(NULL); >>>> return 0; >>>> } >>>> >>> Move the test to hardware_enable()? It's repeated too often. >>> >> >> What do we do about the on_each_cpu(hardware_enable) cases? We couldn't >> tell when to activate/deactive virtualization then, as that's >> semantically bound to "amount of VMs". >> > static int kvm_resume(struct sys_device *dev) > > I don't understand. Moving the test to within the IPI shouldn't > affect anything. Oh, you only want the test to be in hardware_enable and hardware_disable. Now I see what you mean: modify and lock kvm_usage_count outside, but test inside of hardware_enable.