public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Chang, Abner" <abner.chang@amd.com>
To: "Attar, AbdulLateef (Abdul Lateef)" <AbdulLateef.Attar@amd.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Attar, AbdulLateef (Abdul Lateef)" <AbdulLateef.Attar@amd.com>,
	"Grimes, Paul" <Paul.Grimes@amd.com>,
	"Kirkendall, Garrett" <Garrett.Kirkendall@amd.com>,
	Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Jordan Justen <jordan.l.justen@intel.com>
Subject: Re: [PATCH v7 8/8] UefiCpuPkg: Uses SmmSmramSaveStateLib library
Date: Mon, 10 Apr 2023 04:38:22 +0000	[thread overview]
Message-ID: <MN2PR12MB396665BD1560D8F737E6643FEA959@MN2PR12MB3966.namprd12.prod.outlook.com> (raw)
In-Reply-To: <6ec51c656b9b935263a8438ca144649488ffac95.1680847286.git.abdattar@amd.com>

[AMD Official Use Only - General]

Hi Abdul,
Comments inline.

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Friday, April 7, 2023 2:58 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett
> <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>;
> Eric Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Rahul Kumar
> <rahul1.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao
> <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>
> Subject: [PATCH v7 8/8] UefiCpuPkg: Uses SmmSmramSaveStateLib library
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182
> 
> Uses new SmmSmramSaveStateLib library.
> Removes old code.
> 
> Cc: Paul Grimes <paul.grimes@amd.com>
> Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> ---
>  OvmfPkg/OvmfPkgIa32X64.dsc                   |   2 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   2 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |   2 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   |   5 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c   | 500 +------------------
>  5 files changed, 11 insertions(+), 500 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
We should have Ovmf changes in a separate patch.
All others look good.

Thanks
Abner


> index 065b54450647..39465fa23739 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -4,6 +4,7 @@
>  #  Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>  #  (C)
> Copyright 2016 Hewlett Packard Enterprise Development LP<BR>  #
> Copyright (c) Microsoft Corporation.
> +#  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -451,6 +452,7 @@
> [LibraryClasses.common.DXE_SMM_DRIVER]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
>    PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> 
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/Smm
> CpuRendezvousLib.inf
> +
> +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmS
> + mramSaveStateLib.inf
> 
>  [LibraryClasses.common.SMM_CORE]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index 158e05e2646c..c57b6eea0e14 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -6,6 +6,7 @@
>  #
>  # Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>  #
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -97,6 +98,7 @@
> [LibraryClasses]
>    ReportStatusCodeLib
>    SmmCpuFeaturesLib
>    PeCoffGetEntryPointLib
> +  SmmSmramSaveStateLib
> 
>  [Protocols]
>    gEfiSmmAccess2ProtocolGuid               ## CONSUMES
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index a5c2bdd971ca..10df1e908ba8 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -3,6 +3,7 @@ Agent Module to load other modules to deploy SMM
> Entry Vector for X86 CPU.
> 
>  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -50,6 +51,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
> <Library/SmmCpuFeaturesLib.h>  #include
> <Library/PeCoffGetEntryPointLib.h>
>  #include <Library/RegisterCpuFeaturesLib.h>
> +#include <Library/SmmSmramSaveStateLib.h>
> 
>  #include <AcpiCpuData.h>
>  #include <CpuHotPlugData.h>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index c0e368ea9475..64643aec54d0 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -3,6 +3,7 @@ Agent Module to load other modules to deploy SMM
> Entry Vector for X86 CPU.
> 
>  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -278,7 +279,7 @@ SmmReadSaveState (
> 
>    Status = SmmCpuFeaturesReadSaveStateRegister (CpuIndex, Register,
> Width, Buffer);
>    if (Status == EFI_UNSUPPORTED) {
> -    Status = ReadSaveStateRegister (CpuIndex, Register, Width, Buffer);
> +    Status = SmramSaveStateReadRegister (CpuIndex, Register, Width,
> + Buffer);
>    }
> 
>    return Status;
> @@ -330,7 +331,7 @@ SmmWriteSaveState (
> 
>    Status = SmmCpuFeaturesWriteSaveStateRegister (CpuIndex, Register,
> Width, Buffer);
>    if (Status == EFI_UNSUPPORTED) {
> -    Status = WriteSaveStateRegister (CpuIndex, Register, Width, Buffer);
> +    Status = SmramSaveStateWriteRegister (CpuIndex, Register, Width,
> + Buffer);
>    }
> 
>    return Status;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> index c8ddc6083df6..1e316ee0acdb 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> @@ -2,6 +2,8 @@
>  Provides services to access SMRAM Save State Map
> 
>  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> +
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -48,52 +50,6 @@ extern CONST PROCESSOR_SMM_DESCRIPTOR  gcPsd;
> //  #define LMA  BIT10
> 
> -///
> -/// Macro used to simplify the lookup table entries of type
> CPU_SMM_SAVE_STATE_LOOKUP_ENTRY -/// -#define
> SMM_CPU_OFFSET(Field)  OFFSET_OF (SMRAM_SAVE_STATE_MAP, Field)
> -
> -///
> -/// Macro used to simplify the lookup table entries of type
> CPU_SMM_SAVE_STATE_REGISTER_RANGE -/// -#define
> SMM_REGISTER_RANGE(Start, End)  { Start, End, End - Start + 1 }
> -
> -///
> -/// Structure used to describe a range of registers -/// -typedef struct {
> -  EFI_SMM_SAVE_STATE_REGISTER    Start;
> -  EFI_SMM_SAVE_STATE_REGISTER    End;
> -  UINTN                          Length;
> -} CPU_SMM_SAVE_STATE_REGISTER_RANGE;
> -
> -///
> -/// Structure used to build a lookup table to retrieve the widths and offsets -
> /// associated with each supported EFI_SMM_SAVE_STATE_REGISTER value
> -///
> -
> -#define SMM_SAVE_STATE_REGISTER_SMMREVID_INDEX   1
> -#define SMM_SAVE_STATE_REGISTER_IOMISC_INDEX     2
> -#define SMM_SAVE_STATE_REGISTER_IOMEMADDR_INDEX  3
> -#define SMM_SAVE_STATE_REGISTER_MAX_INDEX        4
> -
> -typedef struct {
> -  UINT8      Width32;
> -  UINT8      Width64;
> -  UINT16     Offset32;
> -  UINT16     Offset64Lo;
> -  UINT16     Offset64Hi;
> -  BOOLEAN    Writeable;
> -} CPU_SMM_SAVE_STATE_LOOKUP_ENTRY;
> -
> -///
> -/// Structure used to build a lookup table for the IOMisc width information -
> /// -typedef struct {
> -  UINT8                          Width;
> -  EFI_SMM_SAVE_STATE_IO_WIDTH    IoWidth;
> -} CPU_SMM_SAVE_STATE_IO_WIDTH;
> -
>  ///
>  /// Variables from SMI Handler
>  ///
> @@ -108,463 +64,11 @@ extern CONST UINT16       gcSmiHandlerSize;
>  //
>  IA32_DESCRIPTOR  gSmiHandlerIdtr;
> 
> -///
> -/// Table used by GetRegisterIndex() to convert an
> EFI_SMM_SAVE_STATE_REGISTER -/// value to an index into a table of type
> CPU_SMM_SAVE_STATE_LOOKUP_ENTRY -/// -CONST
> CPU_SMM_SAVE_STATE_REGISTER_RANGE  mSmmCpuRegisterRanges[] = {
> -  SMM_REGISTER_RANGE (EFI_SMM_SAVE_STATE_REGISTER_GDTBASE,
> EFI_SMM_SAVE_STATE_REGISTER_LDTINFO),
> -  SMM_REGISTER_RANGE (EFI_SMM_SAVE_STATE_REGISTER_ES,
> EFI_SMM_SAVE_STATE_REGISTER_RIP),
> -  SMM_REGISTER_RANGE (EFI_SMM_SAVE_STATE_REGISTER_RFLAGS,
> EFI_SMM_SAVE_STATE_REGISTER_CR4),
> -  { (EFI_SMM_SAVE_STATE_REGISTER)0,
> (EFI_SMM_SAVE_STATE_REGISTER)0,      0}
> -};
> -
> -///
> -/// Lookup table used to retrieve the widths and offsets associated with
> each -/// supported EFI_SMM_SAVE_STATE_REGISTER value -/// -CONST
> CPU_SMM_SAVE_STATE_LOOKUP_ENTRY  mSmmCpuWidthOffset[] = {
> -  { 0, 0, 0,                              0,                                   0,                                   FALSE },  //
> Reserved
> -
> -  //
> -  // Internally defined CPU Save State Registers. Not defined in PI SMM CPU
> Protocol.
> -  //
> -  { 4, 4, SMM_CPU_OFFSET (x86.SMMRevId),  SMM_CPU_OFFSET
> (x64.SMMRevId),       0,                                   FALSE }, //
> SMM_SAVE_STATE_REGISTER_SMMREVID_INDEX  = 1
> -  { 4, 4, SMM_CPU_OFFSET (x86.IOMisc),    SMM_CPU_OFFSET (x64.IOMisc),
> 0,                                   FALSE }, // SMM_SAVE_STATE_REGISTER_IOMISC_INDEX
> = 2
> -  { 4, 8, SMM_CPU_OFFSET (x86.IOMemAddr), SMM_CPU_OFFSET
> (x64.IOMemAddr),      SMM_CPU_OFFSET (x64.IOMemAddr) + 4,  FALSE }, //
> SMM_SAVE_STATE_REGISTER_IOMEMADDR_INDEX = 3
> -
> -  //
> -  // CPU Save State registers defined in PI SMM CPU Protocol.
> -  //
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64.GdtBaseLoDword),
> SMM_CPU_OFFSET (x64.GdtBaseHiDword), FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_GDTBASE  = 4
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64.IdtBaseLoDword),
> SMM_CPU_OFFSET (x64.IdtBaseHiDword), FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_IDTBASE  = 5
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64.LdtBaseLoDword),
> SMM_CPU_OFFSET (x64.LdtBaseHiDword), FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_LDTBASE  = 6
> -  { 0, 0, 0,                              0,                                   0,                                   FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_GDTLIMIT = 7
> -  { 0, 0, 0,                              0,                                   0,                                   FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_IDTLIMIT = 8
> -  { 0, 0, 0,                              0,                                   0,                                   FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_LDTLIMIT = 9
> -  { 0, 0, 0,                              0,                                   0,                                   FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_LDTINFO  = 10
> -
> -  { 4, 4, SMM_CPU_OFFSET (x86._ES),       SMM_CPU_OFFSET (x64._ES),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_ES       = 20
> -  { 4, 4, SMM_CPU_OFFSET (x86._CS),       SMM_CPU_OFFSET (x64._CS),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_CS       = 21
> -  { 4, 4, SMM_CPU_OFFSET (x86._SS),       SMM_CPU_OFFSET (x64._SS),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_SS       = 22
> -  { 4, 4, SMM_CPU_OFFSET (x86._DS),       SMM_CPU_OFFSET (x64._DS),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_DS       = 23
> -  { 4, 4, SMM_CPU_OFFSET (x86._FS),       SMM_CPU_OFFSET (x64._FS),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_FS       = 24
> -  { 4, 4, SMM_CPU_OFFSET (x86._GS),       SMM_CPU_OFFSET (x64._GS),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_GS       = 25
> -  { 0, 4, 0,                              SMM_CPU_OFFSET (x64._LDTR),          0,
> FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_LDTR_SEL = 26
> -  { 4, 4, SMM_CPU_OFFSET (x86._TR),       SMM_CPU_OFFSET (x64._TR),
> 0,                                   FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_TR_SEL   =
> 27
> -  { 4, 8, SMM_CPU_OFFSET (x86._DR7),      SMM_CPU_OFFSET (x64._DR7),
> SMM_CPU_OFFSET (x64._DR7)    + 4,    FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_DR7      = 28
> -  { 4, 8, SMM_CPU_OFFSET (x86._DR6),      SMM_CPU_OFFSET (x64._DR6),
> SMM_CPU_OFFSET (x64._DR6)    + 4,    FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_DR6      = 29
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R8),
> SMM_CPU_OFFSET (x64._R8)     + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R8       = 30
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R9),
> SMM_CPU_OFFSET (x64._R9)     + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R9       = 31
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R10),
> SMM_CPU_OFFSET (x64._R10)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R10      = 32
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R11),
> SMM_CPU_OFFSET (x64._R11)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R11      = 33
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R12),
> SMM_CPU_OFFSET (x64._R12)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R12      = 34
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R13),
> SMM_CPU_OFFSET (x64._R13)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R13      = 35
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R14),
> SMM_CPU_OFFSET (x64._R14)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R14      = 36
> -  { 0, 8, 0,                              SMM_CPU_OFFSET (x64._R15),
> SMM_CPU_OFFSET (x64._R15)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_R15      = 37
> -  { 4, 8, SMM_CPU_OFFSET (x86._EAX),      SMM_CPU_OFFSET (x64._RAX),
> SMM_CPU_OFFSET (x64._RAX)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RAX      = 38
> -  { 4, 8, SMM_CPU_OFFSET (x86._EBX),      SMM_CPU_OFFSET (x64._RBX),
> SMM_CPU_OFFSET (x64._RBX)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RBX      = 39
> -  { 4, 8, SMM_CPU_OFFSET (x86._ECX),      SMM_CPU_OFFSET (x64._RCX),
> SMM_CPU_OFFSET (x64._RCX)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RCX      = 40
> -  { 4, 8, SMM_CPU_OFFSET (x86._EDX),      SMM_CPU_OFFSET (x64._RDX),
> SMM_CPU_OFFSET (x64._RDX)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RDX      = 41
> -  { 4, 8, SMM_CPU_OFFSET (x86._ESP),      SMM_CPU_OFFSET (x64._RSP),
> SMM_CPU_OFFSET (x64._RSP)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RSP      = 42
> -  { 4, 8, SMM_CPU_OFFSET (x86._EBP),      SMM_CPU_OFFSET (x64._RBP),
> SMM_CPU_OFFSET (x64._RBP)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RBP      = 43
> -  { 4, 8, SMM_CPU_OFFSET (x86._ESI),      SMM_CPU_OFFSET (x64._RSI),
> SMM_CPU_OFFSET (x64._RSI)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RSI      = 44
> -  { 4, 8, SMM_CPU_OFFSET (x86._EDI),      SMM_CPU_OFFSET (x64._RDI),
> SMM_CPU_OFFSET (x64._RDI)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RDI      = 45
> -  { 4, 8, SMM_CPU_OFFSET (x86._EIP),      SMM_CPU_OFFSET (x64._RIP),
> SMM_CPU_OFFSET (x64._RIP)    + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RIP      = 46
> -
> -  { 4, 8, SMM_CPU_OFFSET (x86._EFLAGS),   SMM_CPU_OFFSET
> (x64._RFLAGS),        SMM_CPU_OFFSET (x64._RFLAGS) + 4,    TRUE  }, //
> EFI_SMM_SAVE_STATE_REGISTER_RFLAGS   = 51
> -  { 4, 8, SMM_CPU_OFFSET (x86._CR0),      SMM_CPU_OFFSET (x64._CR0),
> SMM_CPU_OFFSET (x64._CR0)    + 4,    FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_CR0      = 52
> -  { 4, 8, SMM_CPU_OFFSET (x86._CR3),      SMM_CPU_OFFSET (x64._CR3),
> SMM_CPU_OFFSET (x64._CR3)    + 4,    FALSE }, //
> EFI_SMM_SAVE_STATE_REGISTER_CR3      = 53
> -  { 0, 4, 0,                              SMM_CPU_OFFSET (x64._CR4),           0,
> FALSE }, //  EFI_SMM_SAVE_STATE_REGISTER_CR4      = 54
> -};
> -
> -///
> -/// Lookup table for the IOMisc width information -/// -CONST
> CPU_SMM_SAVE_STATE_IO_WIDTH  mSmmCpuIoWidth[] = {
> -  { 0, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT8  },  // Undefined           = 0
> -  { 1, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT8  },  //
> SMM_IO_LENGTH_BYTE  = 1
> -  { 2, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT16 },  //
> SMM_IO_LENGTH_WORD  = 2
> -  { 0, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT8  },  // Undefined           = 3
> -  { 4, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT32 },  //
> SMM_IO_LENGTH_DWORD = 4
> -  { 0, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT8  },  // Undefined           = 5
> -  { 0, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT8  },  // Undefined           = 6
> -  { 0, EFI_SMM_SAVE_STATE_IO_WIDTH_UINT8  }   // Undefined           = 7
> -};
> -
> -///
> -/// Lookup table for the IOMisc type information -/// -CONST
> EFI_SMM_SAVE_STATE_IO_TYPE  mSmmCpuIoType[] = {
> -  EFI_SMM_SAVE_STATE_IO_TYPE_OUTPUT,     // SMM_IO_TYPE_OUT_DX
> = 0
> -  EFI_SMM_SAVE_STATE_IO_TYPE_INPUT,      // SMM_IO_TYPE_IN_DX
> = 1
> -  EFI_SMM_SAVE_STATE_IO_TYPE_STRING,     // SMM_IO_TYPE_OUTS
> = 2
> -  EFI_SMM_SAVE_STATE_IO_TYPE_STRING,     // SMM_IO_TYPE_INS           =
> 3
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 4
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 5
> -  EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX, //
> SMM_IO_TYPE_REP_OUTS      = 6
> -  EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX, // SMM_IO_TYPE_REP_INS
> = 7
> -  EFI_SMM_SAVE_STATE_IO_TYPE_OUTPUT,     //
> SMM_IO_TYPE_OUT_IMMEDIATE = 8
> -  EFI_SMM_SAVE_STATE_IO_TYPE_INPUT,      //
> SMM_IO_TYPE_OUT_IMMEDIATE = 9
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 10
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 11
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 12
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 13
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0,         // Undefined                 = 14
> -  (EFI_SMM_SAVE_STATE_IO_TYPE)0          // Undefined                 = 15
> -};
> -
>  ///
>  /// The mode of the CPU at the time an SMI occurs  ///
>  UINT8  mSmmSaveStateRegisterLma;
> 
> -/**
> -  Read information from the CPU save state.
> -
> -  @param  Register  Specifies the CPU register to read form the save state.
> -
> -  @retval 0   Register is not valid
> -  @retval >0  Index into mSmmCpuWidthOffset[] associated with Register
> -
> -**/
> -UINTN
> -GetRegisterIndex (
> -  IN EFI_SMM_SAVE_STATE_REGISTER  Register
> -  )
> -{
> -  UINTN  Index;
> -  UINTN  Offset;
> -
> -  for (Index = 0, Offset = SMM_SAVE_STATE_REGISTER_MAX_INDEX;
> mSmmCpuRegisterRanges[Index].Length != 0; Index++) {
> -    if ((Register >= mSmmCpuRegisterRanges[Index].Start) && (Register <=
> mSmmCpuRegisterRanges[Index].End)) {
> -      return Register - mSmmCpuRegisterRanges[Index].Start + Offset;
> -    }
> -
> -    Offset += mSmmCpuRegisterRanges[Index].Length;
> -  }
> -
> -  return 0;
> -}
> -
> -/**
> -  Read a CPU Save State register on the target processor.
> -
> -  This function abstracts the differences that whether the CPU Save State
> register is in the
> -  IA32 CPU Save State Map or X64 CPU Save State Map.
> -
> -  This function supports reading a CPU Save State register in SMBase
> relocation handler.
> -
> -  @param[in]  CpuIndex       Specifies the zero-based index of the CPU save
> state.
> -  @param[in]  RegisterIndex  Index into mSmmCpuWidthOffset[] look up
> table.
> -  @param[in]  Width          The number of bytes to read from the CPU save
> state.
> -  @param[out] Buffer         Upon return, this holds the CPU register value
> read from the save state.
> -
> -  @retval EFI_SUCCESS           The register was read from Save State.
> -  @retval EFI_NOT_FOUND         The register is not defined for the Save State
> of Processor.
> -  @retval EFI_INVALID_PARAMETER  This or Buffer is NULL.
> -
> -**/
> -EFI_STATUS
> -ReadSaveStateRegisterByIndex (
> -  IN UINTN  CpuIndex,
> -  IN UINTN  RegisterIndex,
> -  IN UINTN  Width,
> -  OUT VOID  *Buffer
> -  )
> -{
> -  SMRAM_SAVE_STATE_MAP  *CpuSaveState;
> -
> -  if (RegisterIndex == 0) {
> -    return EFI_NOT_FOUND;
> -  }
> -
> -  CpuSaveState = gSmst->CpuSaveState[CpuIndex];
> -
> -  if (mSmmSaveStateRegisterLma ==
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT) {
> -    //
> -    // If 32-bit mode width is zero, then the specified register can not be
> accessed
> -    //
> -    if (mSmmCpuWidthOffset[RegisterIndex].Width32 == 0) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // If Width is bigger than the 32-bit mode width, then the specified
> register can not be accessed
> -    //
> -    if (Width > mSmmCpuWidthOffset[RegisterIndex].Width32) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    //
> -    // Write return buffer
> -    //
> -    ASSERT (CpuSaveState != NULL);
> -    CopyMem (Buffer, (UINT8 *)CpuSaveState +
> mSmmCpuWidthOffset[RegisterIndex].Offset32, Width);
> -  } else {
> -    //
> -    // If 64-bit mode width is zero, then the specified register can not be
> accessed
> -    //
> -    if (mSmmCpuWidthOffset[RegisterIndex].Width64 == 0) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // If Width is bigger than the 64-bit mode width, then the specified
> register can not be accessed
> -    //
> -    if (Width > mSmmCpuWidthOffset[RegisterIndex].Width64) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    //
> -    // Write at most 4 of the lower bytes of the return buffer
> -    //
> -    CopyMem (Buffer, (UINT8 *)CpuSaveState +
> mSmmCpuWidthOffset[RegisterIndex].Offset64Lo, MIN (4, Width));
> -    if (Width > 4) {
> -      //
> -      // Write at most 4 of the upper bytes of the return buffer
> -      //
> -      CopyMem ((UINT8 *)Buffer + 4, (UINT8 *)CpuSaveState +
> mSmmCpuWidthOffset[RegisterIndex].Offset64Hi, Width - 4);
> -    }
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Read a CPU Save State register on the target processor.
> -
> -  This function abstracts the differences that whether the CPU Save State
> register is in the
> -  IA32 CPU Save State Map or X64 CPU Save State Map.
> -
> -  This function supports reading a CPU Save State register in SMBase
> relocation handler.
> -
> -  @param[in]  CpuIndex       Specifies the zero-based index of the CPU save
> state.
> -  @param[in]  RegisterIndex  Index into mSmmCpuWidthOffset[] look up
> table.
> -  @param[in]  Width          The number of bytes to read from the CPU save
> state.
> -  @param[out] Buffer         Upon return, this holds the CPU register value
> read from the save state.
> -
> -  @retval EFI_SUCCESS           The register was read from Save State.
> -  @retval EFI_NOT_FOUND         The register is not defined for the Save State
> of Processor.
> -  @retval EFI_INVALID_PARAMETER Buffer is NULL, or Width does not meet
> requirement per Register type.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -ReadSaveStateRegister (
> -  IN UINTN                        CpuIndex,
> -  IN EFI_SMM_SAVE_STATE_REGISTER  Register,
> -  IN UINTN                        Width,
> -  OUT VOID                        *Buffer
> -  )
> -{
> -  UINT32                      SmmRevId;
> -  SMRAM_SAVE_STATE_IOMISC     IoMisc;
> -  EFI_SMM_SAVE_STATE_IO_INFO  *IoInfo;
> -
> -  //
> -  // Check for special EFI_SMM_SAVE_STATE_REGISTER_LMA
> -  //
> -  if (Register == EFI_SMM_SAVE_STATE_REGISTER_LMA) {
> -    //
> -    // Only byte access is supported for this register
> -    //
> -    if (Width != 1) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    *(UINT8 *)Buffer = mSmmSaveStateRegisterLma;
> -
> -    return EFI_SUCCESS;
> -  }
> -
> -  //
> -  // Check for special EFI_SMM_SAVE_STATE_REGISTER_IO
> -  //
> -  if (Register == EFI_SMM_SAVE_STATE_REGISTER_IO) {
> -    //
> -    // Get SMM Revision ID
> -    //
> -    ReadSaveStateRegisterByIndex (CpuIndex,
> SMM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId),
> &SmmRevId);
> -
> -    //
> -    // See if the CPU supports the IOMisc register in the save state
> -    //
> -    if (SmmRevId < SMRAM_SAVE_STATE_MIN_REV_ID_IOMISC) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // Get the IOMisc register value
> -    //
> -    ReadSaveStateRegisterByIndex (CpuIndex,
> SMM_SAVE_STATE_REGISTER_IOMISC_INDEX, sizeof (IoMisc.Uint32),
> &IoMisc.Uint32);
> -
> -    //
> -    // Check for the SMI_FLAG in IOMisc
> -    //
> -    if (IoMisc.Bits.SmiFlag == 0) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // Only support IN/OUT, but not INS/OUTS/REP INS/REP OUTS.
> -    //
> -    if ((mSmmCpuIoType[IoMisc.Bits.Type] !=
> EFI_SMM_SAVE_STATE_IO_TYPE_INPUT) &&
> -        (mSmmCpuIoType[IoMisc.Bits.Type] !=
> EFI_SMM_SAVE_STATE_IO_TYPE_OUTPUT))
> -    {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // Compute index for the I/O Length and I/O Type lookup tables
> -    //
> -    if ((mSmmCpuIoWidth[IoMisc.Bits.Length].Width == 0) ||
> (mSmmCpuIoType[IoMisc.Bits.Type] == 0)) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // Make sure the incoming buffer is large enough to hold IoInfo before
> accessing
> -    //
> -    if (Width < sizeof (EFI_SMM_SAVE_STATE_IO_INFO)) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    //
> -    // Zero the IoInfo structure that will be returned in Buffer
> -    //
> -    IoInfo = (EFI_SMM_SAVE_STATE_IO_INFO *)Buffer;
> -    ZeroMem (IoInfo, sizeof (EFI_SMM_SAVE_STATE_IO_INFO));
> -
> -    //
> -    // Use lookup tables to help fill in all the fields of the IoInfo structure
> -    //
> -    IoInfo->IoPort  = (UINT16)IoMisc.Bits.Port;
> -    IoInfo->IoWidth = mSmmCpuIoWidth[IoMisc.Bits.Length].IoWidth;
> -    IoInfo->IoType  = mSmmCpuIoType[IoMisc.Bits.Type];
> -    ReadSaveStateRegister (CpuIndex,
> EFI_SMM_SAVE_STATE_REGISTER_RAX,
> mSmmCpuIoWidth[IoMisc.Bits.Length].Width, &IoInfo->IoData);
> -    return EFI_SUCCESS;
> -  }
> -
> -  //
> -  // Convert Register to a register lookup table index
> -  //
> -  return ReadSaveStateRegisterByIndex (CpuIndex, GetRegisterIndex
> (Register), Width, Buffer); -}
> -
> -/**
> -  Write value to a CPU Save State register on the target processor.
> -
> -  This function abstracts the differences that whether the CPU Save State
> register is in the
> -  IA32 CPU Save State Map or X64 CPU Save State Map.
> -
> -  This function supports writing a CPU Save State register in SMBase
> relocation handler.
> -
> -  @param[in] CpuIndex       Specifies the zero-based index of the CPU save
> state.
> -  @param[in] RegisterIndex  Index into mSmmCpuWidthOffset[] look up
> table.
> -  @param[in] Width          The number of bytes to read from the CPU save
> state.
> -  @param[in] Buffer         Upon entry, this holds the new CPU register value.
> -
> -  @retval EFI_SUCCESS           The register was written to Save State.
> -  @retval EFI_NOT_FOUND         The register is not defined for the Save State
> of Processor.
> -  @retval EFI_INVALID_PARAMETER  ProcessorIndex or Width is not correct.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -WriteSaveStateRegister (
> -  IN UINTN                        CpuIndex,
> -  IN EFI_SMM_SAVE_STATE_REGISTER  Register,
> -  IN UINTN                        Width,
> -  IN CONST VOID                   *Buffer
> -  )
> -{
> -  UINTN                 RegisterIndex;
> -  SMRAM_SAVE_STATE_MAP  *CpuSaveState;
> -
> -  //
> -  // Writes to EFI_SMM_SAVE_STATE_REGISTER_LMA are ignored
> -  //
> -  if (Register == EFI_SMM_SAVE_STATE_REGISTER_LMA) {
> -    return EFI_SUCCESS;
> -  }
> -
> -  //
> -  // Writes to EFI_SMM_SAVE_STATE_REGISTER_IO are not supported
> -  //
> -  if (Register == EFI_SMM_SAVE_STATE_REGISTER_IO) {
> -    return EFI_NOT_FOUND;
> -  }
> -
> -  //
> -  // Convert Register to a register lookup table index
> -  //
> -  RegisterIndex = GetRegisterIndex (Register);
> -  if (RegisterIndex == 0) {
> -    return EFI_NOT_FOUND;
> -  }
> -
> -  CpuSaveState = gSmst->CpuSaveState[CpuIndex];
> -
> -  //
> -  // Do not write non-writable SaveState, because it will cause exception.
> -  //
> -  if (!mSmmCpuWidthOffset[RegisterIndex].Writeable) {
> -    return EFI_UNSUPPORTED;
> -  }
> -
> -  //
> -  // Check CPU mode
> -  //
> -  if (mSmmSaveStateRegisterLma ==
> EFI_SMM_SAVE_STATE_REGISTER_LMA_32BIT) {
> -    //
> -    // If 32-bit mode width is zero, then the specified register can not be
> accessed
> -    //
> -    if (mSmmCpuWidthOffset[RegisterIndex].Width32 == 0) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // If Width is bigger than the 32-bit mode width, then the specified
> register can not be accessed
> -    //
> -    if (Width > mSmmCpuWidthOffset[RegisterIndex].Width32) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    //
> -    // Write SMM State register
> -    //
> -    ASSERT (CpuSaveState != NULL);
> -    CopyMem ((UINT8 *)CpuSaveState +
> mSmmCpuWidthOffset[RegisterIndex].Offset32, Buffer, Width);
> -  } else {
> -    //
> -    // If 64-bit mode width is zero, then the specified register can not be
> accessed
> -    //
> -    if (mSmmCpuWidthOffset[RegisterIndex].Width64 == 0) {
> -      return EFI_NOT_FOUND;
> -    }
> -
> -    //
> -    // If Width is bigger than the 64-bit mode width, then the specified
> register can not be accessed
> -    //
> -    if (Width > mSmmCpuWidthOffset[RegisterIndex].Width64) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    //
> -    // Write at most 4 of the lower bytes of SMM State register
> -    //
> -    CopyMem ((UINT8 *)CpuSaveState +
> mSmmCpuWidthOffset[RegisterIndex].Offset64Lo, Buffer, MIN (4, Width));
> -    if (Width > 4) {
> -      //
> -      // Write at most 4 of the upper bytes of SMM State register
> -      //
> -      CopyMem ((UINT8 *)CpuSaveState +
> mSmmCpuWidthOffset[RegisterIndex].Offset64Hi, (UINT8 *)Buffer + 4,
> Width - 4);
> -    }
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
>  /**
>    Hook the code executed immediately after an RSM instruction on the
> currently
>    executing CPU.  The mode of code executed immediately after RSM must
> be
> --
> 2.25.1

      reply	other threads:[~2023-04-10  4:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-07  6:57 [PATCH v7 0/8] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib Abdul Lateef Attar
2023-04-07  6:57 ` [PATCH v7 1/8] MdePkg: Adds AMD SMRAM save state map Abdul Lateef Attar
2023-04-07  6:57 ` [PATCH v7 2/8] UefiCpuPkg: Adds SmmSmramSaveStateLib library class Abdul Lateef Attar
2023-04-07  6:57 ` [PATCH v7 3/8] UefiCpuPkg: Implements " Abdul Lateef Attar
2023-04-07  6:57 ` [PATCH v7 4/8] UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code Abdul Lateef Attar
2023-04-07  6:57 ` [PATCH v7 5/8] UefiCpuPkg: Initial implementation of AMD's SmmCpuFeaturesLib Abdul Lateef Attar
2023-04-07  6:57 ` [PATCH v7 6/8] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family Abdul Lateef Attar
2023-04-10  3:20   ` Chang, Abner
2023-04-07  6:58 ` [PATCH v7 7/8] UefiCpuPkg: Implements SmmSmramSaveStateLib for Intel Abdul Lateef Attar
2023-04-10  4:22   ` [edk2-devel] " Chang, Abner
2023-04-07  6:58 ` [PATCH v7 8/8] UefiCpuPkg: Uses SmmSmramSaveStateLib library Abdul Lateef Attar
2023-04-10  4:38   ` Chang, Abner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MN2PR12MB396665BD1560D8F737E6643FEA959@MN2PR12MB3966.namprd12.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox