From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: x86: kvmclock: abstract save/restore sched_clock_state (v2) Date: Wed, 15 Feb 2012 12:28:33 +0200 Message-ID: <4F3B88D1.5070405@redhat.com> References: <20120207210542.GC20618@amt.cnet> <20120213130727.GA8052@amt.cnet> <4F392A38.4030808@redhat.com> <20120213155207.GA10105@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Igor Mammedov , kvm@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, x86@kernel.org, johnstul@us.ibm.com, riel@redhat.com, amit.shah@redhat.com To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:64357 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752167Ab2BOK2m (ORCPT ); Wed, 15 Feb 2012 05:28:42 -0500 In-Reply-To: <20120213155207.GA10105@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On 02/13/2012 05:52 PM, Marcelo Tosatti wrote: > > > { > > >+ x86_platform.restore_sched_clock_state(); > > Isn't it too early? It is scarry to say hypervisor to write to some > > memory location and than completely replace page-tables and half of > > cpu state in __restore_processor_state. Wouldn't that have a potential > > of writing into a place that is not restored hv_clock and restored > > hv_clock might still be stale? > > No, memory is copied in swsusp_arch_resume(), which happens > before restore_processor_state. restore_processor_state() is only > setting up registers and MTRR. > In addition, kvmclock uses physical addresses, so page table changes don't matter. Note we could have done this in __save_processor_state()/__restore_processor_state() (it's just reading and writing an MSR, like we do for MSR_IA32_MISC_ENABLE), but I think your patch is the right way. I'd like an ack from the x86 maintainers though. -- error compiling committee.c: too many arguments to function