From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laszlo Ersek Subject: Re: [PATCH v2 3/3] x86: Clear MTRRs on vCPU reset Date: Thu, 14 Aug 2014 23:23:49 +0200 Message-ID: <53ED28E5.9040305@redhat.com> References: <20140814191147.13303.61655.stgit@gimli.home> <20140814192415.13303.34846.stgit@gimli.home> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: qemu-stable@nongnu.org To: Alex Williamson , qemu-devel@nongnu.org, kvm@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26891 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932072AbaHNVXw (ORCPT ); Thu, 14 Aug 2014 17:23:52 -0400 In-Reply-To: <20140814192415.13303.34846.stgit@gimli.home> Sender: kvm-owner@vger.kernel.org List-ID: 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