From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=jordan.l.justen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 39DE921959CB2 for ; Tue, 19 Feb 2019 11:45:01 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Feb 2019 11:45:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,388,1544515200"; d="scan'208";a="148134529" Received: from mmdandap-mobl1.amr.corp.intel.com (HELO localhost) ([10.254.8.66]) by fmsmga001.fm.intel.com with ESMTP; 19 Feb 2019 11:45:00 -0800 MIME-Version: 1.0 In-Reply-To: References: <20190218101015.23399-1-jordan.l.justen@intel.com> From: Jordan Justen To: Laszlo Ersek , edk2-devel@lists.01.org Cc: Peter Fang , Maurice Ma , Ard Biesheuvel , Anthony Perard , Julien Grall Message-ID: <155060550047.7367.10327364927488705594@jljusten-skl> User-Agent: alot/0.8 Date: Tue, 19 Feb 2019 11:45:00 -0800 Subject: Re: [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 register X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Feb 2019 19:45:02 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2019-02-18 04:17:26, Laszlo Ersek wrote: > On 02/18/19 11:10, Jordan Justen wrote: > > Clear the CD (Cache Disable) flag in the CR0 register. When the VM > > implements the CD flag, this can substantially decrease the time it > > takes to decompress the firmware volumes. > >=20 > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jordan Justen > > Tested-by: Peter Fang > > Cc: Peter Fang > > Cc: Maurice Ma > > Cc: Laszlo Ersek > > Cc: Ard Biesheuvel > > Cc: Anthony Perard > > Cc: Julien Grall > > --- > > OvmfPkg/Sec/Ia32/SecEntry.nasm | 8 +++++++- > > OvmfPkg/Sec/X64/SecEntry.nasm | 8 +++++++- > > 2 files changed, 14 insertions(+), 2 deletions(-) > >=20 > > diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry= .nasm > > index 03501969eb..fc7f47385a 100644 > > --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm > > +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm > > @@ -40,6 +40,13 @@ extern ASM_PFX(SecCoreStartupWithStack) > > global ASM_PFX(_ModuleEntryPoint) > > ASM_PFX(_ModuleEntryPoint): > > =20 > > + ; > > + ; Clear the CD (Cache Disable) flag in the CR0 register. > > + ; > > + mov eax, cr0 > > + and eax, ~(1 << 30) > > + mov cr0, eax > > + > > ; > > ; Fill the temporary RAM with the initial stack value. > > ; The loop below will seed the heap as well, but that's harmless. > > @@ -71,4 +78,3 @@ ASM_PFX(_ModuleEntryPoint): > > push eax > > push ebp > > call ASM_PFX(SecCoreStartupWithStack) > > - > > diff --git a/OvmfPkg/Sec/X64/SecEntry.nasm b/OvmfPkg/Sec/X64/SecEntry.n= asm > > index d76adcffd8..7471b3a3e3 100644 > > --- a/OvmfPkg/Sec/X64/SecEntry.nasm > > +++ b/OvmfPkg/Sec/X64/SecEntry.nasm > > @@ -41,6 +41,13 @@ extern ASM_PFX(SecCoreStartupWithStack) > > global ASM_PFX(_ModuleEntryPoint) > > ASM_PFX(_ModuleEntryPoint): > > =20 > > + ; > > + ; Clear the CD (Cache Disable) flag in the CR0 register. > > + ; > > + mov rax, cr0 > > + and eax, ~(1 << 30) > > + mov cr0, rax > > + > > ; > > ; Fill the temporary RAM with the initial stack value. > > ; The loop below will seed the heap as well, but that's harmless. > > @@ -72,4 +79,3 @@ ASM_PFX(_ModuleEntryPoint): > > mov rdx, rsp > > sub rsp, 0x20 > > call ASM_PFX(SecCoreStartupWithStack) > > - > >=20 >=20 > In commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in initial > page tables", 2013-09-24), we said >=20 > In UEFI X64 we use other mechanisms to disable caching. > (CD/NW in CR0 and MTRRs.) >=20 > That suggests that having caching disabled in SEC is a good thing. I think there can be good reasons to disable caching on a platform, such as during memory initialization, but I'm not sure any apply to OVMF. As far as I know, kvm still doesn't attempt to disable caching via the CR0.CD bit or the MTRRs. This kind of backs up the previous statement about there not being a reason to disable caching in OVMF. > What has changed? I assume Peter reported a problem. >=20 > ... Is this by any chance related to Linux commit 879ae1880449 ("KVM: > x86: obey KVM_X86_QUIRK_CD_NW_CLEARED in kvm_set_cr0()", 2015-11-04) -- > or CR0.CD virtualization in KVM, in general? >=20 > (The commit message says "When the VM implements the CD flag", and not > "When the *VMM* implements the CD flag", so I'm unsure.) Yeah, I guess we probably want VMM, and I don't think the observed issue applies to kvm. -Jordan