Re: [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 register

[Laszlo Ersek <lersek@redhat.com>]

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 <jordan.l.justen@intel.com>
> Tested-by: Peter Fang <peter.fang@intel.com>
> Cc: Peter Fang <peter.fang@intel.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> ---
>  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.

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.)

Thanks
Laszlo