Re: [edk2-devel] [PATCH v3 1/4] OvmfPkg/Sec: Setup MTRR early in the boot process.

["Laszlo Ersek" <lersek@redhat.com>]

On 1/30/24 14:04, Gerd Hoffmann wrote:
> Specifically before running lzma uncompress of the main firmware volume.
> This is needed to make sure caching is enabled, otherwise the uncompress
> can be extremely slow.
> 
> Adapt the ASSERTs and MTRR setup in PlatformInitLib to the changes.
> 
> Background:  Depending on virtual machine configuration kvm may uses EPT
> memory types to apply guest MTRR settings.  In case MTRRs are disabled
> kvm will use the uncachable memory type for all mappings.  The
> vmx_get_mt_mask() function in the linux kernel handles this and can be
> found here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/x86/kvm/vmx/vmx.c?h=v6.7.1#n7580
> 
> In most VM configurations kvm uses MTRR_TYPE_WRBACK unconditionally.  In
> case the VM has a mdev device assigned that is not the case though.
> 
> Before commit e8aa4c6546ad ("UefiCpuPkg/ResetVector: Cache Disable
> should not be set by default in CR0") kvm also ended up using
> MTRR_TYPE_WRBACK due to KVM_X86_QUIRK_CD_NW_CLEARED.  After that commit
> kvm evaluates guest mtrr settings, which why setting up MTRRs early is
> important now.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/IntelTdx/Sec/SecMain.c              | 32 +++++++++++++++++++++
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c | 10 +++----
>  OvmfPkg/Sec/SecMain.c                       | 32 +++++++++++++++++++++
>  3 files changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/OvmfPkg/IntelTdx/Sec/SecMain.c b/OvmfPkg/IntelTdx/Sec/SecMain.c
> index 42a587adfa57..a218ca17a01a 100644
> --- a/OvmfPkg/IntelTdx/Sec/SecMain.c
> +++ b/OvmfPkg/IntelTdx/Sec/SecMain.c
> @@ -27,6 +27,8 @@
>  #include <Library/TdxHelperLib.h>
>  #include <Library/CcProbeLib.h>
>  #include <Library/PeilessStartupLib.h>
> +#include <Register/Intel/ArchitecturalMsr.h>
> +#include <Register/Intel/Cpuid.h>
>  
>  #define SEC_IDT_ENTRY_COUNT  34
>  
> @@ -48,6 +50,31 @@ IA32_IDT_GATE_DESCRIPTOR  mIdtEntryTemplate = {
>    }
>  };
>  
> +//
> +// Enable MTRR early, set default type to write back.
> +// Needed to make sure caching is enabled,
> +// without this lzma decompress can be very slow.
> +//
> +STATIC
> +VOID
> +SecMtrrSetup (
> +  VOID
> +  )
> +{
> +  CPUID_VERSION_INFO_EDX           Edx;
> +  MSR_IA32_MTRR_DEF_TYPE_REGISTER  DefType;
> +
> +  AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, &Edx.Uint32);
> +  if (!Edx.Bits.MTRR) {
> +    return;
> +  }
> +
> +  DefType.Uint64    = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
> +  DefType.Bits.Type = 6; /* write back */
> +  DefType.Bits.E    = 1; /* enable */
> +  AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
> +}
> +
>  VOID
>  EFIAPI
>  SecCoreStartupWithStack (
> @@ -204,6 +231,11 @@ SecCoreStartupWithStack (
>    InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
>    DisableApicTimerInterrupt ();
>  
> +  //
> +  // Initialize MTRR
> +  //
> +  SecMtrrSetup ();
> +
>    PeilessStartup (&SecCoreData);
>  
>    ASSERT (FALSE);
> diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> index f042517bb64a..e89f63eee054 100644
> --- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> +++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> @@ -1082,18 +1082,18 @@ PlatformQemuInitializeRam (
>      MtrrGetAllMtrrs (&MtrrSettings);
>  
>      //
> -    // MTRRs disabled, fixed MTRRs disabled, default type is uncached
> +    // See SecMtrrSetup(), default type should be write back
>      //
> -    ASSERT ((MtrrSettings.MtrrDefType & BIT11) == 0);
> +    ASSERT ((MtrrSettings.MtrrDefType & BIT11) != 0);
>      ASSERT ((MtrrSettings.MtrrDefType & BIT10) == 0);
> -    ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == 0);
> +    ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == MTRR_CACHE_WRITE_BACK);
>  
>      //
>      // flip default type to writeback
>      //
> -    SetMem (&MtrrSettings.Fixed, sizeof MtrrSettings.Fixed, 0x06);
> +    SetMem (&MtrrSettings.Fixed, sizeof MtrrSettings.Fixed, MTRR_CACHE_WRITE_BACK);
>      ZeroMem (&MtrrSettings.Variables, sizeof MtrrSettings.Variables);
> -    MtrrSettings.MtrrDefType |= BIT11 | BIT10 | 6;
> +    MtrrSettings.MtrrDefType |= BIT10;
>      MtrrSetAllMtrrs (&MtrrSettings);
>  
>      //
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index 31da5d0ace51..46c54f2984ff 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -30,6 +30,8 @@
>  #include <Ppi/MpInitLibDep.h>
>  #include <Library/TdxHelperLib.h>
>  #include <Library/CcProbeLib.h>
> +#include <Register/Intel/ArchitecturalMsr.h>
> +#include <Register/Intel/Cpuid.h>
>  #include "AmdSev.h"
>  
>  #define SEC_IDT_ENTRY_COUNT  34
> @@ -744,6 +746,31 @@ FindAndReportEntryPoints (
>    return;
>  }
>  
> +//
> +// Enable MTRR early, set default type to write back.
> +// Needed to make sure caching is enabled,
> +// without this lzma decompress can be very slow.
> +//
> +STATIC
> +VOID
> +SecMtrrSetup (
> +  VOID
> +  )
> +{
> +  CPUID_VERSION_INFO_EDX           Edx;
> +  MSR_IA32_MTRR_DEF_TYPE_REGISTER  DefType;
> +
> +  AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, &Edx.Uint32);
> +  if (!Edx.Bits.MTRR) {
> +    return;
> +  }
> +
> +  DefType.Uint64    = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
> +  DefType.Bits.Type = 6; /* write back */
> +  DefType.Bits.E    = 1; /* enable */
> +  AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
> +}
> +
>  VOID
>  EFIAPI
>  SecCoreStartupWithStack (
> @@ -942,6 +969,11 @@ SecCoreStartupWithStack (
>    InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
>    DisableApicTimerInterrupt ();
>  
> +  //
> +  // Initialize MTRR
> +  //
> +  SecMtrrSetup ();
> +
>    //
>    // Initialize Debug Agent to support source level debug in SEC/PEI phases before memory ready.
>    //

Cannot comment on the "OvmfPkg/IntelTdx/Sec/SecMain.c" source file
changes, but for the rest:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114818): https://edk2.groups.io/g/devel/message/114818
Mute This Topic: https://groups.io/mt/104052591/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-