From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH] Guest stop notification Date: Thu, 1 Dec 2011 19:25:03 -0200 Message-ID: <20111201212503.GB25290@amt.cnet> References: <1322602574-27072-1-git-send-email-emunson@mgebm.net> <4ED79138.60603@siemens.com> <20111201172244.GB8803@mgebm.net> <4ED7BB11.1050309@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "ryanh@linux.vnet.ibm.com" , "aliguori@us.ibm.com" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "qemu-devel@nongnu.org" , Eric B Munson , "avi@redhat.com" To: Jan Kiszka Return-path: Content-Disposition: inline In-Reply-To: <4ED7BB11.1050309@siemens.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org On Thu, Dec 01, 2011 at 06:36:17PM +0100, Jan Kiszka wrote: > On 2011-12-01 18:22, Eric B Munson wrote: > > On Thu, 01 Dec 2011, Jan Kiszka wrote: > > > >> On 2011-11-29 22:36, Eric B Munson wrote: > >>> Often when a guest is stopped from the qemu console, it will report spurious > >>> soft lockup warnings on resume. There are kernel patches being discussed that > >>> will give the host the ability to tell the guest that it is being stopped and > >>> should ignore the soft lockup warning that generates. > >>> > >>> Signed-off-by: Eric B Munson > >>> Cc: ryanh@linux.vnet.ibm.com > >>> Cc: aliguori@us.ibm.com > >>> Cc: mtosatti@redhat.com > >>> Cc: avi@redhat.com > >>> Cc: kvm@vger.kernel.org > >>> Cc: linux-kernel@vger.kernel.org > >>> --- > >>> target-i386/kvm.c | 6 ++++++ > >>> 1 files changed, 6 insertions(+), 0 deletions(-) > >>> > >>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c > >>> index 5bfc21f..defd364 100644 > >>> --- a/target-i386/kvm.c > >>> +++ b/target-i386/kvm.c > >>> @@ -336,12 +336,18 @@ static int kvm_inject_mce_oldstyle(CPUState *env) > >>> return 0; > >>> } > >>> > >>> +static void kvm_put_guest_paused(CPUState *penv) > >>> +{ > >>> + kvm_vcpu_ioctl(penv, KVM_GUEST_PAUSED, 0); > >>> +} > >> > >> I see no need in encapsulating this in a separate function. > >> > >>> + > >>> static void cpu_update_state(void *opaque, int running, RunState state) > >>> { > >>> CPUState *env = opaque; > >>> > >>> if (running) { > >>> env->tsc_valid = false; > >>> + kvm_put_guest_paused(env); > >> > >> checkpatch.pl would have asked you to remove this tab. > >> > >> More general: > >> > >> Why is this x86-only? If the kernel interface is x86-only, what prevents > >> making it generic right from the beginning? > > > > Sorry, missed this question on the first pass, this is x86 only because the > > flag used lives in the pvclock structure. AFAICT, there aren't any other > > architectures out there that implement paravirtualized clocks yet. > > That's an implementation "detail" of the kernel. The interface (IOCTL or > kvm_run field) is generic, no? > > I would just fire this notification from generic code, evaluate the > error (that was lacking so far), and only report it if it's something > else than "not supported". Yes, it should live in hw/kvmclock.c preferably.