From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37018) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XI2V5-0008Vi-56 for qemu-devel@nongnu.org; Thu, 14 Aug 2014 17:24:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XI2Uy-0005GC-50 for qemu-devel@nongnu.org; Thu, 14 Aug 2014 17:23:59 -0400 Message-ID: <53ED28E5.9040305@redhat.com> Date: Thu, 14 Aug 2014 23:23:49 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <20140814191147.13303.61655.stgit@gimli.home> <20140814192415.13303.34846.stgit@gimli.home> In-Reply-To: <20140814192415.13303.34846.stgit@gimli.home> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 3/3] x86: Clear MTRRs on vCPU reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson , qemu-devel@nongnu.org, kvm@vger.kernel.org Cc: qemu-stable@nongnu.org On 08/14/14 21:24, Alex Williamson wrote: > The SDM specifies (June 2014 Vol3 11.11.5): > > On a hardware reset, the P6 and more recent processors clear the > valid flags in variable-range MTRRs and clear the E flag in the > IA32_MTRR_DEF_TYPE MSR to disable all MTRRs. All other bits in the > MTRRs are undefined. > > We currently do none of that, so whatever MTRR settings you had prior > to reset is what you have after reset. Usually this doesn't matter > because KVM often ignores the guest mappings and uses write-back > anyway. However, if you have an assigned device and an IOMMU that > allows NoSnoop for that device, KVM defers to the guest memory > mappings which are now stale after reset. The result is that OVMF > rebooting on such a configuration takes a full minute to LZMA > decompress the firmware volume, a process that is nearly instant on > the initial boot. > > Signed-off-by: Alex Williamson > Cc: Laszlo Ersek > Cc: qemu-stable@nongnu.org > --- > > target-i386/cpu.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 6d008ab..9768be1 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2588,6 +2588,16 @@ static void x86_cpu_reset(CPUState *s) > > env->xcr0 = 1; > > + /* > + * SDM 11.11.5 requires: > + * - IA32_MTRR_DEF_TYPE MSR.E = 0 > + * - IA32_MTRR_PHYSMASKn.V = 0 > + * All other bits are undefined. For simplification, zero it all. > + */ > + env->mtrr_deftype = 0; > + memset(env->mtrr_var, 0, sizeof(env->mtrr_var)); > + memset(env->mtrr_fixed, 0, sizeof(env->mtrr_fixed)); > + > #if !defined(CONFIG_USER_ONLY) > /* We hard-wire the BSP to the first CPU. */ > if (s->cpu_index == 0) { > I like this heavy-handed approach. Reviewed-by: Laszlo Ersek