From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57371) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHiDE-00057N-Tu for qemu-devel@nongnu.org; Wed, 13 Aug 2014 19:44:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHiD8-0000Lq-OS for qemu-devel@nongnu.org; Wed, 13 Aug 2014 19:44:12 -0400 Message-ID: <53EBF842.9080604@redhat.com> Date: Thu, 14 Aug 2014 01:44:02 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <20140813190916.12400.59842.stgit@gimli.home> <53EBCB9C.70801@redhat.com> <1407967589.9800.241.camel@ul30vt.home> <53EBF211.1030701@redhat.com> In-Reply-To: <53EBF211.1030701@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] x86: Reset MTRR on vCPU reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, qemu-stable@nongnu.org On 08/14/14 01:17, Laszlo Ersek wrote: > - With KVM, the lack of loading MTRR state from KVM, combined with the > (partial) storing of MTRR state to KVM, has two consequences: > - migration invalidates (loses) MTRR state, I'll concede that migration *already* loses MTRR state (on KVM), even before your patch. On the incoming host, the difference is that pre-patch, the guest continues running (after migration) with MTRRs in the "initial" KVM state, while post-patch, the guest continues running after an explicit zeroing of the variable MTRR masks and the deftype. I admit that it wouldn't be right to say that the patch "causes" MTRR state loss. With that, I think I've actually convinced myself that your patch is correct: The x86_cpu_reset() hunk is correct in any case, independently of KVM vs. TCG. (On TCG it even improves MTRR conformance.) Splitting that hunk into a separate patch might be worthwhile, but not overly important. The kvm_put_msrs() hunk forces a zero write to the variable MTRR PhysMasks and the DefType, on both reset and on incoming migration. For reset, this is correct behavior. For incoming migration, it is not, but it certainly shouldn't qualify as a regression, relative to the current status (where MTRR state is simply lost and replaced with initial MTRR state on the incoming host). I think the above "end results" could be expressed more clearly in the code, but I'm already wondering if you'll ever talk to me again, so I'm willing to give my R-b if you think that's useful... :) (Again, I might be wrong, easily.) Thanks Laszlo