From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id E3F23AC11CC for ; Wed, 10 Apr 2024 13:57:48 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=TX2DKqIcnS+iGQqNo8Kkws2AoUWenYW8sRjiWReeaoU=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe; s=20240206; t=1712757467; v=1; b=lp2L2aRfxS4dB2pTZqQ1W6IblHO+cjoFS3bvVRdVkvQ2DxHb70BGbkT8M4t2GWkARpEdp2og KBRHTGWjFmT0YK7kM6wem0g4GfHEJpKFHt6uu5WEVMyXvBEeRly+kNJaNxsKfQGjfqX4flr5SOx py5D/iIIMOzziB3M2UlzFx0ZHayQNbUBjUqaCdhxfccl6vLVOrrx3De1TtYCylQmVCcNR9scT/z DpB9ThJno1ANDQg5fQuJzPk7E+A7F4MFe9h+aRc0ZNfBGr+iDSETAKM4bnVzr25tSdWHQGmwJnO 8gDFgYr7w9juJHnWwTtAkJcEKia/A4SA4seO0Iqy0oGeg== X-Received: by 127.0.0.2 with SMTP id sPVlYY7687511xV86nRcWWE2; Wed, 10 Apr 2024 06:57:47 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by mx.groups.io with SMTP id smtpd.web10.306.1712757450770649687 for ; Wed, 10 Apr 2024 06:57:47 -0700 X-CSE-ConnectionGUID: BTt82lFCQsaH7ZS3kS3/FQ== X-CSE-MsgGUID: CQSV733QSzaRffnSqIC9xw== X-IronPort-AV: E=McAfee;i="6600,9927,11039"; a="30602173" X-IronPort-AV: E=Sophos;i="6.07,190,1708416000"; d="scan'208";a="30602173" X-Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2024 06:57:47 -0700 X-CSE-ConnectionGUID: o1iXXV+IS/G/X3hvYbF4XQ== X-CSE-MsgGUID: mIp6c/7eTn6yMFpRBP0PcQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,190,1708416000"; d="scan'208";a="51778181" X-Received: from sh1gapp1009.ccr.corp.intel.com ([10.239.189.219]) by fmviesa001.fm.intel.com with ESMTP; 10 Apr 2024 06:57:45 -0700 From: "Wu, Jiaxin" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Jiewen Yao , Gerd Hoffmann , Ray Ni Subject: [edk2-devel] [PATCH v1 09/13] OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid Date: Wed, 10 Apr 2024 21:57:20 +0800 Message-Id: <20240410135724.15344-10-jiaxin.wu@intel.com> In-Reply-To: <20240410135724.15344-1-jiaxin.wu@intel.com> References: <20240410135724.15344-1-jiaxin.wu@intel.com> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Wed, 10 Apr 2024 06:57:47 -0700 Resent-From: jiaxin.wu@intel.com Reply-To: devel@edk2.groups.io,jiaxin.wu@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: nz5Bpu6mwF7bCpp0wE3CsOtKx7686176AA= X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=lp2L2aRf; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io This patch refines the SmmAccess implementation: 1. SmramMap will be retrieved from the gEfiSmmSmramMemoryGuid instead of original from the TSEG Memory Base register. 2. Remove the gEfiAcpiVariableGuid creation, thus the DESCRIPTOR_INDEX definition can be also cleaned. The gEfiAcpiVariableGuid HOB will be created in the OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf. Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Gerd Hoffmann Cc: Ray Ni Signed-off-by: Jiaxin Wu --- OvmfPkg/SmmAccess/SmmAccess2Dxe.c | 4 +- OvmfPkg/SmmAccess/SmmAccess2Dxe.inf | 5 +++ OvmfPkg/SmmAccess/SmmAccessPei.c | 88 +++---------------------------------- OvmfPkg/SmmAccess/SmmAccessPei.inf | 7 +-- OvmfPkg/SmmAccess/SmramInternal.c | 73 +++++++++++------------------- OvmfPkg/SmmAccess/SmramInternal.h | 18 +------- 6 files changed, 41 insertions(+), 154 deletions(-) diff --git a/OvmfPkg/SmmAccess/SmmAccess2Dxe.c b/OvmfPkg/SmmAccess/SmmAccess2Dxe.c index 4b9e6df37f..3371592de7 100644 --- a/OvmfPkg/SmmAccess/SmmAccess2Dxe.c +++ b/OvmfPkg/SmmAccess/SmmAccess2Dxe.c @@ -4,11 +4,11 @@ Q35 TSEG is expected to have been verified and set up by the SmmAccessPei driver. Copyright (C) 2013, 2015, Red Hat, Inc.
- Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -113,12 +113,10 @@ SmmAccess2DxeGetCapabilities ( IN OUT UINTN *SmramMapSize, IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap ) { return SmramAccessGetCapabilities ( - This->LockState, - This->OpenState, SmramMapSize, SmramMap ); } diff --git a/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf b/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf index d86381d0fb..d9f01a13c4 100644 --- a/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf +++ b/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf @@ -3,10 +3,11 @@ # # Q35 TSEG is expected to have been verified and set up by the SmmAccessPei # driver. # # Copyright (C) 2013, 2015, Red Hat, Inc. +# Copyright (c) 2024 Intel Corporation. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -39,17 +40,21 @@ DebugLib PcdLib PciLib UefiBootServicesTableLib UefiDriverEntryPoint + HobLib [Protocols] gEfiSmmAccess2ProtocolGuid ## PRODUCES [FeaturePcd] gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire +[Guids] + gEfiSmmSmramMemoryGuid # ALWAYS_CONSUMED + [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes [Depex] diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c b/OvmfPkg/SmmAccess/SmmAccessPei.c index 0e57b7804c..9459bcc989 100644 --- a/OvmfPkg/SmmAccess/SmmAccessPei.c +++ b/OvmfPkg/SmmAccess/SmmAccessPei.c @@ -9,21 +9,19 @@ This PEIM runs from RAM, so we can write to variables with static storage duration. Copyright (C) 2013, 2015, Red Hat, Inc.
- Copyright (c) 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent **/ -#include #include #include #include -#include #include #include #include #include #include @@ -62,14 +60,10 @@ SmmAccessPeiOpen ( IN EFI_PEI_SERVICES **PeiServices, IN PEI_SMM_ACCESS_PPI *This, IN UINTN DescriptorIndex ) { - if (DescriptorIndex >= DescIdxCount) { - return EFI_INVALID_PARAMETER; - } - // // According to current practice, DescriptorIndex is not considered at all, // beyond validating it. // return SmramAccessOpen (&This->LockState, &This->OpenState); @@ -100,14 +94,10 @@ SmmAccessPeiClose ( IN EFI_PEI_SERVICES **PeiServices, IN PEI_SMM_ACCESS_PPI *This, IN UINTN DescriptorIndex ) { - if (DescriptorIndex >= DescIdxCount) { - return EFI_INVALID_PARAMETER; - } - // // According to current practice, DescriptorIndex is not considered at all, // beyond validating it. // return SmramAccessClose (&This->LockState, &This->OpenState); @@ -137,14 +127,10 @@ SmmAccessPeiLock ( IN EFI_PEI_SERVICES **PeiServices, IN PEI_SMM_ACCESS_PPI *This, IN UINTN DescriptorIndex ) { - if (DescriptorIndex >= DescIdxCount) { - return EFI_INVALID_PARAMETER; - } - // // According to current practice, DescriptorIndex is not considered at all, // beyond validating it. // return SmramAccessLock (&This->LockState, &This->OpenState); @@ -176,12 +162,10 @@ SmmAccessPeiGetCapabilities ( IN OUT UINTN *SmramMapSize, IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap ) { return SmramAccessGetCapabilities ( - This->LockState, - This->OpenState, SmramMapSize, SmramMap ); } @@ -238,18 +222,14 @@ EFIAPI SmmAccessPeiEntryPoint ( IN EFI_PEI_FILE_HANDLE FileHandle, IN CONST EFI_PEI_SERVICES **PeiServices ) { - UINT16 HostBridgeDevId; - UINT8 EsmramcVal; - UINT8 RegMask8; - UINT32 TopOfLowRam, TopOfLowRamMb; - EFI_STATUS Status; - UINTN SmramMapSize; - EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount]; - VOID *GuidHob; + UINT16 HostBridgeDevId; + UINT8 EsmramcVal; + UINT8 RegMask8; + UINT32 TopOfLowRam, TopOfLowRamMb; // // This module should only be included if SMRAM support is required. // ASSERT (FeaturePcdGet (PcdSmmSmramRequire)); @@ -354,69 +334,11 @@ SmmAccessPeiEntryPoint ( DRAMC_REGISTER_Q35 (MCH_SMRAM), (UINT8)((~(UINT32)MCH_SMRAM_D_LCK) & 0xff), MCH_SMRAM_G_SMRAME ); - // - // Create the GUID HOB and point it to the first SMRAM range. - // GetStates (&mAccess.LockState, &mAccess.OpenState); - SmramMapSize = sizeof SmramMap; - Status = SmramAccessGetCapabilities ( - mAccess.LockState, - mAccess.OpenState, - &SmramMapSize, - SmramMap - ); - ASSERT_EFI_ERROR (Status); - - DEBUG_CODE_BEGIN (); - { - UINTN Count; - UINTN Idx; - - Count = SmramMapSize / sizeof SmramMap[0]; - DEBUG (( - DEBUG_VERBOSE, - "%a: SMRAM map follows, %d entries\n", - __func__, - (INT32)Count - )); - DEBUG (( - DEBUG_VERBOSE, - "% 20a % 20a % 20a % 20a\n", - "PhysicalStart(0x)", - "PhysicalSize(0x)", - "CpuStart(0x)", - "RegionState(0x)" - )); - for (Idx = 0; Idx < Count; ++Idx) { - DEBUG (( - DEBUG_VERBOSE, - "% 20Lx % 20Lx % 20Lx % 20Lx\n", - SmramMap[Idx].PhysicalStart, - SmramMap[Idx].PhysicalSize, - SmramMap[Idx].CpuStart, - SmramMap[Idx].RegionState - )); - } - } - DEBUG_CODE_END (); - - GuidHob = BuildGuidHob ( - &gEfiAcpiVariableGuid, - sizeof SmramMap[DescIdxSmmS3ResumeState] - ); - if (GuidHob == NULL) { - return EFI_OUT_OF_RESOURCES; - } - - CopyMem ( - GuidHob, - &SmramMap[DescIdxSmmS3ResumeState], - sizeof SmramMap[DescIdxSmmS3ResumeState] - ); // // SmramAccessLock() depends on "mQ35SmramAtDefaultSmbase"; init the latter // just before exposing the former via PEI_SMM_ACCESS_PPI.Lock(). // diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.inf b/OvmfPkg/SmmAccess/SmmAccessPei.inf index 1698c4ce6c..98bbda20e3 100644 --- a/OvmfPkg/SmmAccess/SmmAccessPei.inf +++ b/OvmfPkg/SmmAccess/SmmAccessPei.inf @@ -5,10 +5,11 @@ # - verify & configure the Q35 TSEG in the entry point, # - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, and expose # it via the gEfiAcpiVariableGuid GUIDed HOB. # # Copyright (C) 2013, 2015, Red Hat, Inc. +# Copyright (c) 2024 Intel Corporation. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -34,13 +35,10 @@ [Packages] MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec OvmfPkg/OvmfPkg.dec -[Guids] - gEfiAcpiVariableGuid - [LibraryClasses] BaseLib BaseMemoryLib DebugLib HobLib @@ -55,10 +53,13 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes +[Guids] + gEfiSmmSmramMemoryGuid # ALWAYS_CONSUMED + [Ppis] gPeiSmmAccessPpiGuid ## PRODUCES [Depex] gEfiPeiMemoryDiscoveredPpiGuid diff --git a/OvmfPkg/SmmAccess/SmramInternal.c b/OvmfPkg/SmmAccess/SmramInternal.c index d391ddc9ae..312f67b304 100644 --- a/OvmfPkg/SmmAccess/SmramInternal.c +++ b/OvmfPkg/SmmAccess/SmramInternal.c @@ -1,20 +1,22 @@ /** @file Functions and types shared by the SMM accessor PEI and DXE modules. Copyright (C) 2015, Red Hat, Inc. + Copyright (c) 2024 Intel Corporation. SPDX-License-Identifier: BSD-2-Clause-Patent **/ -#include +#include #include #include #include #include +#include #include "SmramInternal.h" // // The value of PcdQ35TsegMbytes is saved into this variable at module startup. @@ -164,70 +166,45 @@ SmramAccessLock ( return EFI_SUCCESS; } EFI_STATUS SmramAccessGetCapabilities ( - IN BOOLEAN LockState, - IN BOOLEAN OpenState, IN OUT UINTN *SmramMapSize, IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap ) { - UINTN OriginalSize; - UINT32 TsegMemoryBaseMb, TsegMemoryBase; - UINT64 CommonRegionState; - UINT8 TsegSizeBits; - - OriginalSize = *SmramMapSize; - *SmramMapSize = DescIdxCount * sizeof *SmramMap; - if (OriginalSize < *SmramMapSize) { - return EFI_BUFFER_TOO_SMALL; - } + UINTN BufferSize; + EFI_HOB_GUID_TYPE *GuidHob; + EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *DescriptorBlock; + UINTN Index; // - // Read the TSEG Memory Base register. + // Get Hob list // - TsegMemoryBaseMb = PciRead32 (DRAMC_REGISTER_Q35 (MCH_TSEGMB)); - TsegMemoryBase = (TsegMemoryBaseMb >> MCH_TSEGMB_MB_SHIFT) << 20; + GuidHob = GetFirstGuidHob (&gEfiSmmSmramMemoryGuid); + DescriptorBlock = GET_GUID_HOB_DATA (GuidHob); + ASSERT (DescriptorBlock); - // - // Precompute the region state bits that will be set for all regions. - // - CommonRegionState = (OpenState ? EFI_SMRAM_OPEN : EFI_SMRAM_CLOSED) | - (LockState ? EFI_SMRAM_LOCKED : 0) | - EFI_CACHEABLE; + BufferSize = DescriptorBlock->NumberOfSmmReservedRegions * sizeof (EFI_SMRAM_DESCRIPTOR); - // - // The first region hosts an SMM_S3_RESUME_STATE object. It is located at the - // start of TSEG. We round up the size to whole pages, and we report it as - // EFI_ALLOCATED, so that the SMM_CORE stays away from it. - // - SmramMap[DescIdxSmmS3ResumeState].PhysicalStart = TsegMemoryBase; - SmramMap[DescIdxSmmS3ResumeState].CpuStart = TsegMemoryBase; - SmramMap[DescIdxSmmS3ResumeState].PhysicalSize = - EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (sizeof (SMM_S3_RESUME_STATE))); - SmramMap[DescIdxSmmS3ResumeState].RegionState = - CommonRegionState | EFI_ALLOCATED; + if (*SmramMapSize < BufferSize) { + *SmramMapSize = BufferSize; + return EFI_BUFFER_TOO_SMALL; + } // - // Get the TSEG size bits from the ESMRAMC register. + // Update SmramMapSize to real return SMRAM map size // - TsegSizeBits = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC)) & - MCH_ESMRAMC_TSEG_MASK; + *SmramMapSize = BufferSize; // - // The second region is the main one, following the first. + // Use the hob to publish SMRAM capabilities // - SmramMap[DescIdxMain].PhysicalStart = - SmramMap[DescIdxSmmS3ResumeState].PhysicalStart + - SmramMap[DescIdxSmmS3ResumeState].PhysicalSize; - SmramMap[DescIdxMain].CpuStart = SmramMap[DescIdxMain].PhysicalStart; - SmramMap[DescIdxMain].PhysicalSize = - (TsegSizeBits == MCH_ESMRAMC_TSEG_8MB ? SIZE_8MB : - TsegSizeBits == MCH_ESMRAMC_TSEG_2MB ? SIZE_2MB : - TsegSizeBits == MCH_ESMRAMC_TSEG_1MB ? SIZE_1MB : - mQ35TsegMbytes * SIZE_1MB) - - SmramMap[DescIdxSmmS3ResumeState].PhysicalSize; - SmramMap[DescIdxMain].RegionState = CommonRegionState; + for (Index = 0; Index < DescriptorBlock->NumberOfSmmReservedRegions; Index++) { + SmramMap[Index].PhysicalStart = DescriptorBlock->Descriptor[Index].PhysicalStart; + SmramMap[Index].CpuStart = DescriptorBlock->Descriptor[Index].CpuStart; + SmramMap[Index].PhysicalSize = DescriptorBlock->Descriptor[Index].PhysicalSize; + SmramMap[Index].RegionState = DescriptorBlock->Descriptor[Index].RegionState; + } return EFI_SUCCESS; } diff --git a/OvmfPkg/SmmAccess/SmramInternal.h b/OvmfPkg/SmmAccess/SmramInternal.h index da5b7bbca1..4dd1c6dc9b 100644 --- a/OvmfPkg/SmmAccess/SmramInternal.h +++ b/OvmfPkg/SmmAccess/SmramInternal.h @@ -1,32 +1,18 @@ /** @file Functions and types shared by the SMM accessor PEI and DXE modules. Copyright (C) 2015, Red Hat, Inc. + Copyright (c) 2024 Intel Corporation. SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include -// -// We'll have two SMRAM ranges. -// -// The first is a tiny one that hosts an SMM_S3_RESUME_STATE object, to be -// filled in by the CPU SMM driver during normal boot, for the PEI instance of -// the LockBox library (which will rely on the object during S3 resume). -// -// The other SMRAM range is the main one, for the SMM core and the SMM drivers. -// -typedef enum { - DescIdxSmmS3ResumeState = 0, - DescIdxMain = 1, - DescIdxCount = 2 -} DESCRIPTOR_INDEX; - // // The value of PcdQ35TsegMbytes is saved into this variable at module startup. // extern UINT16 mQ35TsegMbytes; @@ -95,10 +81,8 @@ SmramAccessLock ( IN OUT BOOLEAN *OpenState ); EFI_STATUS SmramAccessGetCapabilities ( - IN BOOLEAN LockState, - IN BOOLEAN OpenState, IN OUT UINTN *SmramMapSize, IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap ); -- 2.16.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117597): https://edk2.groups.io/g/devel/message/117597 Mute This Topic: https://groups.io/mt/105442001/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-