From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 329082194EB7A for ; Wed, 20 Feb 2019 01:37:53 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ABABF13AAA; Wed, 20 Feb 2019 09:37:52 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-220.rdu2.redhat.com [10.10.120.220]) by smtp.corp.redhat.com (Postfix) with ESMTP id D2B6219C57; Wed, 20 Feb 2019 09:37:50 +0000 (UTC) To: "Fang, Peter" , "Justen, Jordan L" , "edk2-devel@lists.01.org" Cc: "Ma, Maurice" , Ard Biesheuvel , Anthony Perard , Julien Grall References: <20190218101015.23399-1-jordan.l.justen@intel.com> <155060550047.7367.10327364927488705594@jljusten-skl> From: Laszlo Ersek Message-ID: <3bf00e59-247f-0645-ca21-12b24bd79e4f@redhat.com> Date: Wed, 20 Feb 2019 10:37:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 20 Feb 2019 09:37:52 +0000 (UTC) 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: Wed, 20 Feb 2019 09:37:53 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 02/20/19 03:43, Fang, Peter wrote: > When CR0.CD is set, KVM *might* emulate it by setting MTRR_TYPE_UNCACHABLE in its EPTE if non-coherent DMA is detected, because VT-d does not provide snooping in this case. I think this was precisely the use case that I had to tweak with Linux commit 879ae1880449. (Which complemented earlier tweak fb279950ba02.) The original commit fb279950ba02 -- i.e., the kernel regression -- mentions "noncoherent_dma guests". > AFAIK, leaving the bit on can have a performance impact for Xen as well. I can't comment on that; haven't seen it reported. > This issue was found when we tried to boot OVMF on ACRN. Ah, yet another "small footprint" VMM. :) > ACRN emulates CR0.CD through guest IPAT and memory decompression took a very long time to finish (tens of seconds). OK. Please include this description of the use case in the commit message. If we have a specific use case that triggers a symptom, the commit message shouldn't be general only. It can be general too, but the specifics should be mentioned. I consider this patch easy to revert (if necessary) for a long time to come. So I'm not too worried about regressing other setups. If we do, we can revert the patch. I'm OK with the patch once the commit message is cleaned up, and we figure out what we want to do about "~(1 << 30)" vs. BTR in the assembly. Thanks, Laszlo >> -----Original Message----- >> From: Justen, Jordan L >> Sent: Tuesday, February 19, 2019 11:45 AM >> To: Laszlo Ersek ; edk2-devel@lists.01.org >> Cc: Fang, Peter ; Ma, Maurice >> ; Ard Biesheuvel ; >> Anthony Perard ; Julien Grall >> >> Subject: Re: [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 >> register >> >> 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. >>>> >>>> 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(-) >>>> >>>> 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): >>>> >>>> + ; >>>> + ; 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.nasm 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): >>>> >>>> + ; >>>> + ; 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) >>>> - >>>> >>> >>> In commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in >>> initial page tables", 2013-09-24), we said >>> >>> In UEFI X64 we use other mechanisms to disable caching. >>> (CD/NW in CR0 and MTRRs.) >>> >>> 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. >>> >>> ... 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? >>> >>> (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