From nobody Sat May 4 01:17:55 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+90807+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+90807+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 165640606903165.27881126146758; Tue, 28 Jun 2022 01:47:49 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id 09JDYY1788612xPAHDAPtAso; Tue, 28 Jun 2022 01:47:48 -0700 X-Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web08.53688.1656406067733066959 for ; Tue, 28 Jun 2022 01:47:47 -0700 X-IronPort-AV: E=McAfee;i="6400,9594,10391"; a="264719317" X-IronPort-AV: E=Sophos;i="5.92,227,1650956400"; d="scan'208";a="264719317" X-Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jun 2022 01:47:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.92,227,1650956400"; d="scan'208";a="679950639" X-Received: from sh1gapp1009.ccr.corp.intel.com ([10.239.189.79]) by FMSMGA003.fm.intel.com with ESMTP; 28 Jun 2022 01:47:45 -0700 From: "Wu, Jiaxin" To: devel@edk2.groups.io Cc: Eric Dong , Ray Ni Subject: [edk2-devel] [PATCH v1] UefiCpuPkg: Add PCD to control SMRR enable & SmmFeatureControl support Date: Tue, 28 Jun 2022 16:47:39 +0800 Message-Id: <20220628084739.12216-1-jiaxin.wu@intel.com> Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,jiaxin.wu@intel.com X-Gm-Message-State: l4t0NQU9rJJiZ3n7TLmQaX3nx1787277AA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1656406068; bh=sarhxXGOAyJspTZH/MJsY2BRjOpusW6j8hKm4nKJ0gs=; h=Cc:Date:From:Reply-To:Subject:To; b=dainfM7EukE5WNkNufYRGf8AYunA7eXngRbL5X6HVWAISqUEKI6xPC25WpTVyySWy00 urf4+wODakR5o18ulJSXDQpJDUG3SxeUoU9vc0aJgSTJ+ZVR77fCNyGwxI3sC13CecdbG c4xgL7W6wNb3gTkbJjagDAXtIISmHXOZ+Zw= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1656406070781100003 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3962 Two SMM variables (mSmrrSupported & mSmmFeatureControlSupported) are global variables, they control whether the SMRR and SMM Feature Control MSR will be restored respectively. To avoid the TOCTOU, add PCD to control SMRR & SmmFeatureControl enable. Change-Id: I6835e4b0e12c5e6f52effb60fd9224e3eb97fc0d Cc: Eric Dong Cc: Ray Ni Signed-off-by: Jiaxin Wu --- .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 ++ .../SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c | 84 ++++--------------= ---- .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 4 ++ .../StandaloneMmCpuFeaturesLib.inf | 4 ++ UefiCpuPkg/UefiCpuPkg.dec | 12 ++++ UefiCpuPkg/UefiCpuPkg.uni | 12 ++++ 6 files changed, 48 insertions(+), 72 deletions(-) diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/U= efiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf index 35292dac31..7b5cef9700 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf @@ -33,5 +33,9 @@ MemoryAllocationLib DebugLib =20 [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOME= TIMES_CONSUMES + +[FeaturePcd] + gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable ## CONSUMES diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c= b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c index 78de7f8407..b88cdece2a 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c @@ -35,20 +35,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // MSRs required for configuration of SMM Code Access Check // #define SMM_FEATURES_LIB_IA32_MCA_CAP 0x17D #define SMM_CODE_ACCESS_CHK_BIT BIT58 =20 -// -// Set default value to assume SMRR is not supported -// -BOOLEAN mSmrrSupported =3D FALSE; - -// -// Set default value to assume MSR_SMM_FEATURE_CONTROL is not supported -// -BOOLEAN mSmmFeatureControlSupported =3D FALSE; - // // Set default value to assume IA-32 Architectural MSRs are used // UINT32 mSmrrPhysBaseMsr =3D SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE; UINT32 mSmrrPhysMaskMsr =3D SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK; @@ -81,39 +71,27 @@ CpuFeaturesLibInitialization ( UINTN ModelId; =20 // // Retrieve CPU Family and Model // - AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx); + AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, NULL); FamilyId =3D (RegEax >> 8) & 0xf; ModelId =3D (RegEax >> 4) & 0xf; if ((FamilyId =3D=3D 0x06) || (FamilyId =3D=3D 0x0f)) { ModelId =3D ModelId | ((RegEax >> 12) & 0xf0); } =20 - // - // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability - // - if ((RegEdx & BIT12) !=3D 0) { - // - // Check MTRR_CAP MSR bit 11 for SMRR support - // - if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) !=3D 0) { - mSmrrSupported =3D TRUE; - } - } - // // Intel(R) 64 and IA-32 Architectures Software Developer's Manual // Volume 3C, Section 35.3 MSRs in the Intel(R) Atom(TM) Processor Family // // If CPU Family/Model is 06_1CH, 06_26H, 06_27H, 06_35H or 06_36H, then // SMRR Physical Base and SMM Physical Mask MSRs are not available. // if (FamilyId =3D=3D 0x06) { if ((ModelId =3D=3D 0x1C) || (ModelId =3D=3D 0x26) || (ModelId =3D=3D = 0x27) || (ModelId =3D=3D 0x35) || (ModelId =3D=3D 0x36)) { - mSmrrSupported =3D FALSE; + ASSERT (!FeaturePcdGet (PcdSmrrEnable)); } } =20 // // Intel(R) 64 and IA-32 Architectures Software Developer's Manual @@ -194,14 +172,10 @@ SmmCpuFeaturesInitializeProcessor ( IN CPU_HOT_PLUG_DATA *CpuHotPlugData ) { SMRAM_SAVE_STATE_MAP *CpuState; UINT64 FeatureControl; - UINT32 RegEax; - UINT32 RegEdx; - UINTN FamilyId; - UINTN ModelId; =20 // // Configure SMBASE. // CpuState =3D (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMB= ASE + SMRAM_SAVE_STATE_MAP_OFFSET); @@ -214,17 +188,17 @@ SmmCpuFeaturesInitializeProcessor ( // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used,= then // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is set b= efore // accessing SMRR base/mask MSRs. If Lock(BIT0) of MSR_FEATURE_CONTROL = MSR(0x3A) // is set, then the MSR is locked and can not be modified. // - if (mSmrrSupported && (mSmrrPhysBaseMsr =3D=3D SMM_FEATURES_LIB_IA32_COR= E_SMRR_PHYSBASE)) { + if (mSmrrPhysBaseMsr =3D=3D SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE) { FeatureControl =3D AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL= ); if ((FeatureControl & BIT3) =3D=3D 0) { - if ((FeatureControl & BIT0) =3D=3D 0) { + if (((FeatureControl & BIT0) =3D=3D 0) && (FeaturePcdGet (PcdSmrrEna= ble))) { AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureContr= ol | BIT3); } else { - mSmrrSupported =3D FALSE; + ASSERT (!FeaturePcdGet (PcdSmrrEnable)); } } } =20 // @@ -232,11 +206,11 @@ SmmCpuFeaturesInitializeProcessor ( // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first norma= l SMI. // The code that initializes SMM environment is running in normal mode // from SMRAM region. If SMRR is enabled here, then the SMRAM region // is protected and the normal mode code execution will fail. // - if (mSmrrSupported) { + if (FeaturePcdGet (PcdSmrrEnable)) { // // SMRR size cannot be less than 4-KBytes // SMRR size must be of length 2^n // SMRR base alignment cannot be less than SMRR length // @@ -256,44 +230,10 @@ SmmCpuFeaturesInitializeProcessor ( AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1) & = EFI_MSR_SMRR_MASK)); mSmrrEnabled[CpuIndex] =3D FALSE; } } =20 - // - // Retrieve CPU Family and Model - // - AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx); - FamilyId =3D (RegEax >> 8) & 0xf; - ModelId =3D (RegEax >> 4) & 0xf; - if ((FamilyId =3D=3D 0x06) || (FamilyId =3D=3D 0x0f)) { - ModelId =3D ModelId | ((RegEax >> 12) & 0xf0); - } - - // - // Intel(R) 64 and IA-32 Architectures Software Developer's Manual - // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM) - // Processor Family. - // - // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation - // Intel(R) Core(TM) Processor Family MSRs. - // - if (FamilyId =3D=3D 0x06) { - if ((ModelId =3D=3D 0x3C) || (ModelId =3D=3D 0x45) || (ModelId =3D=3D = 0x46) || - (ModelId =3D=3D 0x3D) || (ModelId =3D=3D 0x47) || (ModelId =3D=3D = 0x4E) || (ModelId =3D=3D 0x4F) || - (ModelId =3D=3D 0x3F) || (ModelId =3D=3D 0x56) || (ModelId =3D=3D = 0x57) || (ModelId =3D=3D 0x5C) || - (ModelId =3D=3D 0x8C)) - { - // - // Check to see if the CPU supports the SMM Code Access Check feature - // Do not access this MSR unless the CPU supports the SmmRegFeatureC= ontrol - // - if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) & SMM_CODE_ACCESS_= CHK_BIT) !=3D 0) { - mSmmFeatureControlSupported =3D TRUE; - } - } - } - // // Call internal worker function that completes the CPU initialization // FinishSmmCpuFeaturesInitializeProcessor (); } @@ -381,11 +321,11 @@ VOID EFIAPI SmmCpuFeaturesDisableSmrr ( VOID ) { - if (mSmrrSupported && mNeedConfigureMtrrs) { + if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) { AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EF= I_MSR_SMRR_PHYS_MASK_VALID); } } =20 /** @@ -396,11 +336,11 @@ VOID EFIAPI SmmCpuFeaturesReenableSmrr ( VOID ) { - if (mSmrrSupported && mNeedConfigureMtrrs) { + if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) { AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI= _MSR_SMRR_PHYS_MASK_VALID); } } =20 /** @@ -417,11 +357,11 @@ SmmCpuFeaturesRendezvousEntry ( ) { // // If SMRR is supported and this is the first normal SMI, then enable SM= RR // - if (mSmrrSupported && !mSmrrEnabled[CpuIndex]) { + if (FeaturePcdGet (PcdSmrrEnable) && !mSmrrEnabled[CpuIndex]) { AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI= _MSR_SMRR_PHYS_MASK_VALID); mSmrrEnabled[CpuIndex] =3D TRUE; } } =20 @@ -458,11 +398,11 @@ EFIAPI SmmCpuFeaturesIsSmmRegisterSupported ( IN UINTN CpuIndex, IN SMM_REG_NAME RegName ) { - if (mSmmFeatureControlSupported && (RegName =3D=3D SmmRegFeatureControl)= ) { + if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName =3D=3D SmmReg= FeatureControl)) { return TRUE; } =20 return FALSE; } @@ -484,11 +424,11 @@ EFIAPI SmmCpuFeaturesGetSmmRegister ( IN UINTN CpuIndex, IN SMM_REG_NAME RegName ) { - if (mSmmFeatureControlSupported && (RegName =3D=3D SmmRegFeatureControl)= ) { + if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName =3D=3D SmmReg= FeatureControl)) { return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL); } =20 return 0; } @@ -510,11 +450,11 @@ SmmCpuFeaturesSetSmmRegister ( IN UINTN CpuIndex, IN SMM_REG_NAME RegName, IN UINT64 Value ) { - if (mSmmFeatureControlSupported && (RegName =3D=3D SmmRegFeatureControl)= ) { + if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName =3D=3D SmmReg= FeatureControl)) { AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL, Value); } } =20 /** diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf = b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf index 022351f593..85214ee31c 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf @@ -68,7 +68,11 @@ gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOME= TIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize ## SOME= TIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStmExceptionStackSize ## SOME= TIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONS= UMES =20 +[FeaturePcd] + gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable ## CONSUMES + [Depex] gEfiMpServiceProtocolGuid diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLi= b.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf index ec97041d8b..3eacab48db 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf @@ -34,5 +34,9 @@ MemoryAllocationLib PcdLib =20 [FixedPcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOME= TIMES_CONSUMES + +[FeaturePcd] + gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable ## CONSUMES diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 1951eb294c..55cbe7605f 100644 --- a/UefiCpuPkg/UefiCpuPkg.dec +++ b/UefiCpuPkg/UefiCpuPkg.dec @@ -153,10 +153,22 @@ # TRUE - SMM Feature Control MSR will be locked.
# FALSE - SMM Feature Control MSR will not be locked.
# @Prompt Lock SMM Feature Control MSR. gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock|TRUE|BOOLEAN|0x= 3213210B =20 + ## Indicates if SMRR will be enabled.

+ # TRUE - SMRR will be enabled.
+ # FALSE - SMRR will not be enabled.
+ # @Prompt Enable SMRR. + gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable|TRUE|BOOLEAN|0x3213210D + + ## Indicates if SmmFeatureControl will be enabled.

+ # TRUE - SmmFeatureControl will be enabled.
+ # FALSE - SmmFeatureControl will not be enabled.
+ # @Prompt Support SmmFeatureControl. + gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable|TRUE|BOOLEAN|0x3213= 2110 + [PcdsFixedAtBuild] ## List of exception vectors which need switching stack. # This PCD will only take into effect if PcdCpuStackGuard is enabled. # By default exception #DD(8), #PF(14) are supported. # @Prompt Specify exception vectors which need switching stack. diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni index 219c1963bf..d17bcfd10c 100644 --- a/UefiCpuPkg/UefiCpuPkg.uni +++ b/UefiCpuPkg/UefiCpuPkg.uni @@ -98,10 +98,22 @@ =20 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmFeatureControlMsrLock_HELP = #language en-US "Lock SMM Feature Control MSR?

\n" = "TRUE - locked.
\n" = "FALSE - unlocked.
" =20 +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmrrEnable_PROMPT #language en-U= S "Indicates if SMRR will be enabled." + +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmrrEnable_HELP #language en-US = "Indicates if SMRR will be enabled.

\n" + = "TRUE - SMRR will be enabled.
\n" + = "FALSE - SMRR will not be enabled.
" + +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmmFeatureControlEnable_PROMPT #= language en-US "Indicates if SmmFeatureControl will be enabled." + +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmmFeatureControlEnable_HELP #la= nguage en-US "Indicates if SmmFeatureControl will be enabled.

\n" + = "TRUE - SmmFeatureControl will be enabled.
\n" + = "FALSE - SmmFeatureControl will not be enabled.
" + #string STR_gUefiCpuPkgTokenSpaceGuid_PcdPeiTemporaryRamStackSize_PROMPT = #language en-US "Stack size in the temporary RAM" =20 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdPeiTemporaryRamStackSize_HELP #l= anguage en-US "Specifies stack size in the temporary RAM. 0 means half of T= emporaryRamSize." =20 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileSize_PROMPT #langua= ge en-US "SMM profile data buffer size" --=20 2.16.2.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90807): https://edk2.groups.io/g/devel/message/90807 Mute This Topic: https://groups.io/mt/92040046/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-